EVP: Fix memory leak in EVP_PKEY_CTX_dup()
authorRichard Levitte <levitte@openssl.org>
Wed, 16 Dec 2020 16:01:06 +0000 (17:01 +0100)
committerRichard Levitte <levitte@openssl.org>
Thu, 17 Dec 2020 11:02:08 +0000 (12:02 +0100)
In most error cases, EVP_PKEY_CTX_dup() would only free the EVP_PKEY_CTX
without freeing the duplicated contents.

Fixes #13503

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13661)

crypto/evp/pmeth_lib.c

index f817173555151395d678af84a311142fc9c784f7..8fc309dc99e74ddc80beec6b79e8aa3f171471b7 100644 (file)
@@ -455,12 +455,6 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
 {
     EVP_PKEY_CTX *rctx;
 
-    if (((pctx->pmeth == NULL) || (pctx->pmeth->copy == NULL))
-            && ((EVP_PKEY_CTX_IS_DERIVE_OP(pctx)
-                 && pctx->op.kex.exchprovctx == NULL)
-                || (EVP_PKEY_CTX_IS_SIGNATURE_OP(pctx)
-                    && pctx->op.sig.sigprovctx == NULL)))
-        return NULL;
 # ifndef OPENSSL_NO_ENGINE
     /* Make sure it's safe to copy a pkey context using an ENGINE */
     if (pctx->engine && !ENGINE_init(pctx->engine)) {
@@ -483,10 +477,8 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
     rctx->propquery = NULL;
     if (pctx->propquery != NULL) {
         rctx->propquery = OPENSSL_strdup(pctx->propquery);
-        if (rctx->propquery == NULL) {
-            OPENSSL_free(rctx);
-            return NULL;
-        }
+        if (rctx->propquery == NULL)
+            goto err;
     }
     rctx->legacy_keytype = pctx->legacy_keytype;
 
@@ -494,16 +486,16 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
         if (pctx->op.kex.exchange != NULL) {
             rctx->op.kex.exchange = pctx->op.kex.exchange;
             if (!EVP_KEYEXCH_up_ref(rctx->op.kex.exchange))
-                goto end;
+                goto err;
         }
         if (pctx->op.kex.exchprovctx != NULL) {
             if (!ossl_assert(pctx->op.kex.exchange != NULL))
-                goto end;
+                goto err;
             rctx->op.kex.exchprovctx
                 = pctx->op.kex.exchange->dupctx(pctx->op.kex.exchprovctx);
             if (rctx->op.kex.exchprovctx == NULL) {
                 EVP_KEYEXCH_free(rctx->op.kex.exchange);
-                goto end;
+                goto err;
             }
             return rctx;
         }
@@ -511,16 +503,16 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
         if (pctx->op.sig.signature != NULL) {
             rctx->op.sig.signature = pctx->op.sig.signature;
             if (!EVP_SIGNATURE_up_ref(rctx->op.sig.signature))
-                goto end;
+                goto err;
         }
         if (pctx->op.sig.sigprovctx != NULL) {
             if (!ossl_assert(pctx->op.sig.signature != NULL))
-                goto end;
+                goto err;
             rctx->op.sig.sigprovctx
                 = pctx->op.sig.signature->dupctx(pctx->op.sig.sigprovctx);
             if (rctx->op.sig.sigprovctx == NULL) {
                 EVP_SIGNATURE_free(rctx->op.sig.signature);
-                goto end;
+                goto err;
             }
             return rctx;
         }
@@ -528,16 +520,16 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
         if (pctx->op.ciph.cipher != NULL) {
             rctx->op.ciph.cipher = pctx->op.ciph.cipher;
             if (!EVP_ASYM_CIPHER_up_ref(rctx->op.ciph.cipher))
-                goto end;
+                goto err;
         }
         if (pctx->op.ciph.ciphprovctx != NULL) {
             if (!ossl_assert(pctx->op.ciph.cipher != NULL))
-                goto end;
+                goto err;
             rctx->op.ciph.ciphprovctx
                 = pctx->op.ciph.cipher->dupctx(pctx->op.ciph.ciphprovctx);
             if (rctx->op.ciph.ciphprovctx == NULL) {
                 EVP_ASYM_CIPHER_free(rctx->op.ciph.cipher);
-                goto end;
+                goto err;
             }
             return rctx;
         }
@@ -545,22 +537,22 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
         if (pctx->op.encap.kem != NULL) {
             rctx->op.encap.kem = pctx->op.encap.kem;
             if (!EVP_KEM_up_ref(rctx->op.encap.kem))
-                goto end;
+                goto err;
         }
         if (pctx->op.encap.kemprovctx != NULL) {
             if (!ossl_assert(pctx->op.encap.kem != NULL))
-                goto end;
+                goto err;
             rctx->op.encap.kemprovctx
                 = pctx->op.encap.kem->dupctx(pctx->op.encap.kemprovctx);
             if (rctx->op.encap.kemprovctx == NULL) {
                 EVP_KEM_free(rctx->op.encap.kem);
-                goto end;
+                goto err;
             }
             return rctx;
         }
     } else if (EVP_PKEY_CTX_IS_GEN_OP(pctx)) {
         /* Not supported - This would need a gen_dupctx() to work */
-        goto end;
+        goto err;
     }
 
     rctx->pmeth = pctx->pmeth;
@@ -587,17 +579,13 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
             rctx->keymgmt = tmp_keymgmt;
             return rctx;
         }
-        goto err;
-    }
-    if (pctx->pmeth->copy(rctx, pctx) > 0)
+    } else if (pctx->pmeth->copy(rctx, pctx) > 0) {
         return rctx;
+    }
 err:
     rctx->pmeth = NULL;
     EVP_PKEY_CTX_free(rctx);
     return NULL;
-end:
-    OPENSSL_free(rctx);
-    return NULL;
 }
 
 int EVP_PKEY_meth_add0(const EVP_PKEY_METHOD *pmeth)