in_mqtt: fix off-by-one in publish topic length validation#12048
in_mqtt: fix off-by-one in publish topic length validation#12048saddamr3e wants to merge 1 commit into
Conversation
Signed-off-by: Saddam <saddamr3e@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PUBLISH packet parsing in mqtt_handle_publish was adjusted so that conn->buf_pos advances past the 2-byte topic length field before frame_avail is recomputed and hlen is validated against the remaining buffer, aligning the validation window with the subsequent topic pointer. ChangesMQTT PUBLISH Topic Parsing Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing this. On a closer read the topic bytes are only consumed in mqtt_data_append(), which is gated by the |
AddressSanitizer, malformed PUBLISH that fills the connection buffer:
mqtt_handle_publish() compares the 2-byte topic length (hlen) against frame_avail while conn->buf_pos still points at the length field's low byte, so frame_avail counts that byte as space for the topic. The buf_pos++ right after moves onto the topic, so a PUBLISH whose declared topic length equals frame_avail reads one byte past buf_frame_end; when the packet fills the buffer to buffer_size that byte is buf[buffer_size]. Moving the increment above the check makes frame_avail measure the actual topic space. Existing in_mqtt runtime tests still pass.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit