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)

Author: Frauschi

src/tls.c

@@ -16088,11 +16088,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;
 
@@ -16103,15 +16110,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;
@@ -16124,10 +16135,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);