mirror of
https://github.com/wolfSSL/wolfssl.git
synced 2026-07-08 09:20:59 +02:00
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)
This commit is contained in:
committed by
JacobBarthelmeh
parent
e5ab7fa745
commit
1823f2e9fc
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user