Commit 86a6357d1d for openssl.org
commit 86a6357d1d3accc3278cdf21590759ae861d694a
Author: Acture <acturea@gmail.com>
Date: Fri Mar 27 20:02:17 2026 +0800
Fix double-free of tlsmac in cipher dupctx implementations
Multiple cipher dupctx functions perform shallow copies (via
OPENSSL_memdup or *dctx = *sctx through IMPLEMENT_CIPHER_HW_COPYCTX)
without deep-copying the heap-allocated tlsmac buffer. When both the
original and duplicated contexts are freed, ossl_cipher_generic_reset_ctx
calls OPENSSL_free(ctx->tlsmac) twice on the same address.
Add ossl_cipher_generic_dupctx_tlsmac() as a centralized helper that
deep-copies tlsmac after any shallow dupctx copy. Apply it to all
affected implementations listed in #30548, plus cipher_aes_wrp and
cipher_chacha20 which had hand-rolled fixes that are now unified.
Fixes #30548
Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com>
Reviewed-by: Andrew Dinh <andrewd@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
MergeDate: Sat Aug 8 15:12:21 2026
(Merged from https://github.com/openssl/openssl/pull/30603)
diff --git a/providers/implementations/ciphers/cipher_aes.c b/providers/implementations/ciphers/cipher_aes.c
index 3638919aeb..0bb6b2fdf1 100644
--- a/providers/implementations/ciphers/cipher_aes.c
+++ b/providers/implementations/ciphers/cipher_aes.c
@@ -43,7 +43,10 @@ static void *aes_dupctx(void *ctx)
if (ret == NULL)
return NULL;
in->base.hw->copyctx(&ret->base, &in->base);
-
+ if (!ossl_cipher_generic_dupctx_tlsmac(&ret->base, &in->base)) {
+ OPENSSL_clear_free(ret, sizeof(*ret));
+ return NULL;
+ }
return ret;
}
diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
index 747a30f287..e048ecc7d3 100644
--- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
+++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
@@ -306,6 +306,7 @@ static void *aes_cbc_hmac_sha1_newctx(void *provctx, size_t kbits,
static void *aes_cbc_hmac_sha1_dupctx(void *provctx)
{
PROV_AES_HMAC_SHA1_CTX *ctx = provctx;
+ PROV_AES_HMAC_SHA1_CTX *dctx;
if (!ossl_prov_is_running())
return NULL;
@@ -313,7 +314,14 @@ static void *aes_cbc_hmac_sha1_dupctx(void *provctx)
if (ctx == NULL)
return NULL;
- return OPENSSL_memdup(ctx, sizeof(*ctx));
+ dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base_ctx.base,
+ &ctx->base_ctx.base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
+ }
+ return dctx;
}
static void aes_cbc_hmac_sha1_freectx(void *vctx)
@@ -355,11 +363,22 @@ static void *aes_cbc_hmac_sha256_newctx(void *provctx, size_t kbits,
static void *aes_cbc_hmac_sha256_dupctx(void *provctx)
{
PROV_AES_HMAC_SHA256_CTX *ctx = provctx;
+ PROV_AES_HMAC_SHA256_CTX *dctx;
if (!ossl_prov_is_running())
return NULL;
- return OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (ctx == NULL)
+ return NULL;
+
+ dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base_ctx.base,
+ &ctx->base_ctx.base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
+ }
+ return dctx;
}
static void aes_cbc_hmac_sha256_freectx(void *vctx)
diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha_etm.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha_etm.c
index 12037ff3b9..20bfc12c43 100644
--- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha_etm.c
+++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha_etm.c
@@ -203,11 +203,19 @@ static void aes_cbc_hmac_sha1_etm_freectx(void *vctx)
static void *aes_cbc_hmac_sha1_etm_dupctx(void *provctx)
{
PROV_AES_HMAC_SHA1_ETM_CTX *ctx = provctx;
+ PROV_AES_HMAC_SHA1_ETM_CTX *dctx;
if (ctx == NULL)
return NULL;
- return OPENSSL_memdup(ctx, sizeof(*ctx));
+ dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base_ctx.base,
+ &ctx->base_ctx.base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
+ }
+ return dctx;
}
static void *aes_cbc_hmac_sha256_etm_newctx(void *provctx, size_t kbits,
@@ -240,11 +248,19 @@ static void aes_cbc_hmac_sha256_etm_freectx(void *vctx)
static void *aes_cbc_hmac_sha256_etm_dupctx(void *provctx)
{
PROV_AES_HMAC_SHA256_ETM_CTX *ctx = provctx;
+ PROV_AES_HMAC_SHA256_ETM_CTX *dctx;
if (ctx == NULL)
return NULL;
- return OPENSSL_memdup(ctx, sizeof(*ctx));
+ dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base_ctx.base,
+ &ctx->base_ctx.base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
+ }
+ return dctx;
}
static void *aes_cbc_hmac_sha512_etm_newctx(void *provctx, size_t kbits,
@@ -277,11 +293,19 @@ static void aes_cbc_hmac_sha512_etm_freectx(void *vctx)
static void *aes_cbc_hmac_sha512_etm_dupctx(void *provctx)
{
PROV_AES_HMAC_SHA512_ETM_CTX *ctx = provctx;
+ PROV_AES_HMAC_SHA512_ETM_CTX *dctx;
if (ctx == NULL)
return NULL;
- return OPENSSL_memdup(ctx, sizeof(*ctx));
+ dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base_ctx.base,
+ &ctx->base_ctx.base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
+ }
+ return dctx;
}
#define IMPLEMENT_CIPHER(nm, sub, kbits, blkbits, ivbits, flags) \
diff --git a/providers/implementations/ciphers/cipher_aes_wrp.c b/providers/implementations/ciphers/cipher_aes_wrp.c
index c1dc05e3a9..ec94ea7fd4 100644
--- a/providers/implementations/ciphers/cipher_aes_wrp.c
+++ b/providers/implementations/ciphers/cipher_aes_wrp.c
@@ -76,14 +76,10 @@ static void *aes_wrap_dupctx(void *wctx)
if (ctx == NULL)
return NULL;
dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
-
- if (dctx != NULL && dctx->base.tlsmac != NULL && dctx->base.alloced) {
- dctx->base.tlsmac = OPENSSL_memdup(dctx->base.tlsmac,
- dctx->base.tlsmacsize);
- if (dctx->base.tlsmac == NULL) {
- OPENSSL_free(dctx);
- dctx = NULL;
- }
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base, &ctx->base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
}
return dctx;
}
diff --git a/providers/implementations/ciphers/cipher_chacha20.c b/providers/implementations/ciphers/cipher_chacha20.c
index 19b77d77aa..1d415897dc 100644
--- a/providers/implementations/ciphers/cipher_chacha20.c
+++ b/providers/implementations/ciphers/cipher_chacha20.c
@@ -79,16 +79,14 @@ static void *chacha20_dupctx(void *vctx)
PROV_CHACHA20_CTX *ctx = (PROV_CHACHA20_CTX *)vctx;
PROV_CHACHA20_CTX *dupctx = NULL;
- if (ctx != NULL) {
- dupctx = OPENSSL_memdup(ctx, sizeof(*dupctx));
- if (dupctx != NULL && dupctx->base.tlsmac != NULL && dupctx->base.alloced) {
- dupctx->base.tlsmac = OPENSSL_memdup(dupctx->base.tlsmac,
- dupctx->base.tlsmacsize);
- if (dupctx->base.tlsmac == NULL) {
- OPENSSL_free(dupctx);
- dupctx = NULL;
- }
- }
+ if (ctx == NULL)
+ return NULL;
+
+ dupctx = OPENSSL_memdup(ctx, sizeof(*dupctx));
+ if (dupctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dupctx->base, &ctx->base)) {
+ OPENSSL_clear_free(dupctx, sizeof(*dupctx));
+ return NULL;
}
return dupctx;
}
diff --git a/providers/implementations/ciphers/cipher_chacha20_poly1305.c b/providers/implementations/ciphers/cipher_chacha20_poly1305.c
index 63b263c139..55781ac7c2 100644
--- a/providers/implementations/ciphers/cipher_chacha20_poly1305.c
+++ b/providers/implementations/ciphers/cipher_chacha20_poly1305.c
@@ -68,13 +68,10 @@ static void *chacha20_poly1305_dupctx(void *provctx)
if (ctx == NULL)
return NULL;
dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
- if (dctx != NULL && dctx->base.tlsmac != NULL && dctx->base.alloced) {
- dctx->base.tlsmac = OPENSSL_memdup(dctx->base.tlsmac,
- dctx->base.tlsmacsize);
- if (dctx->base.tlsmac == NULL) {
- OPENSSL_free(dctx);
- dctx = NULL;
- }
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base, &ctx->base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
}
return dctx;
}
diff --git a/providers/implementations/ciphers/cipher_rc4_hmac_md5.c b/providers/implementations/ciphers/cipher_rc4_hmac_md5.c
index f3fbf2e1c4..5683a63f9d 100644
--- a/providers/implementations/ciphers/cipher_rc4_hmac_md5.c
+++ b/providers/implementations/ciphers/cipher_rc4_hmac_md5.c
@@ -76,10 +76,18 @@ static void rc4_hmac_md5_freectx(void *vctx)
static void *rc4_hmac_md5_dupctx(void *vctx)
{
PROV_RC4_HMAC_MD5_CTX *ctx = vctx;
+ PROV_RC4_HMAC_MD5_CTX *dctx;
if (ctx == NULL)
return NULL;
- return OPENSSL_memdup(ctx, sizeof(*ctx));
+
+ dctx = OPENSSL_memdup(ctx, sizeof(*ctx));
+ if (dctx != NULL
+ && !ossl_cipher_generic_dupctx_tlsmac(&dctx->base, &ctx->base)) {
+ OPENSSL_clear_free(dctx, sizeof(*dctx));
+ return NULL;
+ }
+ return dctx;
}
static int rc4_hmac_md5_einit(void *ctx, const unsigned char *key,
diff --git a/providers/implementations/ciphers/ciphercommon.c b/providers/implementations/ciphers/ciphercommon.c
index a6320d2d81..01b34e4ea0 100644
--- a/providers/implementations/ciphers/ciphercommon.c
+++ b/providers/implementations/ciphers/ciphercommon.c
@@ -148,6 +148,25 @@ void ossl_cipher_generic_reset_ctx(PROV_CIPHER_CTX *ctx)
}
}
+/*
+ * Deep-copy the tlsmac buffer after a shallow dupctx copy.
+ * Must be called after OPENSSL_memdup or *dctx = *sctx to avoid
+ * double-free when both contexts are freed.
+ * Returns 1 on success, 0 on allocation failure.
+ */
+int ossl_cipher_generic_dupctx_tlsmac(PROV_CIPHER_CTX *dst,
+ const PROV_CIPHER_CTX *src)
+{
+ if (src->tlsmac != NULL && src->alloced) {
+ dst->tlsmac = OPENSSL_memdup(src->tlsmac, src->tlsmacsize);
+ if (dst->tlsmac == NULL) {
+ dst->alloced = 0;
+ return 0;
+ }
+ }
+ return 1;
+}
+
static int cipher_generic_init_internal(PROV_CIPHER_CTX *ctx,
const unsigned char *key, size_t keylen,
const unsigned char *iv, size_t ivlen,
diff --git a/providers/implementations/include/prov/ciphercommon.h b/providers/implementations/include/prov/ciphercommon.h
index 39f3c36397..b34d62946a 100644
--- a/providers/implementations/include/prov/ciphercommon.h
+++ b/providers/implementations/include/prov/ciphercommon.h
@@ -106,6 +106,8 @@ struct prov_cipher_hw_st {
};
void ossl_cipher_generic_reset_ctx(PROV_CIPHER_CTX *ctx);
+int ossl_cipher_generic_dupctx_tlsmac(PROV_CIPHER_CTX *dst,
+ const PROV_CIPHER_CTX *src);
OSSL_FUNC_cipher_encrypt_init_fn ossl_cipher_generic_einit;
OSSL_FUNC_cipher_decrypt_init_fn ossl_cipher_generic_dinit;
OSSL_FUNC_cipher_update_fn ossl_cipher_generic_block_update;
diff --git a/test/build.info b/test/build.info
index 19cf81e726..53c4a2380d 100644
--- a/test/build.info
+++ b/test/build.info
@@ -813,6 +813,11 @@ IF[{- !$disabled{tests} -}]
INCLUDE[cipher_overhead_test]=.. ../include ../apps/include
DEPEND[cipher_overhead_test]=../libcrypto.a ../libssl.a libtestutil.a
+ PROGRAMS{noinst}=cipher_dupctx_test
+ SOURCE[cipher_dupctx_test]=cipher_dupctx_test.c
+ INCLUDE[cipher_dupctx_test]=../include ../apps/include
+ DEPEND[cipher_dupctx_test]=../libcrypto libtestutil.a
+
SOURCE[uitest]=uitest.c ../apps/lib/apps_ui.c
INCLUDE[uitest]=.. ../include ../apps/include
DEPEND[uitest]=../libcrypto ../libssl libtestutil.a
diff --git a/test/cipher_dupctx_test.c b/test/cipher_dupctx_test.c
new file mode 100644
index 0000000000..acbe12bf54
--- /dev/null
+++ b/test/cipher_dupctx_test.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2026 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License"). You may not use
+ * this file except in compliance with the License. You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+/*
+ * Test that EVP_CIPHER_CTX_copy correctly deep-copies the tlsmac buffer
+ * to prevent double-free when both contexts are freed.
+ * See https://github.com/openssl/openssl/issues/30548
+ */
+
+#include <string.h>
+#include <openssl/evp.h>
+#include <openssl/err.h>
+#include <openssl/params.h>
+#include <openssl/core_names.h>
+#include <openssl/prov_ssl.h>
+#include "testutil.h"
+
+/*
+ * Test that duplicating a cipher context with a heap-allocated tlsmac
+ * buffer does not cause a double-free when both contexts are freed.
+ *
+ * The tlsmac buffer is allocated during TLS CBC decryption via
+ * ossl_cipher_tlsunpadblock -> ssl3_cbc_copy_mac -> OPENSSL_malloc.
+ * Without the fix, the shallow copy in dupctx causes both the original
+ * and duplicated context to share the same pointer, leading to a
+ * double-free in ossl_cipher_generic_reset_ctx.
+ */
+static int test_dupctx_tlsmac(int idx)
+{
+ static const char *cipher_names[] = {
+ "AES-128-CBC",
+ "AES-256-CBC"
+ };
+ const char *name = cipher_names[idx];
+ EVP_CIPHER_CTX *ctx = NULL, *dupctx = NULL;
+ EVP_CIPHER *cipher = NULL;
+ unsigned char key[32] = { 0 };
+ unsigned char iv[16] = { 0 };
+ unsigned char buf[64];
+ int outl = 0;
+ int ret = 0;
+ unsigned int tls_ver = TLS1_VERSION;
+ size_t mac_size = 20; /* SHA1 */
+ OSSL_PARAM params[3];
+
+ cipher = EVP_CIPHER_fetch(NULL, name, NULL);
+ if (!TEST_ptr(cipher))
+ goto err;
+
+ ctx = EVP_CIPHER_CTX_new();
+ if (!TEST_ptr(ctx))
+ goto err;
+
+ if (!TEST_true(EVP_DecryptInit_ex(ctx, cipher, NULL, key, iv)))
+ goto err;
+
+ /* Set TLS parameters to trigger tlsmac allocation */
+ params[0] = OSSL_PARAM_construct_uint(OSSL_CIPHER_PARAM_TLS_VERSION,
+ &tls_ver);
+ params[1] = OSSL_PARAM_construct_size_t(OSSL_CIPHER_PARAM_TLS_MAC_SIZE,
+ &mac_size);
+ params[2] = OSSL_PARAM_construct_end();
+
+ if (!TEST_true(EVP_CIPHER_CTX_set_params(ctx, params)))
+ goto err;
+
+ /*
+ * Perform a decrypt update with enough data to trigger tlsmac
+ * allocation. Buffer needs at least: block_size + mac_size + 1.
+ * For AES-CBC: 16 + 20 + 1 = 37 minimum. Use 64 for safety.
+ * Last byte is padding length (0 = 1 byte of padding).
+ */
+ memset(buf, 0, sizeof(buf));
+ ERR_clear_error();
+ if (!EVP_DecryptUpdate(ctx, buf, &outl, buf, sizeof(buf)))
+ ERR_clear_error();
+
+ /*
+ * Duplicate the context. Before the fix, this created a shallow
+ * copy that shared the tlsmac pointer.
+ */
+ dupctx = EVP_CIPHER_CTX_new();
+ if (!TEST_ptr(dupctx))
+ goto err;
+
+ if (!TEST_true(EVP_CIPHER_CTX_copy(dupctx, ctx)))
+ goto err;
+
+ /*
+ * Free both contexts. Without the fix, the second free triggers
+ * a double-free on the shared tlsmac pointer.
+ * With ASan enabled, this would be detected as "attempting double-free".
+ */
+ EVP_CIPHER_CTX_free(ctx);
+ ctx = NULL;
+ EVP_CIPHER_CTX_free(dupctx);
+ dupctx = NULL;
+
+ ret = 1;
+
+err:
+ EVP_CIPHER_CTX_free(ctx);
+ EVP_CIPHER_CTX_free(dupctx);
+ EVP_CIPHER_free(cipher);
+ return ret;
+}
+
+int setup_tests(void)
+{
+ ADD_ALL_TESTS(test_dupctx_tlsmac, 2);
+ return 1;
+}
diff --git a/test/recipes/30-test_cipher_dupctx.t b/test/recipes/30-test_cipher_dupctx.t
new file mode 100644
index 0000000000..cf0ed3c7d9
--- /dev/null
+++ b/test/recipes/30-test_cipher_dupctx.t
@@ -0,0 +1,16 @@
+#! /usr/bin/env perl
+# Copyright 2026 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the Apache License 2.0 (the "License"). You may not use
+# this file except in compliance with the License. You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+use OpenSSL::Test;
+use OpenSSL::Test::Utils;
+
+setup("test_cipher_dupctx");
+
+plan tests => 1;
+
+ok(run(test(["cipher_dupctx_test"])), "cipher dupctx tlsmac deep copy");