tuanpmt / esp_mqtt

MQTT client library for ESP8266
http://tuanpm.net/post/esp_mqtt/
MIT License
1.14k stars 400 forks source link

buffer corruption because of unterminated data #161

Closed davydnorris closed 5 years ago

davydnorris commented 5 years ago

https://github.com/tuanpmt/esp_mqtt/blob/37cab7cd8a42d51dc9ca448a6eef447ce8ce5b3e/mqtt/mqtt.c#L304

This line copies the character data that is received from the MQTT broker but leaves it unterminated, leading to buffer corruption later. Suggest adding the line

client->mqtt_state.in_buffer[len] = '\0';

after it, which prevents overruns when using the data directly from the buffer.

someburner commented 5 years ago

I'm not sure this is suitable to have in the library itself. There is no requirement that you must be passing strings over MQTT. But I suppose so long as len is not changed this shouldn't affect anything.

davydnorris commented 5 years ago

Exactly. If you're sending string data then the zero termination means you save a copy from the buffer. If you're not, you would be using len to work out how much data you have anyway.