-
Notifications
You must be signed in to change notification settings - Fork 292
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.
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
Buffer overflow in publishPacket #122
Comments
@pakokol Would you want to implement this? If not, I'll implement it in a new pull request. |
@brentru sorry for the late response I didn't got any notification that you answered here on github maybe I have accidentally turned off the notifications. I suppose I could implement this if you prefer me to do it. |
@pakokol No problem, still up for it? |
@brentru sure why not. Just tell me how is the policity in fixing bugs. It is my first time to collaborate in a project on github :) |
I edited MAXBUFFERSIZE to increase it to 1000. Adafruit - A nice way to implement this might be: That would leave things as they are, but allow a user to change the buffer in their own code without editing the header file. |
Fixes adafruit#109 Fixes adafruit#122 After spending a long time pulling my hair to understand why I was hitting a panic when attempting to read from my registered subscriptions, I found out that the subscriptions member of the Adafruit_MQTT instance was corrupted. :( Turns out the memory corruption was caused by my publish call, where the payload I was providing was bigger than the allocated space in the buffer for construction of the packet (see buffer[MAXBUFFERSIZE]). To protect myself from ever making this mistake again, I am proposing a simple logic in publishPacket where instead of silently corrupting memory, the code uses as much payload as it can fit in the available space. By seeing the truncated payload, user can decide whether he/she should 1)break it up into different topics, 2) put the payload on a diet, or 3) increase MAXBUFFERSIZE.
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
I have an issue that might be related? If I publish an outgoing (client to adafruit.io) mqtt message that is >128 characters, it sends ok (as does follow-on outgoing messages). However the next incoming subscriber message causes coredump. My array is 256 characters, fyi. I am seeing coredumps after a certain amount of time with this, which I think is related. 127 character message is fine. Note the length of the topic seems not relevant in my case. |
Yes, that sounds extremely familiar. Try using my fix to see if it helps with the crash. The packet will be Best, -- flaviof |
Thanks! |
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Avoid memory corruption from happening when data payload provided in Adafruit_MQTT::publishPacket is greater than MAXBUFFERSIZE. In order to do that, a helper function is being added to calculate how much space is available for the payload after subtracting what is used as the header. Pull request adafruit#166 Fixes adafruit#109 Fixes adafruit#122 Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
The function uint16_t Adafruit_MQTT::publishPacket has buffer overflow if you send a packet that is larger than the MAXBUFFERSIZE (topic len + data len).
As you can see from the code there isn't any check if we already went past the packet size and there fore we write over the boundaries.
The text was updated successfully, but these errors were encountered: