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 all commits
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
117 changes: 101 additions & 16 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,9 +14766,27 @@ 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 (`cbShim`) and their delta
* is added back into the word32 total.
*
* MAINTAINER NOTE: do NOT pass &length to any *_GET_SIZE function
* that expects a `word16*` out-parameter -- that would be a type
* mismatch (UB) and would silently bypass the overflow detection
* below. When adding a new extension case, either:
* - use `length += FOO_GET_SIZE(...)` when the helper returns a
* word16 by value, or
* - use the cbShim pattern: `cbShim = 0; ret = FOO_GET_SIZE(...,
* &cbShim); length += cbShim;`
*/
word32 length = 0;
word16 cbShim = 0;
byte isRequest = (msgType == client_hello ||
msgType == certificate_request);
(void)cbShim;

while ((extension = list)) {
list = extension->next;
Expand Down Expand Up @@ -14846,19 +14870,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 +14899,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 @@ -14937,12 +14975,26 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
break;
}

/* Early exit: stop accumulating as soon as the running total
* cannot possibly fit the 2-byte wire length. Check *before*
* marking the extension as processed so the semaphore is not
* left in an inconsistent state on the error path. */
if (length > WOLFSSL_MAX_16BIT) {
WOLFSSL_MSG("TLSX_GetSize extension length exceeds word16");
return BUFFER_E;
}
Comment thread
embhorn marked this conversation as resolved.

/* marks the extension as processed so ctx level */
/* extensions don't overlap with ssl level ones. */
TURN_ON(semaphore, TLSX_ToSemaphore((word16)extension->type));
}

*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 +15007,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 +15022,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,9 +15253,24 @@ static int TLSX_Write(TLSX* list, byte* output, byte* semaphore,
/* if we encountered an error propagate it */
if (ret != 0)
break;

if (offset <= prevOffset) {
WOLFSSL_MSG("TLSX_Write extension length exceeds word16");
return BUFFER_E;
}
}

*pOffset += offset;
/* Only validate and commit the aggregate offset when the loop
* completed without error; on the error path, leave *pOffset
* unchanged and return the original failure reason so callers
* see the real error instead of a masking BUFFER_E. */
if (ret == 0) {
if ((word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {
WOLFSSL_MSG("TLSX_Write total extensions length exceeds word16");
return BUFFER_E;
}
*pOffset += offset;
}

return ret;
}
Expand Down Expand Up @@ -16283,6 +16355,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 +16565,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
Loading
Loading