Skip to content

Fenrir fixes #10230

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
julek-wolfssl wants to merge 14 commits into
base: master
Choose a base branch
from
+525 −20
Open

Fenrir fixes #10230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
646ff6d
tls: fix TLSX_ALPN_GetSize word16 overflow (F-2128)
julek-wolfssl Apr 15, 2026
535aaf2
tls: fix TLSX_TCA_GetSize word16 overflow (F-2131)
julek-wolfssl Apr 15, 2026
b8da926
tls: fix TLSX_PreSharedKey_GetSize word16 overflow (F-2925)
julek-wolfssl Apr 15, 2026
0f9fb2f
tls: fix TLSX_CA_Names_GetSize word16 overflow (F-2927)
julek-wolfssl Apr 15, 2026
ef73b3b
tests: add EMS resumption downgrade negative test (F-2915)
julek-wolfssl Apr 15, 2026
01cc5b1
tests: add ChaCha20-Poly1305 AEAD tag negative test (F-2921)
julek-wolfssl Apr 15, 2026
d97d037
tests: add TLS 1.3 null cipher HMAC negative test (F-2916)
julek-wolfssl Apr 15, 2026
920e175
tests: add SCR verify_data mismatch test (F-2913, F-2914)
julek-wolfssl Apr 15, 2026
9aa69f4
tests: add default ticket key callback HMAC negative test (F-2922)
julek-wolfssl Apr 15, 2026
2df4936
tests: add HRR cipher-suite mismatch negative test (F-2126)
julek-wolfssl Apr 15, 2026
ff60134
tests: add TLS 1.3 ticket age out-of-window test (F-1824)
julek-wolfssl Apr 15, 2026
f3e183a
fenrir: address review feedback on PR 10230
julek-wolfssl Apr 20, 2026
558c329
fixup! fenrir: address review feedback on PR 10230
julek-wolfssl Apr 28, 2026
9d49f7f
TLSX_GetSize: return error as soon as it happens
julek-wolfssl Apr 28, 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
87 changes: 67 additions & 20 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1822,16 +1822,20 @@ static void TLSX_ALPN_FreeAll(ALPN *list, void* heap)
static word16 TLSX_ALPN_GetSize(ALPN *list)
{
ALPN* alpn;
word16 length = OPAQUE16_LEN; /* list length */
word32 length = OPAQUE16_LEN; /* list length */

while ((alpn = list)) {
list = alpn->next;

length++; /* protocol name length is on one byte */
length += (word16)XSTRLEN(alpn->protocol_name);
length += (word32)XSTRLEN(alpn->protocol_name);

if (length > WOLFSSL_MAX_16BIT) {
return 0;
}
}

return length;
return (word16)length;
Comment thread
julek-wolfssl marked this conversation as resolved.
}

/** Writes the ALPN objects of a list in a buffer. */
Expand Down Expand Up @@ -2957,7 +2961,7 @@ static void TLSX_TCA_FreeAll(TCA* list, void* heap)
static word16 TLSX_TCA_GetSize(TCA* list)
{
TCA* tca;
word16 length = OPAQUE16_LEN; /* list length */
word32 length = OPAQUE16_LEN; /* list length */

while ((tca = list)) {
list = tca->next;
Expand All @@ -2975,9 +2979,13 @@ static word16 TLSX_TCA_GetSize(TCA* list)
length += OPAQUE16_LEN + tca->idSz;
break;
}

if (length > WOLFSSL_MAX_16BIT) {
return 0;
}
Comment thread
julek-wolfssl marked this conversation as resolved.
}

return length;
return (word16)length;
}

/** Writes the TCA objects of a list in a buffer. */
Expand Down Expand Up @@ -7592,7 +7600,7 @@ static word16 TLSX_CA_Names_GetSize(void* data)
{
WOLFSSL* ssl = (WOLFSSL*)data;
WOLF_STACK_OF(WOLFSSL_X509_NAME)* names;
word16 size = 0;
word32 size = 0;

/* Length of names */
size += OPAQUE16_LEN;
Expand All @@ -7602,11 +7610,14 @@ static word16 TLSX_CA_Names_GetSize(void* data)

if (name != NULL) {
/* 16-bit length | SEQ | Len | DER of name */
size += (word16)(OPAQUE16_LEN + SetSequence(name->rawLen, seq) +
size += (word32)(OPAQUE16_LEN + SetSequence(name->rawLen, seq) +
name->rawLen);
if (size > WOLFSSL_MAX_16BIT) {
return 0;
}
}
}
return size;
return (word16)size;
Comment thread
julek-wolfssl marked this conversation as resolved.
}

