From f7b4dab9477e196ea1ccabbe8792ca5afdb39c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 5 Aug 2026 17:05:22 +0200 Subject: [PATCH] Fix double free in wolfSSL_X509_EXTENSION_set_data The function freed the extension's dynamically allocated ASN.1 string buffer but left value.data and value.isDynamic pointing at it. The subsequent wolfSSL_ASN1_STRING_copy() call snapshots those fields before copying and frees the old buffer once the copy is complete, so the stale pointer was freed a second time. Any second call to wolfSSL_X509_EXTENSION_set_data() on an extension holding a value of at least CTC_NAME_SIZE bytes hit this, and passing the extension its own value made the copy read freed memory as well. wolfSSL_ASN1_STRING_set() already performs an alias safe replacement and disposes of the previous buffer itself, so drop the manual free. Add a regression test that replaces a dynamically allocated extension value and then sets the value from itself. Fixes F-7340. --- src/x509.c | 12 ++++----- tests/api/test_ossl_x509_ext.c | 46 ++++++++++++++++++++++++++++++++++ tests/api/test_ossl_x509_ext.h | 2 ++ 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/x509.c b/src/x509.c index 2c702989ae..555d402a58 100644 --- a/src/x509.c +++ b/src/x509.c @@ -3776,16 +3776,14 @@ WOLFSSL_ASN1_STRING* wolfSSL_X509_EXTENSION_get_data( int wolfSSL_X509_EXTENSION_set_data(WOLFSSL_X509_EXTENSION* ext, WOLFSSL_ASN1_STRING* data) { - WOLFSSL_ASN1_STRING* current; - if (ext == NULL || data == NULL) return WOLFSSL_FAILURE; - current = wolfSSL_X509_EXTENSION_get_data_internal(ext); - if (current->length > 0 && current->data != NULL && current->isDynamic) { - XFREE(current->data, NULL, DYNAMIC_TYPE_OPENSSL); - } - + /* wolfSSL_ASN1_STRING_copy() defers to wolfSSL_ASN1_STRING_set(), which + * owns the free of any existing dynamic buffer and only releases it once + * the new contents have been copied. Freeing it here as well would leave + * ext->value.data dangling for that copy, so leave the buffer alone; the + * self-aliased case (data == &ext->value) keeps working too. */ return wolfSSL_ASN1_STRING_copy(&ext->value, data); } diff --git a/tests/api/test_ossl_x509_ext.c b/tests/api/test_ossl_x509_ext.c index 18849b4fd6..616f798280 100644 --- a/tests/api/test_ossl_x509_ext.c +++ b/tests/api/test_ossl_x509_ext.c @@ -850,6 +850,52 @@ int test_wolfSSL_X509_EXTENSION_get_data(void) return EXPECT_RESULT(); } +int test_wolfSSL_X509_EXTENSION_set_data(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL) + WOLFSSL_X509_EXTENSION* ext = NULL; + WOLFSSL_ASN1_STRING* str = NULL; +#ifndef WOLFSSL_OLD_EXTDATA_FMT + WOLFSSL_ASN1_STRING* cur = NULL; +#endif + /* Long enough that the ASN.1 STRING data is dynamically allocated. */ + byte longData[CTC_NAME_SIZE * 2]; + + XMEMSET(longData, 'A', sizeof(longData)); + + ExpectNotNull(ext = wolfSSL_X509_EXTENSION_new()); + ExpectNotNull(str = wolfSSL_ASN1_STRING_new()); + ExpectIntEQ(wolfSSL_ASN1_STRING_set(str, longData, (int)sizeof(longData)), + 1); + + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(NULL, NULL), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, NULL), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(NULL, str), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + + /* Replace a dynamically allocated value with another one. */ + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, str), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, str), WOLFSSL_SUCCESS); + +#ifndef WOLFSSL_OLD_EXTDATA_FMT + /* Set the value from itself. */ + ExpectNotNull(cur = wolfSSL_X509_EXTENSION_get_data(ext)); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, cur), WOLFSSL_SUCCESS); + + ExpectNotNull(cur = wolfSSL_X509_EXTENSION_get_data(ext)); + ExpectIntEQ(cur->length, (int)sizeof(longData)); + ExpectBufEQ(cur->data, longData, sizeof(longData)); +#endif + + wolfSSL_ASN1_STRING_free(str); + wolfSSL_X509_EXTENSION_free(ext); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_X509_EXTENSION_get_critical(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_x509_ext.h b/tests/api/test_ossl_x509_ext.h index 13ab76484c..8b25a606c8 100644 --- a/tests/api/test_ossl_x509_ext.h +++ b/tests/api/test_ossl_x509_ext.h @@ -37,6 +37,7 @@ int test_wolfSSL_X509_EXTENSION_new(void); int test_wolfSSL_X509_EXTENSION_dup(void); int test_wolfSSL_X509_EXTENSION_get_object(void); int test_wolfSSL_X509_EXTENSION_get_data(void); +int test_wolfSSL_X509_EXTENSION_set_data(void); int test_wolfSSL_X509_EXTENSION_get_critical(void); int test_wolfSSL_X509_EXTENSION_create_by_OBJ(void); int test_wolfSSL_X509V3_set_ctx(void); @@ -73,6 +74,7 @@ int test_wolfSSL_NAME_CONSTRAINTS_excluded(void); TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_dup), \ TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_get_object), \ TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_get_data), \ + TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_set_data), \ TEST_DECL_GROUP("ossl_x509_ext", \ test_wolfSSL_X509_EXTENSION_get_critical), \ TEST_DECL_GROUP("ossl_x509_ext", \