-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Add barrier for split_off #21153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a barrier to the rust/uncontrolled-allocation-size query to prevent false positives for Vec::split_off and String::split_off methods. These methods are recognized as allocation sinks by the query's generated models, but in practice any allocation they perform is bounded by the size of the original collection, making them safe from uncontrolled allocation vulnerabilities.
Changes:
- Added a
ModelledBarrierclass that blocks data flow to the index argument ofsplit_offmethods onVecandString - Added test case demonstrating that
Vec::split_offis correctly identified as safe - Updated expected test results to reflect line number changes from the new test case
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/security/UncontrolledAllocationSizeExtensions.qll | Implements a new barrier for split_off methods on Vec and String types |
| rust/ql/test/query-tests/security/CWE-770/main.rs | Adds test case for Vec::split_off with uncontrolled size parameter |
| rust/ql/test/query-tests/security/CWE-770/UncontrolledAllocationSize.expected | Updates expected results with adjusted line numbers from new test code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rust/ql/lib/codeql/rust/security/UncontrolledAllocationSizeExtensions.qll
Fixed
Show fixed
Hide fixed
paldepind
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Copilot has a comment about spelling that I think is right.
paldepind
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this!
Add a barrier for
<alloc::vec::Vec>::split_offinrust/uncontrolled-allocation-size. This method is recognised as a (generated) sink, however in practice the size of any allocation made bysplit_offis bounded by the size of the original vector - so it's never an "uncontrolled" allocation.Update: I've created an issue for adding support for Rust barriers being defined in models-as-data, and convert this barrier to MaD. This isn't particularly high priority right now but if we end up with a lot of barriers the importance could increase.