Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Rust: Improve rust/access-invalid-pointer #20851

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e0f0305
Rust: Add test cases for rust/access-invalid-pointer based on real wo…
geoffw0 Nov 13, 2025
6c3566a
Rust: It turns out we need test cases for immutable pointers as well.
geoffw0 Nov 17, 2025
d804229
Rust: Add missing model.
geoffw0 Nov 13, 2025
41a6bf0
Rust: Add barrier for null pointer checks to the query.
geoffw0 Nov 13, 2025
7258993
Rust: Clean up the query slightly.
geoffw0 Nov 17, 2025
7c8e44d
Rust: Change note.
geoffw0 Nov 17, 2025
8109613
Rust: Correct + clarify qldoc.
geoffw0 Nov 17, 2025
9db1722
Rust: Accept consistency check changes.
geoffw0 Nov 21, 2025
03fc4cb
Merge remote-tracking branch 'upstream/main' into access-invalid-poin…
geoffw0 Nov 21, 2025
988aca1
Rust: Correct QLDoc comment.
geoffw0 Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ extensions:
- ["core::ptr::dangling", "ReturnValue", "pointer-invalidate", "manual"]
- ["core::ptr::dangling_mut", "ReturnValue", "pointer-invalidate", "manual"]
- ["core::ptr::null", "ReturnValue", "pointer-invalidate", "manual"]
- ["core::ptr::null_mut", "ReturnValue", "pointer-invalidate", "manual"]
- ["v8::primitives::null", "ReturnValue", "pointer-invalidate", "manual"]
- addsTo:
pack: codeql/rust-all
Expand Down
7 changes: 7 additions & 0 deletions rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ private import codeql.rust.dataflow.FlowSource
private import codeql.rust.dataflow.FlowSink
private import codeql.rust.Concepts
private import codeql.rust.dataflow.internal.Node
private import codeql.rust.security.Barriers as Barriers

/**
* Provides default sources, sinks and barriers for detecting accesses to
Expand Down Expand Up @@ -59,4 +60,10 @@ module AccessInvalidPointer {
private class ModelsAsDataSink extends Sink {
ModelsAsDataSink() { sinkNode(this, "pointer-access") }
}

/**
* A barrier for invalid pointer access vulnerabilities for values checked to
* be non-`null`.
*/
private class NotNullCheckBarrier extends Barrier instanceof Barriers::NotNullCheckBarrier { }
}
24 changes: 24 additions & 0 deletions rust/ql/lib/codeql/rust/security/Barriers.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
private import codeql.rust.dataflow.DataFlow
private import codeql.rust.internal.TypeInference as TypeInference
private import codeql.rust.internal.Type
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
private import codeql.rust.controlflow.CfgNodes as CfgNodes
private import codeql.rust.frameworks.stdlib.Builtins as Builtins

/**
Expand Down Expand Up @@ -40,3 +42,25 @@
)
}
}

/**
* Holds if guard expression `g` having result `branch` indicates that the
* sub-expression `node` is not null. For example when `ptr.is_null()` is
* `false`, we have that `ptr` is not null.
*/
private predicate notNullCheck(AstNode g, Expr e, boolean branch) {
exists(MethodCallExpr call |
call.getStaticTarget().getName().getText() = "is_null" and
g = call and
e = call.getReceiver() and
branch = false
)
}

/**
* A node representing a value checked to be non-null. This may be an
* appropriate taint flow barrier for some queries.
*/
class NotNullCheckBarrier extends DataFlow::Node {
NotNullCheckBarrier() { this = DataFlow::BarrierGuard<notNullCheck/3>::getABarrierNode() }
}
4 changes: 4 additions & 0 deletions rust/ql/src/change-notes/2025-17-11-access-invalid-pointer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `rust/access-invalid-pointer` query has been improved with new flow sources and barriers.
8 changes: 5 additions & 3 deletions rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import AccessInvalidPointerFlow::PathGraph
* A data flow configuration for accesses to invalid pointers.
*/
module AccessInvalidPointerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) { node instanceof AccessInvalidPointer::Source }
import AccessInvalidPointer

predicate isSink(DataFlow::Node node) { node instanceof AccessInvalidPointer::Sink }
predicate isSource(DataFlow::Node node) { node instanceof Source }

predicate isBarrier(DataFlow::Node barrier) { barrier instanceof AccessInvalidPointer::Barrier }
predicate isSink(DataFlow::Node node) { node instanceof Sink }

predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }

