Skip to content

Fix buffer overflow bug introduced in PR44 #70

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

Fix buffer overflow bug introduced in PR44 #70

wants to merge 0 commits into from

Conversation

nooj
Copy link
Contributor

@nooj nooj commented Jul 5, 2022 •
edited
Loading

PR #44 introduced a buffer overflow bug that will corrupt the heap if the transmit payload size is increased after any send. For an example of the bug, see below.

The patch included here will cause the existing tx payload buffer to be freed upon any call to MqttClient::setTxPayloadSize(). A subsequent call to MqttClient::write() will allocate space using the new tx payload size (existing behavior). Data previously added to an outgoing message but not sent (via MqttClient::endMessage()) will be lost.

Minimal example:

// make test_string be a string of length > 256 characters
mqttClient.beginMessage("topic");
mqttClient.print(test_string);  // prints first 256 chars of test_string
mqttClient.endMessage();
        
mqttClient.setTxPayloadSize(512);
mqttClient.beginMessage("topic");
mqttClient.print(test_string);  // heap corruption in version 46d65e3!
mqttClient.endMessage();

Output with CORE_DEBUG_LEVEL=5

[V][ssl_client.cpp:295] send_ssl_data(): Writing HTTP request with 17 bytes...
[V][ssl_client.cpp:295] send_ssl_data(): Writing HTTP request with 256 bytes...
CORRUPT HEAP: multi_heap.c:432 detected at 0x3ffd6ab4
abort() was called at PC 0x4008d447 on core 1

@nooj nooj changed the title Fix buffer overflow bug introduced in PR #44 Fix buffer overflow bug introduced in PR44 Jul 5, 2022
@github-actions
Copy link

github-actions bot commented Jul 5, 2022

Memory usage change @ c6a0904

Board flash % RAM for global variables %
arduino:megaavr:uno2018 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkr1000 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkrwifi1010 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:nano_33_iot 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/WiFiAdvancedCallback
flash		 % examples/WiFiAdvancedCallback
RAM for global variables		 % examples/WiFiEcho
flash		 % examples/WiFiEcho
RAM for global variables		 % examples/WiFiEchoCallback
flash		 % examples/WiFiEchoCallback
RAM for global variables		 % examples/WiFiSimpleReceive
flash		 % examples/WiFiSimpleReceive
RAM for global variables		 % examples/WiFiSimpleReceiveCallback
flash		 % examples/WiFiSimpleReceiveCallback
RAM for global variables		 % examples/WiFiSimpleSender
flash		 % examples/WiFiSimpleSender
RAM for global variables		 %
arduino:megaavr:uno2018 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:samd:mkr1000 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:samd:mkrwifi1010 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:samd:nano_33_iot 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
Click for full report CSV 
Board,examples/WiFiAdvancedCallback<br>flash,%,examples/WiFiAdvancedCallback<br>RAM for global variables,%,examples/WiFiEcho<br>flash,%,examples/WiFiEcho<br>RAM for global variables,%,examples/WiFiEchoCallback<br>flash,%,examples/WiFiEchoCallback<br>RAM for global variables,%,examples/WiFiSimpleReceive<br>flash,%,examples/WiFiSimpleReceive<br>RAM for global variables,%,examples/WiFiSimpleReceiveCallback<br>flash,%,examples/WiFiSimpleReceiveCallback<br>RAM for global variables,%,examples/WiFiSimpleSender<br>flash,%,examples/WiFiSimpleSender<br>RAM for global variables,%
arduino:megaavr:uno2018,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:mkr1000,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:mkrwifi1010,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:nano_33_iot,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0

@per1234 per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Jul 5, 2022
@aentinger aentinger closed this Jul 6, 2022
@aentinger aentinger mentioned this pull request Jul 6, 2022
Merged
@per1234 per1234 added the conclusion: duplicate Has already been submitted label Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nooj @aentinger @per1234