Skip to content

Fix integer overflow - #201

Open
pennam wants to merge 3 commits into
arduino-libraries:masterfrom
pennam:fix-integer-overflow
Open

Fix integer overflow#201
pennam wants to merge 3 commits into
arduino-libraries:masterfrom
pennam:fix-integer-overflow

Conversation

@pennam

Copy link
Copy Markdown

Chunk size is accumulated from hex digits with no bounds check, so a long
enough size overflows iChunkLength (int) - UB, yielding a bogus/negative
length. Guard the accumulation with an INT_MAX pre-check so it can never
overflow.

pennam added 3 commits July 13, 2026 12:08
Chunk size is accumulated from hex digits with no bounds check, so a long
enough size overflows iChunkLength (int) - UB, yielding a bogus/negative
length. Guard the accumulation with an INT_MAX pre-check so it can never
overflow.
read() decremented iChunkLength unconditionally; if it hit 0 mid-chunk it
went negative (UB at INT_MIN). Only decrement while it is still positive.
The old guard computed value*10+digit first, then checked for wrap-around
- but that computation is itself UB and the check may be optimised away.
Use a LONG_MAX pre-check so the overflow never happens.
@github-actions

Copy link
Copy Markdown

Memory usage change @ c762f8d

Boardflash%RAM for global variables%
arduino:samd:mkr1000🔺 0 - +560.0 - +0.020 - 00.0 - 0.0
Click for full report table
Boardexamples/BasicAuthGet
flash
%examples/BasicAuthGet
RAM for global variables
%examples/CustomHeader
flash
%examples/CustomHeader
RAM for global variables
%examples/DweetGet
flash
%examples/DweetGet
RAM for global variables
%examples/DweetPost
flash
%examples/DweetPost
RAM for global variables
%examples/HueBlink
flash
%examples/HueBlink
RAM for global variables
%examples/ParseURL
flash
%examples/ParseURL
RAM for global variables
%examples/PostWithHeaders
flash
%examples/PostWithHeaders
RAM for global variables
%examples/SimpleDelete
flash
%examples/SimpleDelete
RAM for global variables
%examples/SimpleGet
flash
%examples/SimpleGet
RAM for global variables
%examples/SimpleHttpExample
flash
%examples/SimpleHttpExample
RAM for global variables
%examples/SimplePost
flash
%examples/SimplePost
RAM for global variables
%examples/SimplePut
flash
%examples/SimplePut
RAM for global variables
%examples/SimpleWebSocket
flash
%examples/SimpleWebSocket
RAM for global variables
%
arduino:samd:mkr1000560.0200.0560.0200.0560.0200.0560.0200.0560.0200.000.000.0560.0200.0560.0200.0560.0200.0560.0200.0560.0200.0560.0200.0560.0200.0
Click for full report CSV
Board,examples/BasicAuthGet<br>flash,%,examples/BasicAuthGet<br>RAM for global variables,%,examples/CustomHeader<br>flash,%,examples/CustomHeader<br>RAM for global variables,%,examples/DweetGet<br>flash,%,examples/DweetGet<br>RAM for global variables,%,examples/DweetPost<br>flash,%,examples/DweetPost<br>RAM for global variables,%,examples/HueBlink<br>flash,%,examples/HueBlink<br>RAM for global variables,%,examples/ParseURL<br>flash,%,examples/ParseURL<br>RAM for global variables,%,examples/PostWithHeaders<br>flash,%,examples/PostWithHeaders<br>RAM for global variables,%,examples/SimpleDelete<br>flash,%,examples/SimpleDelete<br>RAM for global variables,%,examples/SimpleGet<br>flash,%,examples/SimpleGet<br>RAM for global variables,%,examples/SimpleHttpExample<br>flash,%,examples/SimpleHttpExample<br>RAM for global variables,%,examples/SimplePost<br>flash,%,examples/SimplePost<br>RAM for global variables,%,examples/SimplePut<br>flash,%,examples/SimplePut<br>RAM for global variables,%,examples/SimpleWebSocket<br>flash,%,examples/SimpleWebSocket<br>RAM for global variables,%
arduino:samd:mkr1000,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,0,0.0,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0,56,0.02,0,0.0

Comment threadsrc/HttpClient.cpp

iChunkLength = (iChunkLength * 16) + strtol(digit, NULL, 16);
// Only apply if the value fits in int without overflow.
if (iChunkLength <= ((INT_MAX - digitValue) / 16)) {

@andreagilardoniandreagilardoniJul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is equivalent, but I think it may be easier to read

Suggested change
if (iChunkLength <= ((INT_MAX - digitValue) / 16)) {
if ((iChunkLength * 16) + digitValue <= INT_MAX) {

Comment threadsrc/HttpClient.cpp
iContentLength = _iContentLength;
int digitValue = c - '0';
// Only apply if the value fits in long without overflow.
if (iContentLength <= ((LONG_MAX - digitValue) / 10)) {

@andreagilardoniandreagilardoniJul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is equivalent, but I think it may be easier to read

Suggested change
if (iContentLength <= ((LONG_MAX - digitValue) / 10)) {
if ((iContentLength * 10) + digitValue <= LONG_MAX) {

@per1234per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Jul 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: codeRelated to content of the project itselftype: imperfectionPerceived defect in any part of project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pennam@andreagilardoni@rhpco@per1234