Fix TLS ext bounds checking by embhorn · Pull Request #10220 · wolfSSL/wolfssl

embhorn · 2026-04-14T16:59:36Z

Description

Use an appropriately sized accumulator to track extension sizes while parsing
Check tag size in wc_AesEaxDecryptFinal

Credit for issue report:
"Suryansh Mansharamani, founder of Plainshift AI (plainshift.io)"

Fixes zd21596

Testing

Added test_tls_ext_word16_overflow
Added test case in aes_eax_test

Checklist

added tests
updated/added doxygen
updated appropriate READMEs
Updated manual and documentation

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes size/bounds handling when accumulating TLS extension lengths so oversized extension blocks are rejected (BUFFER_E) instead of silently wrapping 16-bit counters, and adds a regression test for the overflow scenario.

Changes:

Switch extension-length accumulation to a wider type (word32) and add explicit overflow checks in TLSX sizing/writing paths.
Add ECH inner ClientHello length bounds checks to prevent word16 truncation.
Add a regression test that constructs an oversized SessionTicket extension and asserts BUFFER_E.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
wolfssl/internal.h	Exposes several TLSX helpers to tests via `WOLFSSL_TEST_VIS`.
tests/api.c	Adds regression test `test_tls_ext_word16_overflow` and registers it.
src/tls13.c	Adds bounds check before casting ECH inner ClientHello length to `word16`.
src/tls.c	Implements word32 accumulation + overflow detection in TLSX size/write logic and adds ECH expansion length guard.

Comments suppressed due to low confidence (1)

tests/api.c:1

The new safety logic also adds wrap/overflow detection in the write path (TLSX_Write / TLSX_WriteRequest), but the regression test only asserts the sizing path (TLSX_GetRequestSize). To cover the newly added write-side checks, extend this test to also call TLSX_WriteRequest (with a suitably sized output buffer) and assert it returns BUFFER_E for the same oversized extension list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #10220

Scan targets checked: wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #10220

Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize, wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 6
6 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #10220

Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize, wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

tests/api.c:1

The boundary sub-test can become configuration-dependent and fail even when the code is correct: if baseInternal + extHdr >= target, tickSz stays 0, no oversized extension is added, but the test still asserts lenBoundary == 0xFFFF. Consider explicitly asserting baseInternal + extHdr < target (and marking the test failed if not), or skipping this boundary sub-test when the baseline extensions already consume too much space.
wolfcrypt/test/test.c:1
On wc_AesEaxInit failure, the test frees eax but does not call wc_AesEaxFree(eax). If wc_AesEaxInit can partially initialize internal fields that require cleanup on failure paths, this can leak state. Consider calling wc_AesEaxFree(eax) before XFREE(...) in this failure branch as well (the free routine should be safe on partially initialized contexts).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T21:56:26Z

+        }
+    }
+
+    if ((word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {


If an extension writer sets ret != 0, the loop breaks, but the post-loop overflow check can still run and return BUFFER_E, masking the original error. To preserve the real failure cause, gate the total-length check on ret == 0 (or return ret immediately after the loop when ret != 0).

Suggested change

if ((word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {

if (ret == 0 && (word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #10220

Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize, wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

No new issues found in the changed files. ✅

github-actions · 2026-04-15T01:24:34Z

MemBrowse Memory Report

gcc-arm-cortex-m4

FLASH: .text +64 B (+0.0%, 195,275 B / 262,144 B, total: 74% used)

gcc-arm-cortex-m4-tls12

FLASH: .text +64 B (+0.1%, 119,418 B / 262,144 B, total: 46% used)
No memory changes detected for:
gcc-arm-cortex-m4-baremetal- gcc-arm-cortex-m4-min-ecc

Fix TLS ext bounds checking

4bb9883

embhorn self-assigned this Apr 14, 2026

embhorn requested review from Copilot and wolfSSL-Fenrir-bot April 14, 2026 16:59

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Comment thread src/tls.c Outdated

Comment thread wolfssl/internal.h

wolfSSL-Fenrir-bot reviewed Apr 14, 2026

View reviewed changes

Comment thread src/tls.c Outdated

Comment thread src/tls.c

Comment thread src/tls13.c

Comment thread src/tls.c

Comment thread src/tls.c Outdated

embhorn added 2 commits April 14, 2026 12:38

Check tag size in wc_AesEaxDecryptFinal

d9beafb

Fix from review

c60ffbd

embhorn requested a review from wolfSSL-Fenrir-bot April 14, 2026 18:07

Copilot started reviewing on behalf of embhorn April 14, 2026 18:14 View session

wolfSSL-Fenrir-bot reviewed Apr 14, 2026

View reviewed changes

Comment thread wolfcrypt/src/aes.c

Comment thread wolfcrypt/src/aes.c

Comment thread src/tls13.c

Comment thread src/tls.c

Comment thread src/tls.c Outdated

Comment thread src/tls.c

Fix from review

9d29e66

Copilot AI review requested due to automatic review settings April 14, 2026 18:51

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Comment thread src/tls.c Outdated

Comment thread src/tls.c

Copilot started reviewing on behalf of embhorn April 14, 2026 19:21 View session

Fix from review

a2c5197

embhorn requested a review from wolfSSL-Fenrir-bot April 14, 2026 20:12

wolfSSL-Fenrir-bot reviewed Apr 14, 2026

View reviewed changes

Comment thread src/tls13.c

Comment thread src/tls.c

Comment thread src/tls.c

Fix from review

760efdf

embhorn requested review from Copilot and wolfSSL-Fenrir-bot April 14, 2026 21:48

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Copilot started reviewing on behalf of embhorn April 14, 2026 22:00 View session

wolfSSL-Fenrir-bot reviewed Apr 14, 2026

View reviewed changes

Fix from review

92f08a7

	if ((word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {
	if (ret == 0 && (word32)*pOffset + (word32)offset > WOLFSSL_MAX_16BIT) {

Conversation

embhorn commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #10220

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #10220

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #10220

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #10220

Uh oh!

github-actions bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

gcc-arm-cortex-m4

gcc-arm-cortex-m4-tls12

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

embhorn commented Apr 14, 2026 •

edited

Loading

github-actions bot commented Apr 15, 2026 •

edited

Loading