Skip to content

Fix TLSX_EchChangeSNI to check hostname termination #10182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SparkiDev merged 1 commit into from
Apr 15, 2026
Merged

Fix TLSX_EchChangeSNI to check hostname termination #10182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16106,6 +16106,9 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX,
hostNameSz = MAX_PUBLIC_NAME_SZ;

XMEMCPY(serverName, hostName, hostNameSz);
/* Guarantee NUL termination after truncation so that
* TLSX_EchRestoreSNI's XSTRLEN cannot read past the buffer. */
serverName[hostNameSz - 1] = '\0';
Comment thread
SparkiDev marked this conversation as resolved.
Comment thread
SparkiDev marked this conversation as resolved.
}

/* only swap the SNI if one was found; extensions is non-NULL if an
Comment thread
embhorn marked this conversation as resolved.
Expand Down
44 changes: 44 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15042,6 +15042,49 @@ static int test_wolfSSL_Tls13_ECH_disable_conn(void)

return EXPECT_RESULT();
}
/* Regression test: an inner SNI hostname >= MAX_PUBLIC_NAME_SZ (256) bytes
* must not cause a stack-buffer-overflow in TLSX_EchRestoreSNI. Before the
* fix, the truncated copy omitted the NUL terminator and XSTRLEN read past
* the buffer. */
static int test_wolfSSL_Tls13_ECH_long_SNI(void)
{
EXPECT_DECLS;
#if !defined(NO_WOLFSSL_CLIENT)
test_ssl_memio_ctx test_ctx;
/* 300 chars > MAX_PUBLIC_NAME_SZ (256) to exercise truncation */
char longName[300];

XMEMSET(longName, 'a', sizeof(longName) - 1);
longName[sizeof(longName) - 1] = '\0';

XMEMSET(&test_ctx, 0, sizeof(test_ctx));

test_ctx.s_cb.method = wolfTLSv1_3_server_method;
test_ctx.c_cb.method = wolfTLSv1_3_client_method;

test_ctx.s_cb.ctx_ready = test_ech_server_ctx_ready;
test_ctx.s_cb.ssl_ready = test_ech_server_ssl_ready;

ExpectIntEQ(test_ssl_memio_setup(&test_ctx), TEST_SUCCESS);

/* Set ECH configs on the client */
ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, echCbTestConfigs,
echCbTestConfigsLen), WOLFSSL_SUCCESS);

/* Set the over-long SNI as the inner hostname */
ExpectIntEQ(wolfSSL_UseSNI(test_ctx.c_ssl, WOLFSSL_SNI_HOST_NAME,
longName, (word16)XSTRLEN(longName)), WOLFSSL_SUCCESS);

/* The handshake triggers TLSX_EchChangeSNI / TLSX_EchRestoreSNI.
* Before the fix this would stack-buffer-overflow in XSTRLEN.
* The connection may fail (SNI mismatch) but must not crash. */
(void)test_ssl_memio_do_handshake(&test_ctx, 10, NULL);

test_ssl_memio_cleanup(&test_ctx);
#endif /* !NO_WOLFSSL_CLIENT */

return EXPECT_RESULT();
}
#endif /* HAVE_SSL_MEMIO_TESTS_DEPENDENCIES */

/* verify that ECH can be enabled/disabled without issue */
Expand Down Expand Up @@ -35790,6 +35833,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_wolfSSL_Tls13_ECH_new_config),
TEST_DECL(test_wolfSSL_Tls13_ECH_GREASE),
TEST_DECL(test_wolfSSL_Tls13_ECH_disable_conn),
TEST_DECL(test_wolfSSL_Tls13_ECH_long_SNI),
#endif
TEST_DECL(test_wolfSSL_Tls13_ECH_enable_disable),
#endif /* WOLFSSL_TLS13 && HAVE_ECH */
Expand Down
Loading