Security: bufferToUInt reads beyond buffer length without bounds check#2665
Conversation
…out bounds check `getBinaryNodeChildUInt` calls `bufferToUInt(buff, length)` but never validates that `buff.length >= length`. Since `buff` comes from decoded binary protocol nodes (`getBinaryNodeChildBuffer`), a malformed or truncated message from the WhatsApp server (or a MITM) can cause out-of-bounds reads. In Node.js, reading beyond a Buffer returns `undefined`, producing `NaN` (256 * a + undefined), which silently corrupts downstream logic that relies on these integer fields for protocol handling (e.g., parsing device IDs, version numbers, counters). Affected files: generic-utils.ts Signed-off-by: kumburovicbranko682-boop <295886834+kumburovicbranko682-boop@users.noreply.github.com>
|
Thanks for opening this pull request and contributing to the project! The next step is for the maintainers to review your changes. If everything looks good, it will be approved and merged into the main branch. In the meantime, anyone in the community is encouraged to test this pull request and provide feedback. ✅ How to confirm it worksIf you’ve tested this PR, please comment below with: This helps us speed up the review and merge process. 📦 To test this PR locally:If you encounter any issues or have feedback, feel free to comment as well. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesBinary child integer guard
Sequence Diagram(s)Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
Problem
getBinaryNodeChildUIntcallsbufferToUInt(buff, length)but never validates thatbuff.length >= length. Sincebuffcomes from decoded binary protocol nodes (getBinaryNodeChildBuffer), a malformed or truncated message from the WhatsApp server (or a MITM) can cause out-of-bounds reads. In Node.js, reading beyond a Buffer returnsundefined, producingNaN(256 * a + undefined), which silently corrupts downstream logic that relies on these integer fields for protocol handling (e.g., parsing device IDs, version numbers, counters).Severity:
mediumFile:
src/WABinary/generic-utils.tsSolution
In
getBinaryNodeChildUInt, add a length guard before callingbufferToUInt:Changes
src/WABinary/generic-utils.ts(modified)Testing
Summary by cubic
Add a bounds check in getBinaryNodeChildUInt to ensure the buffer is at least the requested length before calling bufferToUInt. This prevents out-of-bounds reads from malformed or truncated protocol nodes and avoids NaN values that could corrupt parsing state.
Written for commit 9a1caa0. Summary will update on new commits.
Summary by CodeRabbit