Skip to content

Commit 373b45c

Browse files
committed
Fix dangling secure_renegotiation pointer after TLSX_FreeAll
ssl->secure_renegotiation caches a pointer into extension data owned by the ssl->extensions list. Three call sites free that list via TLSX_FreeAll without NULLing the cached pointer, leaving it dangling: - wolfSSL_clear() - FreeHandshakeResources() (TLSX_FreeAll branch) - wolfSSL_ResourceFree() After wolfSSL_clear(), calling wolfSSL_SSL_get_secure_renegotiation_support() reads the freed SecureRenegotiation struct. Confirmed heap-use-after-free under ASan with nginx, haproxy, and openssl-compat build profiles. NULL the pointer at all three sites. Add regression test covering the wolfSSL_clear path.
1 parent c36beba commit 373b45c

3 files changed

Lines changed: 46 additions & 0 deletions

File tree

src/internal.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8952,6 +8952,11 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
89528952
#ifdef HAVE_TLS_EXTENSIONS
89538953
#if !defined(NO_TLS)
89548954
TLSX_FreeAll(ssl->extensions, ssl->heap);
8955+
ssl->extensions = NULL;
8956+
#if defined(HAVE_SECURE_RENEGOTIATION) \
8957+
|| defined(HAVE_SERVER_RENEGOTIATION_INFO)
8958+
ssl->secure_renegotiation = NULL;
8959+
#endif
89558960
#endif /* !NO_TLS */
89568961
#ifdef HAVE_ALPN
89578962
if (ssl->alpn_peer_requested != NULL) {
@@ -9315,6 +9320,10 @@ void FreeHandshakeResources(WOLFSSL* ssl)
93159320
/* Some extensions need to be kept for post-handshake querying. */
93169321
TLSX_FreeAll(ssl->extensions, ssl->heap);
93179322
ssl->extensions = NULL;
9323+
#if defined(HAVE_SECURE_RENEGOTIATION) \
9324+
|| defined(HAVE_SERVER_RENEGOTIATION_INFO)
9325+
ssl->secure_renegotiation = NULL;
9326+
#endif
93189327
#else
93199328
#if !defined(NO_CERTS) && !defined(WOLFSSL_NO_SIGALG)
93209329
TLSX_Remove(&ssl->extensions, TLSX_SIGNATURE_ALGORITHMS, ssl->heap);

src/ssl.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10051,6 +10051,10 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out,
1005110051
#if defined(HAVE_TLS_EXTENSIONS) && !defined(NO_TLS)
1005210052
TLSX_FreeAll(ssl->extensions, ssl->heap);
1005310053
ssl->extensions = NULL;
10054+
#if defined(HAVE_SECURE_RENEGOTIATION) \
10055+
|| defined(HAVE_SERVER_RENEGOTIATION_INFO)
10056+
ssl->secure_renegotiation = NULL;
10057+
#endif
1005410058
#endif
1005510059

1005610060
if (ssl->keys.encryptionOn) {

tests/api.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10203,6 +10203,38 @@ static int test_wolfSSL_wolfSSL_UseSecureRenegotiation(void)
1020310203
return EXPECT_RESULT();
1020410204
}
1020510205

10206+
/* TLSX_FreeAll frees the SecureRenegotiation struct but the cached pointer
10207+
* ssl->secure_renegotiation was not cleared, causing a use-after-free when
10208+
* queried after wolfSSL_clear(). */
10209+
static int test_wolfSSL_clear_secure_renegotiation(void)
10210+
{
10211+
EXPECT_DECLS;
10212+
#if (defined(HAVE_SECURE_RENEGOTIATION) || \
10213+
defined(HAVE_SERVER_RENEGOTIATION_INFO)) && \
10214+
(defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) && \
10215+
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS)
10216+
WOLFSSL_CTX *ctx = wolfSSL_CTX_new(wolfSSLv23_client_method());
10217+
WOLFSSL *ssl = wolfSSL_new(ctx);
10218+
10219+
ExpectNotNull(ctx);
10220+
ExpectNotNull(ssl);
10221+
10222+
ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_UseSecureRenegotiation(ssl));
10223+
ExpectNotNull(ssl->secure_renegotiation);
10224+
if (ssl->secure_renegotiation != NULL)
10225+
ssl->secure_renegotiation->enabled = 1;
10226+
10227+
ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_clear(ssl));
10228+
ExpectNull(ssl->secure_renegotiation);
10229+
ExpectIntEQ(WOLFSSL_FAILURE,
10230+
wolfSSL_SSL_get_secure_renegotiation_support(ssl));
10231+
10232+
wolfSSL_free(ssl);
10233+
wolfSSL_CTX_free(ctx);
10234+
#endif
10235+
return EXPECT_RESULT();
10236+
}
10237+
1020610238
/* Test reconnecting with a different ciphersuite after a renegotiation. */
1020710239
static int test_wolfSSL_SCR_Reconnect(void)
1020810240
{
@@ -35770,6 +35802,7 @@ TEST_CASE testCases[] = {
3577035802
TEST_DECL(test_TLSX_TCA_Find),
3577135803
TEST_DECL(test_TLSX_SNI_GetSize_overflow),
3577235804
TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation),
35805+
TEST_DECL(test_wolfSSL_clear_secure_renegotiation),
3577335806
TEST_DECL(test_wolfSSL_SCR_Reconnect),
3577435807
TEST_DECL(test_wolfSSL_SCR_check_enabled),
3577535808
TEST_DECL(test_tls_ext_duplicate),

0 commit comments

Comments
 (0)