static word16 TLSX_CA_Names_Write(void* data, byte* output)
Expand Down Expand Up @@ -11933,14 +11944,22 @@ static int TLSX_PreSharedKey_GetSize(PreSharedKey* list, byte msgType,
{
if (msgType == client_hello) {
/* Length of identities + Length of binders. */
word16 len = OPAQUE16_LEN + OPAQUE16_LEN;
word32 len = OPAQUE16_LEN + OPAQUE16_LEN;
while (list != NULL) {
/* Each entry has: identity, ticket age and binder. */
len += OPAQUE16_LEN + list->identityLen + OPAQUE32_LEN +
OPAQUE8_LEN + (word16)list->binderLen;
OPAQUE8_LEN + (word32)list->binderLen;
if (len > WOLFSSL_MAX_16BIT) {
WOLFSSL_ERROR_VERBOSE(LENGTH_ERROR);
return LENGTH_ERROR;
}
list = list->next;
}
*pSz += len;
if ((word32)*pSz + len > WOLFSSL_MAX_16BIT) {
WOLFSSL_ERROR_VERBOSE(LENGTH_ERROR);
return LENGTH_ERROR;
}
*pSz += (word16)len;
return 0;
Comment thread
julek-wolfssl marked this conversation as resolved.
}

Expand All @@ -11963,7 +11982,7 @@ static int TLSX_PreSharedKey_GetSize(PreSharedKey* list, byte msgType,
int TLSX_PreSharedKey_GetSizeBinders(PreSharedKey* list, byte msgType,
word16* pSz)
{
word16 len;
word32 len;

if (msgType != client_hello) {
WOLFSSL_ERROR_VERBOSE(SANITY_MSG_E);
Expand All @@ -11973,11 +11992,15 @@ int TLSX_PreSharedKey_GetSizeBinders(PreSharedKey* list, byte msgType,
/* Length of all binders. */
len = OPAQUE16_LEN;
while (list != NULL) {
len += OPAQUE8_LEN + (word16)list->binderLen;
len += OPAQUE8_LEN + (word32)list->binderLen;
if (len > WOLFSSL_MAX_16BIT) {
WOLFSSL_ERROR_VERBOSE(LENGTH_ERROR);
return LENGTH_ERROR;
}
list = list->next;
}

*pSz = len;
*pSz = (word16)len;
return 0;
}

Expand Down Expand Up @@ -14937,8 +14960,15 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,

case TLSX_TRUSTED_CA_KEYS:
/* TCA only sends the list on the request. */
if (isRequest)
length += TCA_GET_SIZE((TCA*)extension->data);
if (isRequest) {
word16 tcaSz = TCA_GET_SIZE((TCA*)extension->data);
/* 0 on non-empty list means 16-bit overflow. */
if (tcaSz == 0 && extension->data != NULL) {
ret = LENGTH_ERROR;
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] LENGTH_ERROR from TCA/ALPN/CA_Names cases can be clobbered by later extension handlers · Incorrect error handling

The new ret = LENGTH_ERROR; break; inside the TCA/ALPN/CERTIFICATE_AUTHORITIES cases only breaks the switch. The outer while keeps iterating, and any subsequent extension that unconditionally assigns ret = ETM_GET_SIZE(...) / PSK_GET_SIZE(...) / SV_GET_SIZE(...) / CKE_GET_SIZE(...) / EDI_GET_SIZE(...) / PHA_GET_SIZE(...) / PKM_GET_SIZE(...) / PSK_WITH_CERT_GET_SIZE(...) overwrites the error with 0, so the caller receives a truncated size and no error.

Fix: After setting ret = LENGTH_ERROR, exit the outer while loop (e.g. via a sentinel flag or goto) so subsequent cases cannot overwrite ret.

Copy link
Copy Markdown
Contributor

@JacobBarthelmeh JacobBarthelmeh Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually is a good point. @julek-wolfssl could you take a look?

Copy link
Copy Markdown
Member Author

@julek-wolfssl julek-wolfssl Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed.

break;
}
length += tcaSz;
}
Comment thread
julek-wolfssl marked this conversation as resolved.
break;

case TLSX_MAX_FRAGMENT_LENGTH:
Expand Down Expand Up @@ -14979,9 +15009,16 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
isRequest);
break;

case TLSX_APPLICATION_LAYER_PROTOCOL:
length += ALPN_GET_SIZE((ALPN*)extension->data);
case TLSX_APPLICATION_LAYER_PROTOCOL: {
word16 alpnSz = ALPN_GET_SIZE((ALPN*)extension->data);
/* 0 on non-empty list means 16-bit overflow. */
if (alpnSz == 0 && extension->data != NULL) {
ret = LENGTH_ERROR;
break;
}
length += alpnSz;
break;
}
#if !defined(NO_CERTS) && !defined(WOLFSSL_NO_SIGALG)
case TLSX_SIGNATURE_ALGORITHMS:
length += SA_GET_SIZE(extension->data);
Expand Down Expand Up @@ -15059,9 +15096,16 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
#endif

#if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES)
case TLSX_CERTIFICATE_AUTHORITIES:
length += CAN_GET_SIZE(extension->data);
case TLSX_CERTIFICATE_AUTHORITIES: {
word16 canSz = CAN_GET_SIZE(extension->data);
/* 0 on non-empty list means 16-bit overflow. */
if (canSz == 0 && extension->data != NULL) {
ret = LENGTH_ERROR;
break;
}
length += canSz;
break;
}
#endif
#endif
#ifdef WOLFSSL_SRTP
Expand Down Expand Up @@ -15101,6 +15145,9 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
break;
}

if (ret != 0)
return ret;

/* 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
Expand Down
79 changes: 79 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10473,6 +10473,78 @@ static int test_wolfSSL_SCR_check_enabled(void)
return EXPECT_RESULT();
}

/* F-2922: wolfSSL_TicketKeyCb must reject a session ticket whose HMAC
* does not match its encrypted contents. */
static int test_wolfSSL_ticket_keycb_bad_hmac(void)
{
EXPECT_DECLS;
#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_TLS12) && \
defined(OPENSSL_EXTRA) && defined(HAVE_AES_CBC) && \
defined(WOLFSSL_AES_256) && \
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
!defined(WOLFSSL_NO_DEF_TICKET_ENC_CB)
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL;
WOLFSSL_CTX *ctx_s = NULL;
WOLFSSL *ssl_c = NULL;
WOLFSSL *ssl_s = NULL;
WOLFSSL_SESSION *session = NULL;

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

ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);

ExpectIntEQ(OpenSSLTicketInit(), 0);
ExpectIntEQ(wolfSSL_CTX_set_tlsext_ticket_key_cb(ctx_s,
myTicketEncCbOpenSSL), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_UseSessionTicket(ssl_c), WOLFSSL_SUCCESS);

ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
ExpectNotNull(session = wolfSSL_get1_session(ssl_c));
ExpectIntGT(session->ticketLen, 0);

/* Corrupt a byte of the ticket HMAC so the server's HMAC
* verification rejects it. */
if (session != NULL && session->ticket != NULL && session->ticketLen > 0)
session->ticket[session->ticketLen - 1] ^= 0xFF;

wolfSSL_free(ssl_c);
ssl_c = NULL;
wolfSSL_free(ssl_s);
ssl_s = NULL;
test_memio_clear_buffer(&test_ctx, 0);
test_memio_clear_buffer(&test_ctx, 1);

ExpectNotNull(ssl_c = wolfSSL_new(ctx_c));
ExpectNotNull(ssl_s = wolfSSL_new(ctx_s));
wolfSSL_SetIOReadCtx(ssl_c, &test_ctx);
wolfSSL_SetIOWriteCtx(ssl_c, &test_ctx);
wolfSSL_SetIOReadCtx(ssl_s, &test_ctx);
wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx);
ExpectIntEQ(wolfSSL_set_session(ssl_c, session), WOLFSSL_SUCCESS);
/* Disable the process-global session cache lookup on the server so that
* the ticket is the only resumption path - with WOLFSSL_TICKET_HAVE_ID
* the server could otherwise resume by session ID. */
if (ssl_s != NULL)
ssl_s->options.sessionCacheOff = 1;

/* Corrupted ticket bytes fail the HMAC check in
* wolfSSL_TicketKeyCb; the session must not resume. */
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
ExpectIntEQ(wolfSSL_session_reused(ssl_c), 0);

wolfSSL_SESSION_free(session);
wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
OpenSSLTicketCleanup();
#endif
return EXPECT_RESULT();
}


