Commit aeea7dfaff for openssl.org
commit aeea7dfaff4449f13a335ca2a3fbc87b8a4417bf
Author: Weidong Wang <kenazcharisma@gmail.com>
Date: Fri Mar 20 05:10:53 2026 -0500
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 1e6ef98c41..00ac258682 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 877e783544..4d8e506c5f 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;
}