From: Weidong Wang <kenazcharisma@gmail.com>
Date: Fri, 20 Mar 2026 10:10:53 +0000 (-0500)
Subject: Fix double-free in mlx_kem_dup() default case
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=aeea7dfaff4449f13a335ca2a3fbc87b8a4417bf;p=thirdparty%2Fopenssl.git

Fix double-free in mlx_kem_dup() default case

Null mkey/xkey immediately after OPENSSL_memdup() so that any failure
path (including propq strdup) can safely call mlx_kem_key_free() without
risking a double-free on the source key's material. Use key->* rather
than ret->* for source-state checks to make ownership explicit.

Test that mlx_kem_dup() with partial key selection (e.g.
EVP_PKEY_PUBLIC_KEY) does not corrupt the original key's mkey/xkey
sub-objects.  Covers X25519MLKEM768, SecP256r1MLKEM768,
and SecP384r1MLKEM1024.

Fixes: 4b1c73d2dd74 "ML-KEM hybrids for TLS"

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
MergeDate: Sun Apr 26 11:14:12 2026
(Merged from https://github.com/openssl/openssl/pull/30511)
---

diff --git a/providers/implementations/keymgmt/mlx_kmgmt.c b/providers/implementations/keymgmt/mlx_kmgmt.c
index 1e6ef98c415..00ac258682a 100644
--- a/providers/implementations/keymgmt/mlx_kmgmt.c
+++ b/providers/implementations/keymgmt/mlx_kmgmt.c
@@ -719,15 +719,17 @@ static void *mlx_kem_dup(const void *vkey, int selection)
         || (ret = OPENSSL_memdup(key, sizeof(*ret))) == NULL)
         return NULL;
 
-    if (ret->propq != NULL
-        && (ret->propq = OPENSSL_strdup(ret->propq)) == NULL) {
+    ret->mkey = ret->xkey = NULL;
+
+    if (key->propq != NULL
+        && (ret->propq = OPENSSL_strdup(key->propq)) == NULL) {
         OPENSSL_free(ret);
         return NULL;
     }
 
     /* Absent key material, nothing left to do */
-    if (ret->mkey == NULL) {
-        if (ret->xkey == NULL)
+    if (key->mkey == NULL) {
+        if (key->xkey == NULL)
             return ret;
         /* Fail if the source key is an inconsistent state */
         OPENSSL_free(ret->propq);
@@ -737,7 +739,6 @@ static void *mlx_kem_dup(const void *vkey, int selection)
 
     switch (selection & OSSL_KEYMGMT_SELECT_KEYPAIR) {
     case 0:
-        ret->xkey = ret->mkey = NULL;
         ret->state = MLX_HAVE_NOKEYS;
         return ret;
     case OSSL_KEYMGMT_SELECT_KEYPAIR:
diff --git a/test/ml_kem_evp_extra_test.c b/test/ml_kem_evp_extra_test.c
index 877e7835441..4d8e506c5f1 100644
--- a/test/ml_kem_evp_extra_test.c
+++ b/test/ml_kem_evp_extra_test.c
@@ -20,6 +20,7 @@
 #include <openssl/param_build.h>
 #include <openssl/rand.h>
 #include <crypto/ml_kem.h>
+#include "crypto/evp.h"
 #include "testutil.h"
 
 static OSSL_LIB_CTX *testctx = NULL;
@@ -420,6 +421,77 @@ static int test_ml_kem_from_data_propq(void)
     return ret;
 }
 
+#ifndef OPENSSL_NO_EC
+static const char *mlx_kem_algs[] = {
+#ifndef OPENSSL_NO_ECX
+    "X25519MLKEM768",
+#endif
+    "SecP256r1MLKEM768",
+    "SecP384r1MLKEM1024",
+};
+#endif
+
+/*
+ * Test that mlx_kem_dup() with partial selection (public-only) does not
+ * corrupt the original key. Before the fix, the default branch of the
+ * switch in mlx_kem_dup() would call mlx_kem_key_free() on a shallow copy
+ * without nulling mkey/xkey first, causing a double-free when the original
+ * key was later freed.
+ */
+#ifndef OPENSSL_NO_EC
+static int test_mlx_kem_dup_partial_selection(int idx)
+{
+    const char *alg = mlx_kem_algs[idx];
+    EVP_PKEY_CTX *genctx = NULL;
+    EVP_PKEY_CTX *encctx = NULL;
+    EVP_PKEY *keypair = NULL;
+    EVP_PKEY *dest = NULL;
+    size_t wrpkeylen = 0, genkeylen = 0;
+    int ret = 0;
+
+    /* Generate an MLX KEM keypair */
+    if (!TEST_ptr(genctx = EVP_PKEY_CTX_new_from_name(testctx, alg, NULL))
+        || !TEST_int_eq(EVP_PKEY_keygen_init(genctx), 1)
+        || !TEST_int_eq(EVP_PKEY_keygen(genctx, &keypair), 1))
+        goto err;
+
+    /*
+     * Attempt a partial copy (public-key only). EVP_PKEY_PUBLIC_KEY includes
+     * OSSL_KEYMGMT_SELECT_PUBLIC_KEY (0x02) but not private, so
+     * selection & OSSL_KEYMGMT_SELECT_KEYPAIR == 0x02 which hits the default
+     * branch in mlx_kem_dup(). This should fail gracefully without corrupting
+     * the source key.
+     */
+    if (!TEST_ptr(dest = EVP_PKEY_new()))
+        goto err;
+    /* Expected to fail — partial duplication is not supported for MLX KEM */
+    evp_keymgmt_util_copy(dest, keypair, EVP_PKEY_PUBLIC_KEY);
+    ERR_clear_error();
+
+    /*
+     * Verify the original keypair is still intact by performing an
+     * encapsulate operation. If the partial copy corrupted the key
+     * (double-freed mkey/xkey), this would crash or trigger ASan.
+     */
+    if (!TEST_ptr(encctx = EVP_PKEY_CTX_new_from_pkey(testctx, keypair, NULL))
+        || !TEST_int_gt(EVP_PKEY_encapsulate_init(encctx, NULL), 0)
+        || !TEST_int_gt(EVP_PKEY_encapsulate(encctx, NULL, &wrpkeylen,
+                            NULL, &genkeylen),
+            0)
+        || !TEST_size_t_gt(wrpkeylen, 0)
+        || !TEST_size_t_gt(genkeylen, 0))
+        goto err;
+
+    ret = 1;
+err:
+    EVP_PKEY_CTX_free(encctx);
+    EVP_PKEY_free(dest);
+    EVP_PKEY_free(keypair);
+    EVP_PKEY_CTX_free(genctx);
+    return ret;
+}
+#endif /* OPENSSL_NO_EC */
+
 int setup_tests(void)
 {
     int test_rand = 0;
@@ -448,5 +520,8 @@ int setup_tests(void)
 
     ADD_TEST(test_ml_kem);
     ADD_TEST(test_ml_kem_from_data_propq);
+#ifndef OPENSSL_NO_EC
+    ADD_ALL_TESTS(test_mlx_kem_dup_partial_selection, OSSL_NELEM(mlx_kem_algs));
+#endif
     return 1;
 }