From 1823f2e9fcdf0c3a6a41d255033d817d23a5ebb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 30 Mar 2026 16:44:50 +0200 Subject: [PATCH] tls: fix ECH heap buffer overflow via publicName SNI pollution In TLSX_EchChangeSNI, the ctx->extensions branch set extensions unconditionally even when TLSX_Find returned NULL. This caused TLSX_UseSNI to attach the attacker-controlled publicName to the shared WOLFSSL_CTX when no inner SNI was configured. TLSX_EchRestoreSNI then failed to clean it up because its removal was gated on serverNameX != NULL. The inner ClientHello was sized before the pollution but written after it, causing TLSX_SNI_Write to memcpy 255 bytes past the allocation boundary. Fix by mirroring the guarded pattern of the ssl->extensions branch: only set extensions when TLSX_Find returns non-NULL, and only perform the SNI swap when extensions is non-NULL. Also move TLSX_Remove in TLSX_EchRestoreSNI outside the serverNameX guard so any injected publicName SNI is always cleaned up. Also return BAD_FUNC_ARG when ECH is used without an inner SNI, preventing ECH ClientHello construction in an invalid configuration. Reported by: Nicholas Carlini (Anthropic) & Thai Duong (Calif.io) --- src/tls.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/tls.c b/src/tls.c index b854b8f8cd..09e6c92174 100644 --- a/src/tls.c +++ b/src/tls.c @@ -16086,11 +16086,18 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX, if (serverNameX == NULL && ssl->ctx && ssl->ctx->extensions) { serverNameX = TLSX_Find(ssl->ctx->extensions, TLSX_SERVER_NAME); - extensions = &ssl->ctx->extensions; + if (serverNameX != NULL) + extensions = &ssl->ctx->extensions; + } + + /* ECH requires an inner SNI to be present for ClientHelloInner. + * Without it, fail instead of mutating extension lists. */ + if (serverNameX == NULL) { + ret = BAD_FUNC_ARG; } /* store the inner server name */ - if (serverNameX != NULL) { + if (ret == 0 && serverNameX != NULL) { char* hostName = ((SNI*)serverNameX->data)->data.host_name; word32 hostNameSz = (word32)XSTRLEN(hostName) + 1; @@ -16101,15 +16108,19 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX, XMEMCPY(serverName, hostName, hostNameSz); } - /* remove the inner server name */ - TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap); + /* only swap the SNI if one was found; extensions is non-NULL if an + * SNI entry was found on ssl->extensions or ctx->extensions */ + if (ret == 0 && extensions != NULL) { + /* remove the inner server name */ + TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap); - /* set the public name as the server name */ - if ((ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME, - ((WOLFSSL_ECH*)echX->data)->echConfig->publicName, - XSTRLEN(((WOLFSSL_ECH*)echX->data)->echConfig->publicName), - ssl->heap)) == WOLFSSL_SUCCESS) - ret = 0; + /* set the public name as the server name */ + if ((ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME, + ((WOLFSSL_ECH*)echX->data)->echConfig->publicName, + XSTRLEN(((WOLFSSL_ECH*)echX->data)->echConfig->publicName), + ssl->heap)) == WOLFSSL_SUCCESS) + ret = 0; + } } *pServerNameX = serverNameX; *pExtensions = extensions; @@ -16122,10 +16133,12 @@ static int TLSX_EchRestoreSNI(WOLFSSL* ssl, char* serverName, { int ret = 0; - if (serverNameX != NULL) { - /* remove the public name SNI */ + /* always remove the publicName SNI we injected, regardless of whether + * there was a prior inner SNI to restore */ + if (extensions != NULL) TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap); + if (serverNameX != NULL) { /* restore the inner server name */ ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME, serverName, XSTRLEN(serverName), ssl->heap);