Skip to content

Fix TLS ext bounds checking #10220

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

Open
embhorn wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix TLS ext bounds checking #10220

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bb9883
Fix TLS ext bounds checking
embhorn Apr 14, 2026
0d4418a
Check tag size in wc_AesEaxDecryptFinal
embhorn Apr 14, 2026
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
97 changes: 82 additions & 15 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6477,7 +6477,7 @@ static int TLSX_SessionTicket_Parse(WOLFSSL* ssl, const byte* input,
return ret;
}

WOLFSSL_LOCAL SessionTicket* TLSX_SessionTicket_Create(word32 lifetime,
WOLFSSL_TEST_VIS SessionTicket* TLSX_SessionTicket_Create(word32 lifetime,
byte* data, word16 size, void* heap)
{
SessionTicket* ticket = (SessionTicket*)XMALLOC(sizeof(SessionTicket),
Expand All @@ -6498,7 +6498,7 @@ WOLFSSL_LOCAL SessionTicket* TLSX_SessionTicket_Create(word32 lifetime,

return ticket;
}
WOLFSSL_LOCAL void TLSX_SessionTicket_Free(SessionTicket* ticket, void* heap)
WOLFSSL_TEST_VIS void TLSX_SessionTicket_Free(SessionTicket* ticket, void* heap)
{
if (ticket) {
XFREE(ticket->data, heap, DYNAMIC_TYPE_TLSX);
Expand Down Expand Up @@ -14159,10 +14159,16 @@ static int TLSX_ECH_ExpandOuterExtensions(WOLFSSL* ssl, WOLFSSL_ECH* ech,
}

if (ret == 0) {
XFREE(ech->innerClientHello, heap, DYNAMIC_TYPE_TMP_BUFFER);
ech->innerClientHello = newInnerCh;
ech->innerClientHelloLen = (word16)newInnerChLen;
newInnerCh = NULL;
if (newInnerChLen > WOLFSSL_MAX_16BIT) {
WOLFSSL_MSG("ECH expanded inner ClientHello exceeds word16");
ret = BUFFER_E;
}
else {
Comment thread
embhorn marked this conversation as resolved.
XFREE(ech->innerClientHello, heap, DYNAMIC_TYPE_TMP_BUFFER);
Comment thread
embhorn marked this conversation as resolved.
ech->innerClientHello = newInnerCh;
ech->innerClientHelloLen = (word16)newInnerChLen;
newInnerCh = NULL;
}
}

if (newInnerCh != NULL)
Expand Down Expand Up @@ -14760,7 +14766,14 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
{
int ret = 0;
TLSX* extension;
word16 length = 0;
/* Use a word32 accumulator so that an extension whose contribution
* pushes the running total past 0xFFFF is detected rather than
* silently wrapped (the TLS extensions block length prefix on the
* wire is a 2-byte field). Callees that take a word16* accumulator
* are invoked via a per-iteration shim and their delta is added
Comment thread
embhorn marked this conversation as resolved.
Outdated
* back into the word32 total. */
word32 length = 0;
word16 cbShim;
byte isRequest = (msgType == client_hello ||
msgType == certificate_request);

Expand Down Expand Up @@ -14846,19 +14859,25 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
#endif
#if defined(HAVE_ENCRYPT_THEN_MAC) && !defined(WOLFSSL_AEAD_ONLY)
case TLSX_ENCRYPT_THEN_MAC:
ret = ETM_GET_SIZE(msgType, &length);
cbShim = 0;
ret = ETM_GET_SIZE(msgType, &cbShim);
length += cbShim;
break;
#endif /* HAVE_ENCRYPT_THEN_MAC */

#if defined(WOLFSSL_TLS13) || !defined(WOLFSSL_NO_TLS12) || !defined(NO_OLD_TLS)
#if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK)
case TLSX_PRE_SHARED_KEY:
cbShim = 0;
ret = PSK_GET_SIZE((PreSharedKey*)extension->data, msgType,
&length);
&cbShim);
length += cbShim;
break;
#ifdef WOLFSSL_TLS13
case TLSX_PSK_KEY_EXCHANGE_MODES:
ret = PKM_GET_SIZE((byte)extension->val, msgType, &length);
cbShim = 0;
ret = PKM_GET_SIZE((byte)extension->val, msgType, &cbShim);
length += cbShim;
break;
#endif
#endif
Expand All @@ -14869,22 +14888,30 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,

#ifdef WOLFSSL_TLS13
case TLSX_SUPPORTED_VERSIONS:
ret = SV_GET_SIZE(extension->data, msgType, &length);
cbShim = 0;
ret = SV_GET_SIZE(extension->data, msgType, &cbShim);
length += cbShim;
break;

case TLSX_COOKIE:
ret = CKE_GET_SIZE((Cookie*)extension->data, msgType, &length);
cbShim = 0;
ret = CKE_GET_SIZE((Cookie*)extension->data, msgType, &cbShim);
length += cbShim;
break;

#ifdef WOLFSSL_EARLY_DATA
case TLSX_EARLY_DATA:
ret = EDI_GET_SIZE(msgType, &length);
cbShim = 0;
ret = EDI_GET_SIZE(msgType, &cbShim);
length += cbShim;
break;
#endif

#ifdef WOLFSSL_POST_HANDSHAKE_AUTH
case TLSX_POST_HANDSHAKE_AUTH:
ret = PHA_GET_SIZE(msgType, &length);
cbShim = 0;
ret = PHA_GET_SIZE(msgType, &cbShim);
length += cbShim;
break;
#endif

Expand Down Expand Up @@ -14940,9 +14967,21 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
/* marks the extension as processed so ctx level */
/* extensions don't overlap with ssl level ones. */
TURN_ON(semaphore, TLSX_ToSemaphore((word16)extension->type));

/* Early exit: stop accumulating as soon as the running total
* cannot possibly fit the 2-byte wire length. */
if (length > WOLFSSL_MAX_16BIT) {
WOLFSSL_MSG("TLSX_GetSize extension length exceeds word16");
return BUFFER_E;
}
Comment thread
embhorn marked this conversation as resolved.
Outdated
Comment thread
embhorn marked this conversation as resolved.
}

*pLength += length;
if ((word32)*pLength + length > WOLFSSL_MAX_16BIT) {
WOLFSSL_MSG("TLSX_GetSize total extensions length exceeds word16");
return BUFFER_E;
}

*pLength += (word16)length;

return ret;
}
Comment thread
embhorn marked this conversation as resolved.
Expand All @@ -14955,6 +14994,7 @@ static int TLSX_Write(TLSX* list, byte* output, byte* semaphore,
TLSX* extension;
word16 offset = 0;
word16 length_offset = 0;
word32 prevOffset;
byte isRequest = (msgType == client_hello ||
msgType == certificate_request);

Expand All @@ -14969,6 +15009,10 @@ static int TLSX_Write(TLSX* list, byte* output, byte* semaphore,
if (!IS_OFF(semaphore, TLSX_ToSemaphore((word16)extension->type)))
continue; /* skip! */

/* Snapshot offset to detect word16 wrap within this iteration;
* see matching comment in TLSX_GetSize. */
prevOffset = offset;
Comment thread
embhorn marked this conversation as resolved.

/* writes extension type. */
c16toa((word16)extension->type, output + offset);
offset += HELLO_EXT_TYPE_SZ + OPAQUE16_LEN;
Expand Down Expand Up @@ -15196,6 +15240,16 @@ static int TLSX_Write(TLSX* list, byte* output, byte* semaphore,
/* if we encountered an error propagate it */
if (ret != 0)
break;

if (offset < prevOffset) {
Comment thread
embhorn marked this conversation as resolved.
Outdated
WOLFSSL_MSG("TLSX_Write extension length exceeds word16");
return BUFFER_E;
}
}

if ((word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {
Comment thread
embhorn marked this conversation as resolved.
Outdated
Comment thread
embhorn marked this conversation as resolved.
Outdated
WOLFSSL_MSG("TLSX_Write total extensions length exceeds word16");
return BUFFER_E;
}
Comment thread
embhorn marked this conversation as resolved.
Outdated

*pOffset += offset;
Comment thread
embhorn marked this conversation as resolved.
Outdated
Expand Down Expand Up @@ -16283,6 +16337,13 @@ int TLSX_GetRequestSize(WOLFSSL* ssl, byte msgType, word32* pLength)
}
#endif

/* The TLS extensions block length prefix is a 2-byte field, so any
* accumulated total above 0xFFFF must be rejected rather than silently
* truncating and producing a short, malformed handshake message. */
if (length > (word16)(WOLFSSL_MAX_16BIT - OPAQUE16_LEN)) {
Comment thread
embhorn marked this conversation as resolved.
WOLFSSL_MSG("TLSX_GetRequestSize extensions exceed word16");
return BUFFER_E;
}
if (length)
length += OPAQUE16_LEN; /* for total length storage. */

Expand Down Expand Up @@ -16486,6 +16547,12 @@ int TLSX_WriteRequest(WOLFSSL* ssl, byte* output, byte msgType, word32* pOffset)
#endif
#endif

/* Wrap detection for the TLSX_Write calls above is handled inside
* TLSX_Write itself: any iteration that would push the local word16
* offset past 0xFFFF returns BUFFER_E so we never reach here with a
* truncated value. The TLS extensions block length prefix on the
* wire is a 2-byte field, matching this invariant. */

if (offset > OPAQUE16_LEN || msgType != client_hello)
c16toa(offset - OPAQUE16_LEN, output); /* extensions length */

Expand Down
16 changes: 14 additions & 2 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4759,8 +4759,20 @@ int SendTls13ClientHello(WOLFSSL* ssl)
args->ech->type = 0;
/* set innerClientHelloLen to ClientHelloInner + padding + tag */
args->ech->paddingLen = 31 - ((args->length - 1) % 32);
args->ech->innerClientHelloLen = (word16)(args->length +
args->ech->paddingLen + args->ech->hpke->Nt);
{
word32 ichLen = args->length + args->ech->paddingLen +
args->ech->hpke->Nt;
/* Guard against word16 truncation: the wire format field
* for the ECH payload length is two bytes, so anything
* above 0xFFFF cannot be represented and the silent cast
* would cause an undersized allocation and heap overflow
Comment thread
embhorn marked this conversation as resolved.
Comment thread
embhorn marked this conversation as resolved.
* in the subsequent XMEMCPY. */
if (ichLen > WOLFSSL_MAX_16BIT) {
WOLFSSL_MSG("ECH inner ClientHello exceeds word16");
return BUFFER_E;
Comment thread
embhorn marked this conversation as resolved.
}
Comment thread
embhorn marked this conversation as resolved.
args->ech->innerClientHelloLen = (word16)ichLen;
}
/* set the length back to before we computed ClientHelloInner size */
args->length = (word32)args->preXLength;
}
Expand Down
62 changes: 62 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10471,6 +10471,67 @@ static int test_tls_ext_duplicate(void)
return EXPECT_RESULT();
}

/* Regression test: TLSX extension size accumulation must not silently wrap
* the internal word16 accumulator. Prior to the fix, a single extension
* whose size (plus the 4-byte header) pushes the running total past 0xFFFF
* caused TLSX_GetSize / TLSX_Write to return a truncated length, which
* in turn led to undersized buffer writes. The on-wire extensions block
* length is a 2-byte field per RFC 8446 Section 4.2, so the correct
* behavior is to return BUFFER_E rather than wrap. */
static int test_tls_ext_word16_overflow(void)
{
EXPECT_DECLS;
#if defined(HAVE_SESSION_TICKET) && !defined(NO_WOLFSSL_CLIENT) && \
!defined(NO_TLS)
WOLFSSL_CTX* ctx = NULL;
WOLFSSL* ssl = NULL;
SessionTicket* ticket = NULL;
byte* big = NULL;
/* Size chosen so that 4 (ext header) + size > 0xFFFF. */
const word16 bigSz = 0xFFFE;
word32 length = 0;

big = (byte*)XMALLOC(bigSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
ExpectNotNull(big);
if (big != NULL)
XMEMSET(big, 0xA5, bigSz);

ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
ExpectNotNull(ssl = wolfSSL_new(ctx));

/* Build an oversized SessionTicket extension directly on the ssl
* extension list. Going via the public API is not enough here because
* wolfSSL_set_SessionTicket clamps to word16 without creating the
* TLSX entry; the TLSX path is what exercises the accumulator. */
if (EXPECT_SUCCESS()) {
ticket = TLSX_SessionTicket_Create(0, big, bigSz, ssl->heap);
ExpectNotNull(ticket);
}
if (EXPECT_SUCCESS()) {
ExpectIntEQ(TLSX_UseSessionTicket(&ssl->extensions, ticket, ssl->heap),
WOLFSSL_SUCCESS);
/* TLSX_UseSessionTicket takes ownership on success. */
ticket = NULL;
}

/* TLSX_GetRequestSize must refuse to encode: 4-byte ext header +
* 0xFFFE payload + 2-byte block length prefix = 0x10004, which does
* not fit in a word16 wire length. Expect BUFFER_E, not a silently
* wrapped small value. */
if (EXPECT_SUCCESS()) {
int ret = TLSX_GetRequestSize(ssl, client_hello, &length);
ExpectIntEQ(ret, BUFFER_E);
}

if (ticket != NULL)
TLSX_SessionTicket_Free(ticket, ssl->heap);
wolfSSL_free(ssl);
wolfSSL_CTX_free(ctx);
XFREE(big, NULL, DYNAMIC_TYPE_TMP_BUFFER);
#endif
return EXPECT_RESULT();
}


/* Test TLS connection abort when legacy version field indicates TLS 1.3 or
* higher. Based on test_tls_ext_duplicate() but with legacy version modified
Expand Down Expand Up @@ -35773,6 +35834,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_wolfSSL_SCR_Reconnect),
TEST_DECL(test_wolfSSL_SCR_check_enabled),
TEST_DECL(test_tls_ext_duplicate),
TEST_DECL(test_tls_ext_word16_overflow),
TEST_DECL(test_tls_bad_legacy_version),
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
#if defined(HAVE_IO_TESTS_DEPENDENCIES)
Expand Down
10 changes: 5 additions & 5 deletions wolfssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3177,9 +3177,9 @@ WOLFSSL_LOCAL int TLSX_SupportExtensions(WOLFSSL* ssl);
WOLFSSL_LOCAL int TLSX_PopulateExtensions(WOLFSSL* ssl, byte isRequest);

#if defined(WOLFSSL_TLS13) || !defined(NO_WOLFSSL_CLIENT)
WOLFSSL_LOCAL int TLSX_GetRequestSize(WOLFSSL* ssl, byte msgType,
WOLFSSL_TEST_VIS int TLSX_GetRequestSize(WOLFSSL* ssl, byte msgType,
word32* pLength);
WOLFSSL_LOCAL int TLSX_WriteRequest(WOLFSSL* ssl, byte* output,
WOLFSSL_TEST_VIS int TLSX_WriteRequest(WOLFSSL* ssl, byte* output,
byte msgType, word32* pOffset);
Comment thread
embhorn marked this conversation as resolved.
Comment thread
embhorn marked this conversation as resolved.
#endif

Expand Down Expand Up @@ -3585,11 +3585,11 @@ typedef struct TicketEncCbCtx {

#endif /* !WOLFSSL_NO_DEF_TICKET_ENC_CB && !NO_WOLFSSL_SERVER */

WOLFSSL_LOCAL int TLSX_UseSessionTicket(TLSX** extensions,
WOLFSSL_TEST_VIS int TLSX_UseSessionTicket(TLSX** extensions,
SessionTicket* ticket, void* heap);
WOLFSSL_LOCAL SessionTicket* TLSX_SessionTicket_Create(word32 lifetime,
WOLFSSL_TEST_VIS SessionTicket* TLSX_SessionTicket_Create(word32 lifetime,
byte* data, word16 size, void* heap);
WOLFSSL_LOCAL void TLSX_SessionTicket_Free(SessionTicket* ticket, void* heap);
WOLFSSL_TEST_VIS void TLSX_SessionTicket_Free(SessionTicket* ticket, void* heap);

#endif /* HAVE_SESSION_TICKET */

Expand Down
Loading