Fix issues in TLS Extension size calculations#9824
Fix issues in TLS Extension size calculations#9824JacobBarthelmeh merged 4 commits intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes integer overflow vulnerabilities in TLS extension size calculations that could lead to buffer under-allocation. Three functions were using 16-bit variables to accumulate extension sizes, which could wrap past 65,535 and return incorrectly small values.
Changes:
- Changed size accumulation variables from
word16toword32in three GetSize functions - Added overflow detection that returns 0 when sizes exceed
WOLFSSL_MAX_16BIT - Added test case
test_TLSX_SNI_GetSize_overflowto verify the fix
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/tls.c | Fixed integer overflow in TLSX_SNI_GetSize(), TLSX_CSR_GetSize_ex(), and TLSX_CSR2_GetSize() by changing accumulator types to word32 and adding overflow checks |
| tests/api/test_tls_ext.c | Added comprehensive test that creates overflow conditions and verifies both the fix and that the old implementation was vulnerable |
| tests/api/test_tls_ext.h | Added declaration for new overflow test function |
| tests/api.c | Registered new test case in test suite |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Jenkins retest this please |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Three GetSize functions in src/tls.c use word16 (16-bit) variables to accumulate extension sizes in loops. With enough extensions, the value wraps past 65535 back to a small number, causing under-allocation of buffers.
Fix applied to all three functions:
TLSX_SNI_GetSize()TLSX_CSR_GetSize_ex()TLSX_CSR2_GetSize()Fixes zd21239
Testing
Added test case
test_TLSX_SNI_GetSize_overflowChecklist
Credit for finding issue to: