Skip to content

Commit 6d268e3

Browse files
committed
confirmed the bug happens
1 parent 5131698 commit 6d268e3

File tree

2 files changed

+33
-0
lines changed

2 files changed

+33
-0
lines changed

src/MqttClient.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ MqttClient::MqttClient(Client* client) :
7070
_keepAliveInterval(60 * 1000L),
7171
_connectionTimeout(30 * 1000L),
7272
_tx_payload_buffer_size(TX_PAYLOAD_BUFFER_SIZE),
73+
_last_mallocd_size(0), // DEBUG NOOJ
7374
_connectError(MQTT_SUCCESS),
7475
_connected(false),
7576
_subscribeQos(0x00),
@@ -666,6 +667,36 @@ size_t MqttClient::write(const uint8_t *buf, size_t size)
666667

667668
if (_txPayloadBuffer == NULL) {
668669
_txPayloadBuffer = (uint8_t*)malloc(_tx_payload_buffer_size);
670+
_last_mallocd_size = _tx_payload_buffer_size; //DEBUG NOOJ
671+
}
672+
673+
if (_txPayloadBufferIndex + size > _last_mallocd_size) {
674+
log_e(
675+
"MqttClient::write() ERROR: BUFFER OVERFLOW: _last_mallocd_size = %d, total bytes being written = %d",
676+
_last_mallocd_size, _txPayloadBufferIndex + size
677+
);
678+
679+
/*
680+
* Working example of buffer overflow bug:
681+
*
682+
// make my_settings > 512 chars
683+
mqttClient.beginMessage(SETTINGS_TOPIC);
684+
mqttClient.print(my_settings); // prints first 256 chars of my_settings
685+
mqttClient.endMessage();
686+
687+
mqttClient.setTxPayloadSize(512);
688+
mqttClient.beginMessage(SETTINGS_TOPIC);
689+
mqttClient.print(my_settings); // heap corruption
690+
mqttClient.endMessage();
691+
692+
// output
693+
[V][ssl_client.cpp:295] send_ssl_data(): Writing HTTP request with 17 bytes...
694+
[V][ssl_client.cpp:295] send_ssl_data(): Writing HTTP request with 256 bytes...
695+
[I][MqttClient.cpp:815] setTxPayloadSize(): MqttClient::setTxPayloadSize(): NOOJ says: _txPayloadBuffer should be freed and NULLed here.
696+
[E][MqttClient.cpp:677] write(): MqttClient::write() ERROR: BUFFER OVERFLOW: _last_mallocd_size = 256, total bytes being written = 512
697+
CORRUPT HEAP: multi_heap.c:432 detected at 0x3ffd6ab4
698+
abort() was called at PC 0x4008d447 on core 1
699+
*/
669700
}
670701

671702
memcpy(&_txPayloadBuffer[_txPayloadBufferIndex], buf, size);
@@ -803,6 +834,7 @@ void MqttClient::setConnectionTimeout(unsigned long timeout)
803834
void MqttClient::setTxPayloadSize(unsigned short size)
804835
{
805836
// NOOJ WAS HERE
837+
log_i("MqttClient::setTxPayloadSize(): NOOJ says: _txPayloadBuffer should be freed and NULLed here.");
806838
_tx_payload_buffer_size = size;
807839
}
808840

src/MqttClient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class MqttClient : public Client {
144144
unsigned long _keepAliveInterval;
145145
unsigned long _connectionTimeout;
146146
unsigned short _tx_payload_buffer_size;
147+
unsigned short _last_mallocd_size;
147148

148149
int _connectError;
149150
bool _connected;

0 commit comments

Comments
 (0)