[runtime/utils/buffer] use checked add in offset assertion checks #1406
Conversation
d6a87c0 to
e98ba3d
Compare
e98ba3d to
89afa47
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses overflow vulnerabilities in buffer offset calculations by replacing unchecked addition with checked arithmetic operations. The change prevents potential integer overflow scenarios when calculating buffer end offsets.
- Replaces unchecked addition (
+) withchecked_add()and explicit overflow handling - Adds overflow detection to offset calculation in
extractandmergemethods - Maintains existing panic behavior while making overflow conditions explicit
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
89afa47 to
f67eda2
Compare
|
Will adapt the fuzz tests in |
runtime/src/utils/buffer/tip.rs
Outdated
| /// caller is responsible for continuing to manage the data. | ||
| pub(super) fn merge(&mut self, data: &[u8], offset: u64) -> bool { | ||
| let end_offset = offset + data.len() as u64; | ||
| let end_offset = offset.checked_add(data.len() as u64).expect("overflow"); |
There was a problem hiding this comment.
nit: for both of these cases, can we add more detail into "what overflowed"?
I'd prefer something like "offset overflowed"?
There was a problem hiding this comment.
changed to end_offset overflowed
f67eda2 to
a387401
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1406 +/- ##
==========================================
- Coverage 91.69% 91.69% -0.01%
==========================================
Files 275 275
Lines 69192 69197 +5
==========================================
+ Hits 63447 63451 +4
- Misses 5745 5746 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
addresses #1349