#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) && \
!defined(NO_FILESYSTEM) && (!defined(NO_RSA) || defined(HAVE_ECC))
/* Called when writing. */
Expand Down Expand Up @@ -37503,6 +37575,12 @@ TEST_CASE testCases[] = {
TEST_DECL(test_wolfSSL_select_next_proto),
#endif
TEST_DECL(test_tls_ems_downgrade),
TEST_DECL(test_tls_ems_resumption_downgrade),
TEST_DECL(test_tls12_chacha20_poly1305_bad_tag),
TEST_DECL(test_tls13_null_cipher_bad_hmac),
TEST_DECL(test_scr_verify_data_mismatch),
TEST_DECL(test_tls13_hrr_cipher_suite_mismatch),
TEST_DECL(test_tls13_ticket_age_out_of_window),
TEST_DECL(test_wolfSSL_DisableExtendedMasterSecret),
TEST_DECL(test_certificate_authorities_certificate_request),
TEST_DECL(test_certificate_authorities_client_hello),
Expand All @@ -37512,6 +37590,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_wolfSSL_clear_secure_renegotiation),
TEST_DECL(test_wolfSSL_SCR_Reconnect),
TEST_DECL(test_wolfSSL_SCR_check_enabled),
TEST_DECL(test_wolfSSL_ticket_keycb_bad_hmac),
TEST_DECL(test_tls_ext_duplicate),
TEST_DECL(test_tls_ext_word16_overflow),
TEST_DECL(test_tls_bad_legacy_version),
Expand Down
Loading
Loading