Skip to content

Commit 1c2d8bb

Browse files
geoffw0geoffw0
authored
Merge pull request #20851 from geoffw0/access-invalid-pointer-fp
Rust: Improve rust/access-invalid-pointer
2 parents daead03 + 988aca1 commit 1c2d8bb

File tree

10 files changed

+284
-72
lines changed

10 files changed

+284
-72
lines changed

‎rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ extensions:
6060
- ["core::ptr::dangling", "ReturnValue", "pointer-invalidate", "manual"]
6161
- ["core::ptr::dangling_mut", "ReturnValue", "pointer-invalidate", "manual"]
6262
- ["core::ptr::null", "ReturnValue", "pointer-invalidate", "manual"]
63+
- ["core::ptr::null_mut", "ReturnValue", "pointer-invalidate", "manual"]
6364
- ["v8::primitives::null", "ReturnValue", "pointer-invalidate", "manual"]
6465
- addsTo:
6566
pack: codeql/rust-all

‎rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import codeql.rust.dataflow.FlowSource
99
private import codeql.rust.dataflow.FlowSink
1010
private import codeql.rust.Concepts
1111
private import codeql.rust.dataflow.internal.Node
12+
private import codeql.rust.security.Barriers as Barriers
1213

1314
/**
1415
* Provides default sources, sinks and barriers for detecting accesses to
@@ -59,4 +60,10 @@ module AccessInvalidPointer {
5960
private class ModelsAsDataSink extends Sink {
6061
ModelsAsDataSink() { sinkNode(this, "pointer-access") }
6162
}
63+
64+
/**
65+
* A barrier for invalid pointer access vulnerabilities for values checked to
66+
* be non-`null`.
67+
*/
68+
private class NotNullCheckBarrier extends Barrier instanceof Barriers::NotNullCheckBarrier { }
6269
}

‎rust/ql/lib/codeql/rust/security/Barriers.qll‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import rust
77
private import codeql.rust.dataflow.DataFlow
88
private import codeql.rust.internal.TypeInference as TypeInference
99
private import codeql.rust.internal.Type
10+
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
11+
private import codeql.rust.controlflow.CfgNodes as CfgNodes
1012
private import codeql.rust.frameworks.stdlib.Builtins as Builtins
1113

1214
/**
@@ -40,3 +42,25 @@ class IntegralOrBooleanTypeBarrier extends DataFlow::Node {
4042
)
4143
}
4244
}
45+
46+
/**
47+
* Holds if guard expression `g` having result `branch` indicates that the
48+
* sub-expression `e` is not null. For example when `ptr.is_null()` is
49+
* `false`, we have that `ptr` is not null.
50+
*/
51+
private predicate notNullCheck(AstNode g, Expr e, boolean branch) {
52+
exists(MethodCallExpr call |
53+
call.getStaticTarget().getName().getText() = "is_null" and
54+
g = call and
55+
e = call.getReceiver() and
56+
branch = false
57+
)
58+
}
59+
60+
/**
61+
* A node representing a value checked to be non-null. This may be an
62+
* appropriate taint flow barrier for some queries.
63+
*/
64+
class NotNullCheckBarrier extends DataFlow::Node {
65+
NotNullCheckBarrier() { this = DataFlow::BarrierGuard<notNullCheck/3>::getABarrierNode() }
66+
}

‎rust/ql/src/change-notes/2025-17-11-access-invalid-pointer.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `rust/access-invalid-pointer` query has been improved with new flow sources and barriers.

‎rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ import AccessInvalidPointerFlow::PathGraph
2222
* A data flow configuration for accesses to invalid pointers.
2323
*/
2424
module AccessInvalidPointerConfig implements DataFlow::ConfigSig {
25-
predicate isSource(DataFlow::Node node) { node instanceof AccessInvalidPointer::Source }
25+
import AccessInvalidPointer
2626

27-
predicate isSink(DataFlow::Node node) { node instanceof AccessInvalidPointer::Sink }
27+
predicate isSource(DataFlow::Node node) { node instanceof Source }
2828

29-
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof AccessInvalidPointer::Barrier }
29+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
30+
31+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
3032

