lwIP - A Lightweight TCP/IP stack - Bugs: bug #52345, MQTT buffer length check seems...
You are not allowed to post comments on this tracker with your current authentication level.
bug #52345: MQTT buffer length check seems wrong
Submitter: | David Bourgeois <jaguarondi> | ||
Submitted: | Sun 05 Nov 2017 11:28:13 PM UTC | ||
Category: | apps | Severity: | 3 - Normal |
Item Group: | Crash Error | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | git head |
Jump to the original submission
Fri 16 Feb 2018 10:55:26 AM UTC, comment #10: |
David Bourgeois <jaguarondi> |
Wed 14 Feb 2018 11:08:59 AM UTC, comment #9: Should be fixed in eea95459c9b0f150e9b9ab24e80e27e09c2612e5 |
Simon Goldschmidt <goldsimon> |
Mon 12 Feb 2018 08:56:10 PM UTC, comment #8: After testing this agains test.mosquitto.org, I think the original fix was not too bad... |
Simon Goldschmidt <goldsimon> |
Fri 09 Feb 2018 02:34:26 AM UTC, comment #7: Here's how to trigger the issue and the wireshark capture (file mqtt_bug_52345_capture.pcapng attached)
|
David Bourgeois <jaguarondi> |
Mon 29 Jan 2018 06:04:32 AM UTC, comment #6: David, can you provide a wireshark monitoring of the packets that trigger the issue? |
Dirk Ziegelmeier <dziegel> |
Mon 18 Dec 2017 10:23:37 AM UTC, comment #5: Alright, reopened. |
Simon Goldschmidt <goldsimon> |
Mon 18 Dec 2017 10:13:16 AM UTC, comment #4: Sorry for the late answer. We still found a case where this fix doesn't work. When the packet sent has no payload and a QoS of 0, then the last byte of the packet is actually the last byte of the payload which isn't null terminated. So the test will always fail in this case and there's no way to add the null termination for the callback.
|
David Bourgeois <jaguarondi> |
Sun 17 Dec 2017 09:26:18 PM UTC, comment #3: Let's set this fixed until someone complains. |
Simon Goldschmidt <goldsimon> |
Sun 26 Nov 2017 08:09:10 PM UTC, comment #2: Rewrote the fix, I think your fix was wrong. The problem is that the code tries to parse the variable header part of the publish although there is none in the telegram. |
Dirk Ziegelmeier <dziegel> |
Mon 13 Nov 2017 09:30:50 PM UTC, comment #1: From reading the code, it should IMHO be compared to MQTT_VAR_HEADER_BUFFER_LEN - fixed_hdr_idx.
|
Dirk Ziegelmeier <dziegel> |
Sun 05 Nov 2017 11:28:13 PM UTC, original submission:
The buffer length check fails with an empty payload, it seems the comparison should be done towards MQTT_VAR_HEADER_BUFFER_LEN but I don't know if there are some other length to take into account also.
|
David Bourgeois <jaguarondi> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 10 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2018-02-14 | goldsimon | Status | In Progress | Fixed | |
Assigned to | dziegel | goldsimon | |||
Open/Closed | Open | Closed | |||
2018-02-09 | jaguarondi | Attached File | - | Added mqtt_bug_52345_capture.pcapng.gz, #43210 | |
2017-12-18 | goldsimon | Status | Fixed | In Progress | |
Open/Closed | Closed | Open | |||
2017-12-17 | goldsimon | Status | Ready For Test | Fixed | |
Assigned to | None | dziegel | |||
Open/Closed | Open | Closed | |||
2017-11-13 | dziegel | Status | None | Ready For Test |
Powered by Savane 3.13-d3ae.
Corresponding source code
Fix confirmed on our side too. Thanks