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

@@ -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);
 }
 

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;

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",                                           \