-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Improve rust/access-invalid-pointer #20851
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
base: main
Are you sure you want to change the base?
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 improves the rust/access-invalid-pointer query to detect more invalid pointer access vulnerabilities while reducing false positives. The changes add support for detecting null pointers created via core::ptr::null_mut, introduce a null check barrier to reduce false positives, and include comprehensive test cases covering various conditional scenarios.
- Adds a model for
core::ptr::null_mutas a source of invalid pointers - Implements a null check barrier that recognizes when pointers are validated before use (currently only effective for immutable pointers due to SSA limitations)
- Adds extensive test coverage for conditional null checks and short-circuit evaluation scenarios
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/src/change-notes/2025-17-11-access-invalid-pointer.md | Change note documenting the query improvements (contains date format issue in filename) |
| rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql | Refactored to import AccessInvalidPointer module directly and use simplified references |
| rust/ql/lib/codeql/rust/security/Barriers.qll | Adds NotNullCheckBarrier class to recognize null pointer checks via is_null() method |
| rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll | Integrates the new NotNullCheckBarrier into the AccessInvalidPointer analysis |
| rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml | Adds model for core::ptr::null_mut as a source of invalid pointers |
| rust/ql/test/query-tests/security/CWE-825/main.rs | Adds call to new test function test_ptr_invalid_conditions |
| rust/ql/test/query-tests/security/CWE-825/deallocation.rs | Extensive test cases for null pointer checks in various conditional scenarios |
| rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected | Updated expected results reflecting new detections and barrier behavior |
| rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected | Updated line number references due to new code additions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Looks good to me!
Improve
rust/access-invalid-pointer:core::ptr::null_mut(-> more results).Note that the null check barrier only works for immutable pointers (at present), due to some limitations in the design of barrier guards and SSA (specifically, the call to
DfInput::getARead(def)fromSsa.qllsguardChecksSsaDefisn't getting us what we want). I'll write up an issue for that and I would appreciate anybody's thoughts on how best to fix it.