Rust: New query rust/access-after-lifetime-ended#19702
Conversation
…mixes (inspired by early real world results).
…erstand whether the alert is correct.
|
QHelp previews: rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.qhelpAccess of a pointer after its lifetime has endedDereferencing a pointer after the lifetime of its target has ended causes undefined behavior. Memory may be corrupted, causing the program to crash or behave incorrectly, in some cases exposing the program to potential attacks. RecommendationWhen dereferencing a pointer in ExampleIn the following example, fn get_pointer() -> *const i64 {
let val = 123;
&val
} // lifetime of `val` ends here, the pointer becomes dangling
fn example() {
let ptr = get_pointer();
let dereferenced_ptr;
// ...
unsafe {
dereferenced_ptr = *ptr; // BAD: dereferences `ptr` after the lifetime of `val` has ended
}
// ...
}One way to fix this is to change the return type of the function from a pointer to a fn get_box() -> Box<i64> {
let val = 123;
Box::new(val) // copies `val` onto the heap, where it remains for the lifetime of the `Box`.
}
fn example() {
let ptr = get_box();
let dereferenced_ptr;
// ...
dereferenced_ptr = *ptr; // GOOD
// ...
}References
|
There was a problem hiding this comment.
Pull Request Overview
Adds a new CodeQL query to detect pointer dereferences after the lifetime of their target has ended, including examples, documentation, and scope‐tracking support.
- Introduce
AccessAfterLifetime.qlwith configuration, flow definition, and filtering logic - Add QL test harness entries (
.qlref), examples (Good.rs/Bad.rs), and documentation (.qhelp) - Extend the internal QL library to track enclosing blocks (
getEnclosingBlock) and support the new query
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/test/query-tests/security/CWE-825/options.yml | Add futures dependency for async tests |
| rust/ql/test/query-tests/security/CWE-825/main.rs | Invoke newly added test cases in main() |
| rust/ql/test/query-tests/security/CWE-825/deallocation.rs | Update existing invalid-pointer test annotations |
| rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.qlref | Register the new query with test harness |
| rust/ql/src/queries/security/CWE-825/AccessAfterLifetimeGood.rs | Add good example using Box |
| rust/ql/src/queries/security/CWE-825/AccessAfterLifetimeBad.rs | Add bad example that returns a dangling pointer |
| rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql | Implement the new query logic |
| rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.qhelp | Add documentation and examples |
| rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll | Define sources, sinks, barriers, and scope checks |
| rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll | Add Variable.getEnclosingBlock() |
| rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll | Add AstNode.getEnclosingBlock() |
Comments suppressed due to low confidence (1)
rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql:44
- The predicate
isFromMacroExpansion()does not exist; you likely meantisInMacroExpansion()to filter out macro expansions.
not sinkNode.getNode().asExpr().getExpr().isFromMacroExpansion()
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
DCA:
|
mchammer01
left a comment
There was a problem hiding this comment.
@geoffw0 - LGTM ✨
Added a couple of very minor comments/suggestions.
Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com>
|
Thanks for the review @mchammer01 , I've accepted both suggestions. :) |
paldepind
left a comment
There was a problem hiding this comment.
Looks really great to me! Lots of true positives.
rust/ql/src/queries/security/CWE-825/AccessAfterLifetimeGood.rs
Outdated
Show resolved
Hide resolved
rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll
Outdated
Show resolved
Hide resolved
Co-authored-by: Simon Friis Vindum <paldepind@github.com>
| @@ -0,0 +1,6 @@ | |||
| readWithoutDef | |||
There was a problem hiding this comment.
I should investigate those inconsistencies.
New query
rust/access-after-lifetime-ended, for detecting pointer dereferences after the lifetime of the pointed-to object has ended. Makes use of some existing tests that were created forrust/access-invalid-pointer(before I realized that the idea for that query needed breaking into two separate queries). Also adds quite a lot of new test cases as well.Note that the query is currently
@precision mediumdue to several remaining false positive results in the tests (and in MRVA results). I made some effort to fix these, but I didn't get them all, I feel it's time to get what we have merged and plan further improvements as follow-up work.Before merging: