in_syslog: fix integer overflow in octet-counting length parser#11852
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA single-line boundary condition change in the RFC 6587 octet-counting framing parser adjusts the overflow guard from ChangesSyslog RFC 6587 framing overflow boundary
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@TristanInSec would you please sign off the commits ? (DCO error / git commit -s ...) |
603bf05 to
5268bdb
Compare
Hi @edsiper, sure, it's done. Thanks. |
|
Our commit lint complains like as: So, we need to change from a colon usage to another form without colon in the subsequent commit message lines after the commit title line. |
5268bdb to
b0273d2
Compare
|
Fixed -- replaced the colon with a dash to avoid the commit lint error. Thanks for flagging it. |
|
DCO still complains that: Summary Commit sha: b0273d2, Author: TristanInSec, Committer: TristanInSec; Expected "TristanInSec tristan.mtn@gmail.com", but got "Tristan tristan@talencesecurity.com". |
The overflow guard uses strict greater-than (n > SIZE_MAX / 10) which misses the boundary case where n equals SIZE_MAX / 10 exactly. When n = 1844674407370955161 (SIZE_MAX / 10 on 64-bit), the subsequent n * 10 + digit overflows to a small value (0-5). This sets frame_expected_len to 0, which permanently corrupts the connection -- frame_have_len stays set while frame_expected_len is 0, causing all subsequent messages to be silently discarded. Change the guard to >= so that the boundary value is also clamped to SIZE_MAX before the multiplication. Signed-off-by: Tristan <tristan@talencesecurity.com>
b0273d2 to
aae7a48
Compare
Hi @cosmo0920, |
The overflow guard uses strict greater-than (n > SIZE_MAX / 10) which
misses the boundary case where n equals SIZE_MAX / 10 exactly. When
n = 1844674407370955161 (SIZE_MAX / 10 on 64-bit), the subsequent
n * 10 + digit overflows to a small value (0-5). This sets
frame_expected_len to 0, which permanently corrupts the connection
state: frame_have_len stays set while frame_expected_len is 0,
causing all subsequent messages to be silently discarded.
Change the guard to >= so that the boundary value is also clamped to
SIZE_MAX before the multiplication.
Summary by CodeRabbit