🛡️ Sentinel: Harden entry parsing and splitting against integer overflows#3053
🛡️ Sentinel: Harden entry parsing and splitting against integer overflows#3053ChanTsune wants to merge 1 commit into
Conversation
- Use `saturating_add` for size and length calculations in `lib/src/entry.rs` and `cli/src/command/core.rs` to prevent panics on 32-bit platforms or malformed archives. - Use `checked_add` for archive part number increments to safely handle extreme splitting scenarios. - Improve chunk boundary checks during entry splitting using safe arithmetic and stable `Option` methods. - Document security learning in `.jules/sentinel.md`. Co-authored-by: ChanTsune <41658782+ChanTsune@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
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 (3)
📝 WalkthroughWalkthroughThis PR hardens arithmetic operations across archive entry parsing, splitting, and writing against integer overflow. Sentinel documentation establishes the hardening approach, then cumulative size calculations in entry parsing, splitting logic, and split archive writing are updated from unchecked addition to saturating and checked variants. ChangesArithmetic Hardening in Archive Entry Parsing and Splitting
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request implements arithmetic hardening to prevent integer overflows during archive entry parsing and splitting by replacing standard arithmetic with saturating and checked operations. These changes affect the CLI core and library entry logic, and a sentinel record has been added to document the vulnerability prevention. Feedback indicates that the is_none_or method should be replaced with map_or to maintain compatibility with Rust versions prior to 1.82.0.
| if total_size | ||
| .checked_add(chunk.bytes_len()) | ||
| .is_none_or(|sum| max_bytes_len < sum) |
There was a problem hiding this comment.
The is_none_or method was stabilized in Rust 1.82.0. To maintain compatibility with older Rust versions (MSRV), consider using map_or(true, ...) which provides the same logic and is compatible with versions prior to 1.82.0.
| if total_size | |
| .checked_add(chunk.bytes_len()) | |
| .is_none_or(|sum| max_bytes_len < sum) | |
| if total_size | |
| .checked_add(chunk.bytes_len()) | |
| .map_or(true, |sum| max_bytes_len < sum) |
🛡️ Sentinel: [HIGH] Harden entry parsing and splitting against integer overflows
+=and.sum()withsaturating_addfor cumulative size calculations inlibpnaand the CLI.checked_addand safe boundary checks inEntryPart::try_split.checked_add(1).cargo test) and linters (clippy,fmt). Verified code changes visually viaread_file. Verified that logic handles boundary conditions safely without wrapping.PR created automatically by Jules for task 3992568315708466104 started by @ChanTsune
Summary by CodeRabbit
Bug Fixes
Documentation