Skip to content

Commit ff7a32d

Browse files
committed
Fix TLSX_EchChangeSNI to check hostname length
1 parent 178e10e commit ff7a32d

2 files changed

Lines changed: 47 additions & 0 deletions

File tree

src/tls.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16106,6 +16106,9 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX,
1610616106
hostNameSz = MAX_PUBLIC_NAME_SZ;
1610716107

1610816108
XMEMCPY(serverName, hostName, hostNameSz);
16109+
/* Guarantee NUL termination after truncation so that
16110+
* TLSX_EchRestoreSNI's XSTRLEN cannot read past the buffer. */
16111+
serverName[hostNameSz - 1] = '\0';
1610916112
}
1611016113

1611116114
/* only swap the SNI if one was found; extensions is non-NULL if an

tests/api.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15042,6 +15042,49 @@ static int test_wolfSSL_Tls13_ECH_disable_conn(void)
1504215042

1504315043
return EXPECT_RESULT();
1504415044
}
15045+
/* Regression test: an inner SNI hostname >= MAX_PUBLIC_NAME_SZ (256) bytes
15046+
* must not cause a stack-buffer-overflow in TLSX_EchRestoreSNI. Before the
15047+
* fix, the truncated copy omitted the NUL terminator and XSTRLEN read past
15048+
* the buffer. */
15049+
static int test_wolfSSL_Tls13_ECH_long_SNI(void)
15050+
{
15051+
EXPECT_DECLS;
15052+
#if !defined(NO_WOLFSSL_CLIENT)
15053+
test_ssl_memio_ctx test_ctx;
15054+
/* 300 chars > MAX_PUBLIC_NAME_SZ (256) to exercise truncation */
15055+
char longName[300];
15056+
15057+
XMEMSET(longName, 'a', sizeof(longName) - 1);
15058+
longName[sizeof(longName) - 1] = '\0';
15059+
15060+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
15061+
15062+
test_ctx.s_cb.method = wolfTLSv1_3_server_method;
15063+
test_ctx.c_cb.method = wolfTLSv1_3_client_method;
15064+
15065+
test_ctx.s_cb.ctx_ready = test_ech_server_ctx_ready;
15066+
test_ctx.s_cb.ssl_ready = test_ech_server_ssl_ready;
15067+
15068+
ExpectIntEQ(test_ssl_memio_setup(&test_ctx), TEST_SUCCESS);
15069+
15070+
/* Set ECH configs on the client */
15071+
ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, echCbTestConfigs,
15072+
echCbTestConfigsLen), WOLFSSL_SUCCESS);
15073+
15074+
/* Set the over-long SNI as the inner hostname */
15075+
ExpectIntEQ(wolfSSL_UseSNI(test_ctx.c_ssl, WOLFSSL_SNI_HOST_NAME,
15076+
longName, (word16)XSTRLEN(longName)), WOLFSSL_SUCCESS);
15077+
15078+
/* The handshake triggers TLSX_EchChangeSNI / TLSX_EchRestoreSNI.
15079+
* Before the fix this would stack-buffer-overflow in XSTRLEN.
15080+
* The connection may fail (SNI mismatch) but must not crash. */
15081+
(void)test_ssl_memio_do_handshake(&test_ctx, 10, NULL);
15082+
15083+
test_ssl_memio_cleanup(&test_ctx);
15084+
#endif /* !NO_WOLFSSL_CLIENT */
15085+
15086+
return EXPECT_RESULT();
15087+
}
1504515088
#endif /* HAVE_SSL_MEMIO_TESTS_DEPENDENCIES */
1504615089

1504715090
/* verify that ECH can be enabled/disabled without issue */
@@ -35790,6 +35833,7 @@ TEST_CASE testCases[] = {
3579035833
TEST_DECL(test_wolfSSL_Tls13_ECH_new_config),
3579135834
TEST_DECL(test_wolfSSL_Tls13_ECH_GREASE),
3579235835
TEST_DECL(test_wolfSSL_Tls13_ECH_disable_conn),
35836+
TEST_DECL(test_wolfSSL_Tls13_ECH_long_SNI),
3579335837
#endif
3579435838
TEST_DECL(test_wolfSSL_Tls13_ECH_enable_disable),
3579535839
#endif /* WOLFSSL_TLS13 && HAVE_ECH */

0 commit comments

Comments
 (0)