predicate isBarrierOut(DataFlow::Node node) {
// make sinks barriers so that we only report the closest instance
Expand Down
94 changes: 47 additions & 47 deletions rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,27 @@
| lifetime.rs:808:23:808:25 | ptr | lifetime.rs:798:9:798:12 | &val | lifetime.rs:808:23:808:25 | ptr | Access of a pointer to $@ after its lifetime has ended. | lifetime.rs:796:6:796:8 | val | val |
| main.rs:64:23:64:24 | p2 | main.rs:44:26:44:28 | &b2 | main.rs:64:23:64:24 | p2 | Access of a pointer to $@ after its lifetime has ended. | main.rs:43:13:43:14 | b2 | b2 |
edges
| deallocation.rs:148:6:148:7 | p1 | deallocation.rs:151:14:151:15 | p1 | provenance | |
| deallocation.rs:148:6:148:7 | p1 | deallocation.rs:158:14:158:15 | p1 | provenance | |
| deallocation.rs:148:30:148:38 | &raw const my_buffer | deallocation.rs:148:6:148:7 | p1 | provenance | |
| deallocation.rs:228:28:228:43 | ...: ... | deallocation.rs:230:18:230:20 | ptr | provenance | |
| deallocation.rs:240:27:240:42 | ...: ... | deallocation.rs:248:18:248:20 | ptr | provenance | |
| deallocation.rs:257:7:257:10 | ptr1 | deallocation.rs:260:4:260:7 | ptr1 | provenance | |
| deallocation.rs:257:7:257:10 | ptr1 | deallocation.rs:260:4:260:7 | ptr1 | provenance | |
| deallocation.rs:257:14:257:33 | &raw mut ... | deallocation.rs:257:7:257:10 | ptr1 | provenance | |
| deallocation.rs:258:7:258:10 | ptr2 | deallocation.rs:261:4:261:7 | ptr2 | provenance | |
| deallocation.rs:258:7:258:10 | ptr2 | deallocation.rs:261:4:261:7 | ptr2 | provenance | |
| deallocation.rs:258:14:258:33 | &raw mut ... | deallocation.rs:258:7:258:10 | ptr2 | provenance | |
| deallocation.rs:260:4:260:7 | ptr1 | deallocation.rs:263:27:263:30 | ptr1 | provenance | |
| deallocation.rs:261:4:261:7 | ptr2 | deallocation.rs:265:26:265:29 | ptr2 | provenance | |
| deallocation.rs:263:27:263:30 | ptr1 | deallocation.rs:228:28:228:43 | ...: ... | provenance | |
| deallocation.rs:265:26:265:29 | ptr2 | deallocation.rs:240:27:240:42 | ...: ... | provenance | |
| deallocation.rs:276:6:276:9 | ptr1 | deallocation.rs:279:13:279:16 | ptr1 | provenance | |
| deallocation.rs:276:6:276:9 | ptr1 | deallocation.rs:287:13:287:16 | ptr1 | provenance | |
| deallocation.rs:276:13:276:28 | &raw mut ... | deallocation.rs:276:6:276:9 | ptr1 | provenance | |
| deallocation.rs:295:6:295:9 | ptr2 | deallocation.rs:298:13:298:16 | ptr2 | provenance | |
| deallocation.rs:295:6:295:9 | ptr2 | deallocation.rs:308:13:308:16 | ptr2 | provenance | |
| deallocation.rs:295:13:295:28 | &raw mut ... | deallocation.rs:295:6:295:9 | ptr2 | provenance | |
| deallocation.rs:242:6:242:7 | p1 | deallocation.rs:245:14:245:15 | p1 | provenance | |
| deallocation.rs:242:6:242:7 | p1 | deallocation.rs:252:14:252:15 | p1 | provenance | |
| deallocation.rs:242:30:242:38 | &raw const my_buffer | deallocation.rs:242:6:242:7 | p1 | provenance | |
| deallocation.rs:322:28:322:43 | ...: ... | deallocation.rs:324:18:324:20 | ptr | provenance | |
| deallocation.rs:334:27:334:42 | ...: ... | deallocation.rs:342:18:342:20 | ptr | provenance | |
| deallocation.rs:351:7:351:10 | ptr1 | deallocation.rs:354:4:354:7 | ptr1 | provenance | |
| deallocation.rs:351:7:351:10 | ptr1 | deallocation.rs:354:4:354:7 | ptr1 | provenance | |
| deallocation.rs:351:14:351:33 | &raw mut ... | deallocation.rs:351:7:351:10 | ptr1 | provenance | |
| deallocation.rs:352:7:352:10 | ptr2 | deallocation.rs:355:4:355:7 | ptr2 | provenance | |
| deallocation.rs:352:7:352:10 | ptr2 | deallocation.rs:355:4:355:7 | ptr2 | provenance | |
| deallocation.rs:352:14:352:33 | &raw mut ... | deallocation.rs:352:7:352:10 | ptr2 | provenance | |
| deallocation.rs:354:4:354:7 | ptr1 | deallocation.rs:357:27:357:30 | ptr1 | provenance | |
| deallocation.rs:355:4:355:7 | ptr2 | deallocation.rs:359:26:359:29 | ptr2 | provenance | |
| deallocation.rs:357:27:357:30 | ptr1 | deallocation.rs:322:28:322:43 | ...: ... | provenance | |
| deallocation.rs:359:26:359:29 | ptr2 | deallocation.rs:334:27:334:42 | ...: ... | provenance | |
| deallocation.rs:370:6:370:9 | ptr1 | deallocation.rs:373:13:373:16 | ptr1 | provenance | |
| deallocation.rs:370:6:370:9 | ptr1 | deallocation.rs:381:13:381:16 | ptr1 | provenance | |
| deallocation.rs:370:13:370:28 | &raw mut ... | deallocation.rs:370:6:370:9 | ptr1 | provenance | |
| deallocation.rs:389:6:389:9 | ptr2 | deallocation.rs:392:13:392:16 | ptr2 | provenance | |
| deallocation.rs:389:6:389:9 | ptr2 | deallocation.rs:402:13:402:16 | ptr2 | provenance | |
| deallocation.rs:389:13:389:28 | &raw mut ... | deallocation.rs:389:6:389:9 | ptr2 | provenance | |
| lifetime.rs:21:2:21:18 | return ... | lifetime.rs:54:11:54:30 | get_local_dangling(...) | provenance | |
| lifetime.rs:21:9:21:18 | &my_local1 | lifetime.rs:21:2:21:18 | return ... | provenance | |
| lifetime.rs:27:2:27:22 | return ... | lifetime.rs:55:11:55:34 | get_local_dangling_mut(...) | provenance | |
Expand Down Expand Up @@ -212,32 +212,32 @@ models
| 4 | Summary: <alloc::boxed::Box>::as_ptr; Argument[0].Reference.Reference; ReturnValue.Reference; value |
| 5 | Summary: core::ptr::from_ref; Argument[0]; ReturnValue; value |
nodes
| deallocation.rs:148:6:148:7 | p1 | semmle.label | p1 |
| deallocation.rs:148:30:148:38 | &raw const my_buffer | semmle.label | &raw const my_buffer |
| deallocation.rs:151:14:151:15 | p1 | semmle.label | p1 |
| deallocation.rs:158:14:158:15 | p1 | semmle.label | p1 |
| deallocation.rs:228:28:228:43 | ...: ... | semmle.label | ...: ... |
| deallocation.rs:230:18:230:20 | ptr | semmle.label | ptr |
| deallocation.rs:240:27:240:42 | ...: ... | semmle.label | ...: ... |
| deallocation.rs:248:18:248:20 | ptr | semmle.label | ptr |
| deallocation.rs:257:7:257:10 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:257:14:257:33 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:258:7:258:10 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:258:14:258:33 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:260:4:260:7 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:260:4:260:7 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:261:4:261:7 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:261:4:261:7 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:263:27:263:30 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:265:26:265:29 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:276:6:276:9 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:276:13:276:28 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:279:13:279:16 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:287:13:287:16 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:295:6:295:9 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:295:13:295:28 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:298:13:298:16 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:308:13:308:16 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:242:6:242:7 | p1 | semmle.label | p1 |
| deallocation.rs:242:30:242:38 | &raw const my_buffer | semmle.label | &raw const my_buffer |
| deallocation.rs:245:14:245:15 | p1 | semmle.label | p1 |
| deallocation.rs:252:14:252:15 | p1 | semmle.label | p1 |
| deallocation.rs:322:28:322:43 | ...: ... | semmle.label | ...: ... |
| deallocation.rs:324:18:324:20 | ptr | semmle.label | ptr |
| deallocation.rs:334:27:334:42 | ...: ... | semmle.label | ...: ... |
| deallocation.rs:342:18:342:20 | ptr | semmle.label | ptr |
| deallocation.rs:351:7:351:10 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:351:14:351:33 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:352:7:352:10 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:352:14:352:33 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:354:4:354:7 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:354:4:354:7 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:355:4:355:7 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:355:4:355:7 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:357:27:357:30 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:359:26:359:29 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:370:6:370:9 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:370:13:370:28 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:373:13:373:16 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:381:13:381:16 | ptr1 | semmle.label | ptr1 |
| deallocation.rs:389:6:389:9 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:389:13:389:28 | &raw mut ... | semmle.label | &raw mut ... |
| deallocation.rs:392:13:392:16 | ptr2 | semmle.label | ptr2 |
| deallocation.rs:402:13:402:16 | ptr2 | semmle.label | ptr2 |
| lifetime.rs:21:2:21:18 | return ... | semmle.label | return ... |
| lifetime.rs:21:9:21:18 | &my_local1 | semmle.label | &my_local1 |
| lifetime.rs:27:2:27:22 | return ... | semmle.label | return ... |
Expand Down
Loading