| Message ID | 20230515155339.38600-1-frank@lichtenheld.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel] sample-plugins: Fix memleak in client-connect example plugin | expand |
Hi, On Mon, May 15, 2023 at 05:53:39PM +0200, Frank Lichtenheld wrote: > 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. > > 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 | 2 +- > 2 files changed, 7 insertions(+), 1 deletion(-) > > 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); ACK on that part.. > @@ -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); > + snprintf(sess->user, sizeof(sess->user) - 1, "%s", (char *)buf); > } ... but adding "%s" to something that shouldn't have been an snprintf() in the first place feels wrong. Don't we have strncpy() or something for "copy a string over to a limited buffer, and null-terminate"? gert
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..f84cf232 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); + snprintf(sess->user, sizeof(sess->user) - 1, "%s", (char *)buf); } OPENSSL_free(buf);
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. 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 | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)