3133
predicate isBarrierOut(DataFlow::Node node) {
3234
// make sinks barriers so that we only report the closest instance

‎rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected‎

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,27 @@
2424
| 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 |
2525
| 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 |
2626
edges
27-
| deallocation.rs:148:6:148:7 | p1 | deallocation.rs:151:14:151:15 | p1 | provenance | |
28-
| deallocation.rs:148:6:148:7 | p1 | deallocation.rs:158:14:158:15 | p1 | provenance | |
29-
| deallocation.rs:148:30:148:38 | &raw const my_buffer | deallocation.rs:148:6:148:7 | p1 | provenance | |
30-
| deallocation.rs:228:28:228:43 | ...: ... | deallocation.rs:230:18:230:20 | ptr | provenance | |
31-
| deallocation.rs:240:27:240:42 | ...: ... | deallocation.rs:248:18:248:20 | ptr | provenance | |
32-
| deallocation.rs:257:7:257:10 | ptr1 | deallocation.rs:260:4:260:7 | ptr1 | provenance | |
33-
| deallocation.rs:257:7:257:10 | ptr1 | deallocation.rs:260:4:260:7 | ptr1 | provenance | |
34-
| deallocation.rs:257:14:257:33 | &raw mut ... | deallocation.rs:257:7:257:10 | ptr1 | provenance | |
35-
| deallocation.rs:258:7:258:10 | ptr2 | deallocation.rs:261:4:261:7 | ptr2 | provenance | |
36-
| deallocation.rs:258:7:258:10 | ptr2 | deallocation.rs:261:4:261:7 | ptr2 | provenance | |
37-
| deallocation.rs:258:14:258:33 | &raw mut ... | deallocation.rs:258:7:258:10 | ptr2 | provenance | |
38-
| deallocation.rs:260:4:260:7 | ptr1 | deallocation.rs:263:27:263:30 | ptr1 | provenance | |
39-
| deallocation.rs:261:4:261:7 | ptr2 | deallocation.rs:265:26:265:29 | ptr2 | provenance | |
40-
| deallocation.rs:263:27:263:30 | ptr1 | deallocation.rs:228:28:228:43 | ...: ... | provenance | |
41-
| deallocation.rs:265:26:265:29 | ptr2 | deallocation.rs:240:27:240:42 | ...: ... | provenance | |
42-
| deallocation.rs:276:6:276:9 | ptr1 | deallocation.rs:279:13:279:16 | ptr1 | provenance | |
43-
| deallocation.rs:276:6:276:9 | ptr1 | deallocation.rs:287:13:287:16 | ptr1 | provenance | |
44-
| deallocation.rs:276:13:276:28 | &raw mut ... | deallocation.rs:276:6:276:9 | ptr1 | provenance | |
45-
| deallocation.rs:295:6:295:9 | ptr2 | deallocation.rs:298:13:298:16 | ptr2 | provenance | |
46-
| deallocation.rs:295:6:295:9 | ptr2 | deallocation.rs:308:13:308:16 | ptr2 | provenance | |
47-
| deallocation.rs:295:13:295:28 | &raw mut ... | deallocation.rs:295:6:295:9 | ptr2 | provenance | |
27+
| deallocation.rs:242:6:242:7 | p1 | deallocation.rs:245:14:245:15 | p1 | provenance | |
28+
| deallocation.rs:242:6:242:7 | p1 | deallocation.rs:252:14:252:15 | p1 | provenance | |
29+
| deallocation.rs:242:30:242:38 | &raw const my_buffer | deallocation.rs:242:6:242:7 | p1 | provenance | |
30+
| deallocation.rs:322:28:322:43 | ...: ... | deallocation.rs:324:18:324:20 | ptr | provenance | |
31+
| deallocation.rs:334:27:334:42 | ...: ... | deallocation.rs:342:18:342:20 | ptr | provenance | |
32+
| deallocation.rs:351:7:351:10 | ptr1 | deallocation.rs:354:4:354:7 | ptr1 | provenance | |
33+
| deallocation.rs:351:7:351:10 | ptr1 | deallocation.rs:354:4:354:7 | ptr1 | provenance | |
34+
| deallocation.rs:351:14:351:33 | &raw mut ... | deallocation.rs:351:7:351:10 | ptr1 | provenance | |
35+
| deallocation.rs:352:7:352:10 | ptr2 | deallocation.rs:355:4:355:7 | ptr2 | provenance | |
36+
| deallocation.rs:352:7:352:10 | ptr2 | deallocation.rs:355:4:355:7 | ptr2 | provenance | |
37+
| deallocation.rs:352:14:352:33 | &raw mut ... | deallocation.rs:352:7:352:10 | ptr2 | provenance | |
38+
| deallocation.rs:354:4:354:7 | ptr1 | deallocation.rs:357:27:357:30 | ptr1 | provenance | |
39+
| deallocation.rs:355:4:355:7 | ptr2 | deallocation.rs:359:26:359:29 | ptr2 | provenance | |
40+
| deallocation.rs:357:27:357:30 | ptr1 | deallocation.rs:322:28:322:43 | ...: ... | provenance | |
41+
| deallocation.rs:359:26:359:29 | ptr2 | deallocation.rs:334:27:334:42 | ...: ... | provenance | |
42+
| deallocation.rs:370:6:370:9 | ptr1 | deallocation.rs:373:13:373:16 | ptr1 | provenance | |
43+
| deallocation.rs:370:6:370:9 | ptr1 | deallocation.rs:381:13:381:16 | ptr1 | provenance | |
44+
| deallocation.rs:370:13:370:28 | &raw mut ... | deallocation.rs:370:6:370:9 | ptr1 | provenance | |
45+
| deallocation.rs:389:6:389:9 | ptr2 | deallocation.rs:392:13:392:16 | ptr2 | provenance | |
46+
| deallocation.rs:389:6:389:9 | ptr2 | deallocation.rs:402:13:402:16 | ptr2 | provenance | |
47+
| deallocation.rs:389:13:389:28 | &raw mut ... | deallocation.rs:389:6:389:9 | ptr2 | provenance | |
4848
| lifetime.rs:21:2:21:18 | return ... | lifetime.rs:54:11:54:30 | get_local_dangling(...) | provenance | |
4949
| lifetime.rs:21:9:21:18 | &my_local1 | lifetime.rs:21:2:21:18 | return ... | provenance | |
5050
| lifetime.rs:27:2:27:22 | return ... | lifetime.rs:55:11:55:34 | get_local_dangling_mut(...) | provenance | |
@@ -212,32 +212,32 @@ models
212212
| 4 | Summary: <alloc::boxed::Box>::as_ptr; Argument[0].Reference.Reference; ReturnValue.Reference; value |
213213
| 5 | Summary: core::ptr::from_ref; Argument[0]; ReturnValue; value |
214214
nodes
215-
| deallocation.rs:148:6:148:7 | p1 | semmle.label | p1 |
216-
| deallocation.rs:148:30:148:38 | &raw const my_buffer | semmle.label | &raw const my_buffer |
217-
| deallocation.rs:151:14:151:15 | p1 | semmle.label | p1 |
218-
| deallocation.rs:158:14:158:15 | p1 | semmle.label | p1 |
219-
| deallocation.rs:228:28:228:43 | ...: ... | semmle.label | ...: ... |
220-
| deallocation.rs:230:18:230:20 | ptr | semmle.label | ptr |
221-
| deallocation.rs:240:27:240:42 | ...: ... | semmle.label | ...: ... |
222-
| deallocation.rs:248:18:248:20 | ptr | semmle.label | ptr |
223-
| deallocation.rs:257:7:257:10 | ptr1 | semmle.label | ptr1 |
224-
| deallocation.rs:257:14:257:33 | &raw mut ... | semmle.label | &raw mut ... |
225-
| deallocation.rs:258:7:258:10 | ptr2 | semmle.label | ptr2 |
226-
| deallocation.rs:258:14:258:33 | &raw mut ... | semmle.label | &raw mut ... |
227-
| deallocation.rs:260:4:260:7 | ptr1 | semmle.label | ptr1 |
228-
| deallocation.rs:260:4:260:7 | ptr1 | semmle.label | ptr1 |
229-
| deallocation.rs:261:4:261:7 | ptr2 | semmle.label | ptr2 |
230-
| deallocation.rs:261:4:261:7 | ptr2 | semmle.label | ptr2 |
231-
| deallocation.rs:263:27:263:30 | ptr1 | semmle.label | ptr1 |
232-
| deallocation.rs:265:26:265:29 | ptr2 | semmle.label | ptr2 |
233-
| deallocation.rs:276:6:276:9 | ptr1 | semmle.label | ptr1 |
234-
| deallocation.rs:276:13:276:28 | &raw mut ... | semmle.label | &raw mut ... |
235-
| deallocation.rs:279:13:279:16 | ptr1 | semmle.label | ptr1 |
236-
| deallocation.rs:287:13:287:16 | ptr1 | semmle.label | ptr1 |
237-
| deallocation.rs:295:6:295:9 | ptr2 | semmle.label | ptr2 |
238-
| deallocation.rs:295:13:295:28 | &raw mut ... | semmle.label | &raw mut ... |
239-
| deallocation.rs:298:13:298:16 | ptr2 | semmle.label | ptr2 |
240-
| deallocation.rs:308:13:308:16 | ptr2 | semmle.label | ptr2 |
215+
| deallocation.rs:242:6:242:7 | p1 | semmle.label | p1 |
216+
| deallocation.rs:242:30:242:38 | &raw const my_buffer | semmle.label | &raw const my_buffer |
217+
| deallocation.rs:245:14:245:15 | p1 | semmle.label | p1 |
218+
| deallocation.rs:252:14:252:15 | p1 | semmle.label | p1 |
219+
| deallocation.rs:322:28:322:43 | ...: ... | semmle.label | ...: ... |
220+
| deallocation.rs:324:18:324:20 | ptr | semmle.label | ptr |
221+
| deallocation.rs:334:27:334:42 | ...: ... | semmle.label | ...: ... |
222+
| deallocation.rs:342:18:342:20 | ptr | semmle.label | ptr |
223+
| deallocation.rs:351:7:351:10 | ptr1 | semmle.label | ptr1 |
224+
| deallocation.rs:351:14:351:33 | &raw mut ... | semmle.label | &raw mut ... |
225+
| deallocation.rs:352:7:352:10 | ptr2 | semmle.label | ptr2 |
226+
| deallocation.rs:352:14:352:33 | &raw mut ... | semmle.label | &raw mut ... |
227+
| deallocation.rs:354:4:354:7 | ptr1 | semmle.label | ptr1 |
228+
| deallocation.rs:354:4:354:7 | ptr1 | semmle.label | ptr1 |
229+
| deallocation.rs:355:4:355:7 | ptr2 | semmle.label | ptr2 |
230+
| deallocation.rs:355:4:355:7 | ptr2 | semmle.label | ptr2 |
231+
| deallocation.rs:357:27:357:30 | ptr1 | semmle.label | ptr1 |
232+
| deallocation.rs:359:26:359:29 | ptr2 | semmle.label | ptr2 |
233+
| deallocation.rs:370:6:370:9 | ptr1 | semmle.label | ptr1 |
234+
| deallocation.rs:370:13:370:28 | &raw mut ... | semmle.label | &raw mut ... |
235+
| deallocation.rs:373:13:373:16 | ptr1 | semmle.label | ptr1 |
236+
| deallocation.rs:381:13:381:16 | ptr1 | semmle.label | ptr1 |
237+
| deallocation.rs:389:6:389:9 | ptr2 | semmle.label | ptr2 |
238+
| deallocation.rs:389:13:389:28 | &raw mut ... | semmle.label | &raw mut ... |
239+
| deallocation.rs:392:13:392:16 | ptr2 | semmle.label | ptr2 |
240+
| deallocation.rs:402:13:402:16 | ptr2 | semmle.label | ptr2 |
241241
| lifetime.rs:21:2:21:18 | return ... | semmle.label | return ... |
242242
| lifetime.rs:21:9:21:18 | &my_local1 | semmle.label | &my_local1 |
243243
| lifetime.rs:27:2:27:22 | return ... | semmle.label | return ... |

0 commit comments

Comments
 (0)