[Openvpn-devel,1/3] openssl: fix EVP_PKEY_CTX memory leak

Message ID 20210405080007.1665-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel,1/3] openssl: fix EVP_PKEY_CTX memory leak | expand

Commit Message

Antonio Quartulli April 4, 2021, 10 p.m. UTC
From: Antonio Quartulli <antonio@openvpn.net>

A context allocated with EVP_PKEY_CTX_new_id() must be ultimately free'd
by Eng VP_PKEY_CTX_free(). Failing to do so will result in a memory leak.

This bug was discovered using GCC with "-fsanitize=address".

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 src/openvpn/crypto_openssl.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Gert Doering April 5, 2021, 12:40 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Looks reasonable, matches coding style & OpenSSL documentation, and
passes my client-side tests with OpenSSL 1.1.1

Your patch has been applied to the master branch.

commit 24e58164b845614c2176bc6b2a939856fd830c53
Author: Antonio Quartulli
Date:   Mon Apr 5 10:00:05 2021 +0200

     openssl: fix EVP_PKEY_CTX memory leak

     Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20210405080007.1665-1-a@unstable.cc>
     URL: https://www.mail-archive.com/search?l=mid&q=20210405080007.1665-1-a@unstable.cc
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index f3e86863..d54ca6d2 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -1125,37 +1125,41 @@  bool
 ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret,
              int secret_len, uint8_t *output, int output_len)
 {
+    bool ret = false;
     EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_TLS1_PRF, NULL);
     if (!EVP_PKEY_derive_init(pctx))
     {
-        return false;
+        goto out;
     }
 
     if (!EVP_PKEY_CTX_set_tls1_prf_md(pctx, EVP_md5_sha1()))
     {
-        return false;
+        goto out;
     }
 
     if (!EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, secret, secret_len))
     {
-        return false;
+        goto out;
     }
 
     if (!EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed, seed_len))
     {
-        return false;
+        goto out;
     }
 
     size_t out_len = output_len;
     if (!EVP_PKEY_derive(pctx, output, &out_len))
     {
-        return false;
+        goto out;
     }
     if (out_len != output_len)
     {
-        return false;
+        goto out;
     }
-    return true;
+    ret = true;
+out:
+    EVP_PKEY_CTX_free(pctx);
+    return ret;
 }
 #else  /* if OPENSSL_VERSION_NUMBER >= 0x10100000L */
 /*