From: Richard Levitte <levitte@openssl.org>
Date: Fri, 7 Feb 2020 08:13:21 +0000 (+0100)
Subject: X509_PUBKEY_set(): Fix memory leak
X-Git-Tag: openssl-3.0.0-alpha1~502
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=a076951b71a1837e68eaf6bfd92e6a4d40b0940a;p=oweals%2Fopenssl.git

X509_PUBKEY_set(): Fix memory leak

With the provided method of creating the new X509_PUBKEY, an extra
EVP_PKEY is created and needs to be properly cleaned away.

(note: we could choose to keep it just as well, but there are
consequences, explained in a comment in the code)

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/11038)
---

diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c
index 42b94d9198..a583813b58 100644
--- a/crypto/x509/x_pubkey.c
+++ b/crypto/x509/x_pubkey.c
@@ -111,6 +111,22 @@ int X509_PUBKEY_set(X509_PUBKEY **x, EVP_PKEY *pkey)
         goto error;
     }
     *x = pk;
+
+    /*
+     * pk->pkey is NULL when using the legacy routine, but is non-NULL when
+     * going through the serializer, and for all intents and purposes, it's
+     * a perfect copy of |pkey|, just not the same instance.  In that case,
+     * we could simply return early, right here.
+     * However, in the interest of being cautious leaning on paranoia, some
+     * application might very well depend on the passed |pkey| being used
+     * and none other, so we spend a few more cycles throwing away the newly
+     * created |pk->pkey| and replace it with |pkey|.
+     * TODO(3.0) Investigate if it's safe to change to simply return here
+     * if |pk->pkey != NULL|.
+     */
+    if (pk->pkey != NULL)
+        EVP_PKEY_free(pk->pkey);
+
     pk->pkey = pkey;
     return 1;