arduino-libraries / ArduinoMqttClient

ArduinoMqttClient Library for Arduino
GNU Lesser General Public License v2.1
186 stars 73 forks source link

Fix buffer overflow bug introduced in PR44 #70

Closed nooj closed 2 years ago

nooj commented 2 years ago

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
github-actions[bot] commented 2 years ago

Memory usage change @ c6a0904cb78aa326ea853cbc985bfdd630a1ae61

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
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 ```