Fix double free in EVP_PKEY_CTX_dup()

If the internal operations dupctx() fails then a free is done (e.g. EVP_KEYEXCH_free()). If this is not set to NULL the EVP_PKEY_CTX_free() will do a double free.
This was found by testing kdf_dupctx() in kdf_exch.c (Note this always
fails since the internal KDF's do not have a dup method).

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16495)

diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c
index 954166caae0168b1d4504d59e62931308d813567..1af16288236e0f1fde5780c0ef608f9125dfcf83 100644 (file)
--- a/crypto/evp/pmeth_lib.c
+++ b/crypto/evp/pmeth_lib.c
@@ -500,6 +500,7 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
                 = pctx->op.kex.exchange->dupctx(pctx->op.kex.algctx);
             if (rctx->op.kex.algctx == NULL) {
                 EVP_KEYEXCH_free(rctx->op.kex.exchange);
+                rctx->op.kex.exchange = NULL;
                 goto err;
             }
             return rctx;
@@ -517,6 +518,7 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
                 = pctx->op.sig.signature->dupctx(pctx->op.sig.algctx);
             if (rctx->op.sig.algctx == NULL) {
                 EVP_SIGNATURE_free(rctx->op.sig.signature);
+                rctx->op.sig.signature = NULL;
                 goto err;
             }
             return rctx;
@@ -534,6 +536,7 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
                 = pctx->op.ciph.cipher->dupctx(pctx->op.ciph.algctx);
             if (rctx->op.ciph.algctx == NULL) {
                 EVP_ASYM_CIPHER_free(rctx->op.ciph.cipher);
+                rctx->op.ciph.cipher = NULL;
                 goto err;
             }
             return rctx;
@@ -551,6 +554,7 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx)
                 = pctx->op.encap.kem->dupctx(pctx->op.encap.algctx);
             if (rctx->op.encap.algctx == NULL) {
                 EVP_KEM_free(rctx->op.encap.kem);
+                rctx->op.encap.kem = NULL;
                 goto err;
             }
             return rctx;

diff --git a/test/evp_pkey_provided_test.c b/test/evp_pkey_provided_test.c
index 593f7090ebdd6dfc24380e7b367ba09d2a582d63..15c8ce77bb89a5e6873da0f7aa899971746d35fa 100644 (file)
--- a/test/evp_pkey_provided_test.c
+++ b/test/evp_pkey_provided_test.c
@@ -14,6 +14,7 @@
 #include <openssl/provider.h>
 #include <openssl/param_build.h>
 #include <openssl/core_names.h>
+#include <openssl/sha.h>
 #include "crypto/ecx.h"
 #include "crypto/evp.h"          /* For the internal API */
 #include "crypto/bn_dh.h"        /* _bignum_ffdhe2048_p */
@@ -1622,6 +1623,52 @@ static int test_check_dsa(void)
 #endif /* OPENSSL_NO_DSA */
 
 
+static OSSL_PARAM *do_construct_hkdf_params(char *digest, char *key,
+                                            size_t keylen, char *salt)
+{
+    OSSL_PARAM *params = OPENSSL_malloc(sizeof(OSSL_PARAM) * 5);
+    OSSL_PARAM *p = params;
+
+    *p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, digest, 0);
+    *p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT,
+                                             salt, strlen(salt));
+    *p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY,
+                                             (unsigned char *)key, keylen);
+    *p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_MODE,
+                                            "EXTRACT_ONLY", 0);
+    *p = OSSL_PARAM_construct_end();
+
+    return params;
+}
+
+/* Test that EVP_PKEY_CTX_dup() fails gracefully for a KDF */
+static int test_evp_pkey_ctx_dup_kdf_fail(void)
+{
+    int ret = 0;
+    size_t len = 0;
+    EVP_PKEY_CTX *pctx = NULL, *dctx = NULL;
+    OSSL_PARAM *params = NULL;
+
+    if (!TEST_ptr(params = do_construct_hkdf_params("sha256", "secret", 6,
+                                                    "salt")))
+        goto err;
+    if (!TEST_ptr(pctx = EVP_PKEY_CTX_new_from_name(NULL, "HKDF", NULL)))
+        goto err;
+    if (!TEST_int_eq(EVP_PKEY_derive_init_ex(pctx, params), 1))
+        goto err;
+    if (!TEST_int_eq(EVP_PKEY_derive(pctx, NULL, &len), 1)
+        || !TEST_size_t_eq(len, SHA256_DIGEST_LENGTH))
+        goto err;
+    if (!TEST_ptr_null(dctx = EVP_PKEY_CTX_dup(pctx)))
+        goto err;
+    ret = 1;
+err:
+    OPENSSL_free(params);
+    EVP_PKEY_CTX_free(dctx);
+    EVP_PKEY_CTX_free(pctx);
+    return ret;
+}
+
 int setup_tests(void)
 {
     if (!test_skip_common_options()) {
@@ -1632,6 +1679,7 @@ int setup_tests(void)
     if (!TEST_ptr(datadir = test_get_argument(0)))
         return 0;
 
+    ADD_TEST(test_evp_pkey_ctx_dup_kdf_fail);
     ADD_TEST(test_evp_pkey_get_bn_param_large);
     ADD_TEST(test_fromdata_rsa);
 #ifndef OPENSSL_NO_DH