Merge pull request #10210 from ColtonWilley/fix-scr-dangling-ptr-after-tlsx-freeall

Fix dangling secure_renegotiation pointer after TLSX_FreeAll

Author: JacobBarthelmeh

src/internal.c

@@ -8976,6 +8976,11 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
 #ifdef HAVE_TLS_EXTENSIONS
 #if !defined(NO_TLS)
     TLSX_FreeAll(ssl->extensions, ssl->heap);
+    ssl->extensions = NULL;
+#if defined(HAVE_SECURE_RENEGOTIATION) \
+ || defined(HAVE_SERVER_RENEGOTIATION_INFO)
+    ssl->secure_renegotiation = NULL;
+#endif
 #endif /* !NO_TLS */
 #ifdef HAVE_ALPN
     if (ssl->alpn_peer_requested != NULL) {
@@ -9339,6 +9344,10 @@ void FreeHandshakeResources(WOLFSSL* ssl)
     /* Some extensions need to be kept for post-handshake querying. */
     TLSX_FreeAll(ssl->extensions, ssl->heap);
     ssl->extensions = NULL;
+#if defined(HAVE_SECURE_RENEGOTIATION) \
+ || defined(HAVE_SERVER_RENEGOTIATION_INFO)
+    ssl->secure_renegotiation = NULL;
+#endif
 #else
 #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_SIGALG)
     TLSX_Remove(&ssl->extensions, TLSX_SIGNATURE_ALGORITHMS, ssl->heap);

src/ssl.c

@@ -10072,6 +10072,10 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out,
     #if defined(HAVE_TLS_EXTENSIONS) && !defined(NO_TLS)
         TLSX_FreeAll(ssl->extensions, ssl->heap);
         ssl->extensions = NULL;
+      #if defined(HAVE_SECURE_RENEGOTIATION) \
+       || defined(HAVE_SERVER_RENEGOTIATION_INFO)
+        ssl->secure_renegotiation = NULL;
+      #endif
     #endif
 
         if (ssl->keys.encryptionOn) {

tests/api.c

@@ -10277,6 +10277,39 @@ static int test_wolfSSL_wolfSSL_UseSecureRenegotiation(void)
     return EXPECT_RESULT();
 }
 
+/* TLSX_FreeAll frees the SecureRenegotiation struct but the cached pointer
+ * ssl->secure_renegotiation was not cleared, causing a use-after-free when
+ * queried after wolfSSL_clear(). */
+static int test_wolfSSL_clear_secure_renegotiation(void)
+{
+    EXPECT_DECLS;
+#if (defined(HAVE_SECURE_RENEGOTIATION) || \
+     defined(HAVE_SERVER_RENEGOTIATION_INFO)) && \
+    (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS)
+    WOLFSSL_CTX *ctx = wolfSSL_CTX_new(wolfSSLv23_client_method());
+    WOLFSSL     *ssl = wolfSSL_new(ctx);
+    long         support;
+
+    ExpectNotNull(ctx);
+    ExpectNotNull(ssl);
+
+    ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_UseSecureRenegotiation(ssl));
+    ExpectNotNull(ssl->secure_renegotiation);
+    if (ssl->secure_renegotiation != NULL)
+        ssl->secure_renegotiation->enabled = 1;
+
+    ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_clear(ssl));
+    support = wolfSSL_SSL_get_secure_renegotiation_support(ssl);
+    ExpectNull(ssl->secure_renegotiation);
+    ExpectIntEQ(WOLFSSL_FAILURE, support);
+
+    wolfSSL_free(ssl);
+    wolfSSL_CTX_free(ctx);
+#endif
+    return EXPECT_RESULT();
+}
+
 /* Test reconnecting with a different ciphersuite after a renegotiation. */
 static int test_wolfSSL_SCR_Reconnect(void)
 {
@@ -36627,6 +36660,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_TLSX_TCA_Find),
     TEST_DECL(test_TLSX_SNI_GetSize_overflow),
     TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation),
+    TEST_DECL(test_wolfSSL_clear_secure_renegotiation),
     TEST_DECL(test_wolfSSL_SCR_Reconnect),
     TEST_DECL(test_wolfSSL_SCR_check_enabled),
     TEST_DECL(test_tls_ext_duplicate),