From 74cd923a78b490d46af9c3dc5c8dc7a741c5e576 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 16 Dec 2020 17:01:06 +0100 Subject: [PATCH 1/1] EVP: Fix memory leak in EVP_PKEY_CTX_dup() 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 (Merged from https://github.com/openssl/openssl/pull/13661) --- crypto/evp/pmeth_lib.c | 46 ++++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index f817173555..8fc309dc99 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -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) -- 2.34.1