| Message ID | 20230516093534.26384-1-frank@lichtenheld.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v2] sample-plugins: Fix memleak in client-connect example plugin | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> I do not really think this is *necessary* (if malloc() fails for a few bytes, how relevant are these free() calls, and how likely is that the next malloc() inside OpenVPN will just make everything stop?) - but it's good practice, and if it silences semi-irrelevant warnings, the check tools can show us the *real* issues... I have only test compiled the plugins, not actually run anything. Your patch has been applied to the master and release/2.6 branch. commit 2d36678a2be15f7c00a44354ab71e4521ee3a4f3 (master) commit 13b8e155af702a25e86f20da694867c76c169673 (release/2.6) Author: Frank Lichtenheld Date: Tue May 16 11:35:34 2023 +0200 sample-plugins: Fix memleak in client-connect example plugin Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20230516093534.26384-1-frank@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26668.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c b/sample/sample-plugins/client-connect/sample-client-connect.c index 391de344..eb242126 100644 --- a/sample/sample-plugins/client-connect/sample-client-connect.c +++ b/sample/sample-plugins/client-connect/sample-client-connect.c @@ -454,6 +454,9 @@ openvpn_plugin_client_connect_v2(struct plugin_context *context, if (!rl->name || !rl->value) { plugin_log(PLOG_ERR, MODULE, "malloc(return_list->xx) failed"); + free(rl->name); + free(rl->value); + free(rl); return OPENVPN_PLUGIN_FUNC_ERROR; } @@ -509,6 +512,9 @@ openvpn_plugin_client_connect_defer_v2(struct plugin_context *context, if (!rl->name || !rl->value) { plugin_log(PLOG_ERR, MODULE, "malloc(return_list->xx) failed"); + free(rl->name); + free(rl->value); + free(rl); return OPENVPN_PLUGIN_FUNC_ERROR; } diff --git a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c index 6a0a1f69..71badf2c 100644 --- a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c +++ b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c @@ -155,7 +155,7 @@ session_user_set(struct session *sess, X509 *x509) if (!strncasecmp(objbuf, "CN", 2)) { - snprintf(sess->user, sizeof(sess->user) - 1, (char *)buf); + strncpy(sess->user, (char *)buf, sizeof(sess->user) - 1); } OPENSSL_free(buf); @@ -234,7 +234,7 @@ tls_final(struct openvpn_plugin_args_func_in const *args, return OPENVPN_PLUGIN_FUNC_ERROR; } - snprintf(sess->key, sizeof(sess->key) - 1, "%s", key); + strncpy(sess->key, key, sizeof(sess->key) - 1); ovpn_note("app session key: %s", sess->key); switch (plugin->type)
I was looking for memleaks in the code and found this one with cppcheck. Only an example, but no need to leave this bug in it. Also fix fortify problem in keying-material-exporter-demo so I can actually test the compilation of the sample plugins. v2: - remove unneccessary usages of snprintf, replace with strncpy. Change-Id: Ibd1b282afc4a28768be3f165f84ab60ca4d24a9b Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> --- .../sample-plugins/client-connect/sample-client-connect.c | 6 ++++++ .../keying-material-exporter-demo/keyingmaterialexporter.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-)