Skip to content

Commit be6d0d1

Browse files
geoffw0geoffw0
committed
Rust: Work around data flow source issue.
1 parent c2ee421 commit be6d0d1

3 files changed

Lines changed: 76 additions & 34 deletions

File tree

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import codeql.rust.dataflow.DataFlow
88
private import codeql.rust.dataflow.FlowSource
99
private import codeql.rust.dataflow.FlowSink
1010
private import codeql.rust.Concepts
11+
private import codeql.rust.dataflow.internal.Node
1112

1213
/**
1314
* Provides default sources, sinks and barriers for detecting accesses to
@@ -40,6 +41,20 @@ module AccessInvalidPointer {
4041
ModelsAsDataSource() { sourceNode(this, "pointer-invalidate") }
4142
}
4243

44+
/**
45+
* A pointer invalidation from an argument of a modelled call (this is a workaround).
46+
*/
47+
private class ModelsAsDataArgumentSource extends Source {
48+
ModelsAsDataArgumentSource() {
49+
exists(DataFlow::Node n, CallExpr ce, Expr arg |
50+
sourceNode(n, "pointer-invalidate") and
51+
n.(FlowSummaryNode).getSourceElement() = ce.getFunction() and
52+
arg = ce.getArgList().getAnArg() and
53+
this.asExpr().getExpr() = arg
54+
)
55+
}
56+
}
57+
4358
/**
4459
* A pointer access using the unary `*` operator.
4560
*/

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

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,57 @@
11
#select
2-
| deallocation.rs:96:14:96:15 | p1 | deallocation.rs:89:23:89:40 | ...::dangling | deallocation.rs:96:14:96:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:89:23:89:40 | ...::dangling | invalid |
3-
| deallocation.rs:97:14:97:15 | p2 | deallocation.rs:90:21:90:42 | ...::dangling_mut | deallocation.rs:97:14:97:15 | p2 | This operation dereferences a pointer that may be $@. | deallocation.rs:90:21:90:42 | ...::dangling_mut | invalid |
4-
| deallocation.rs:98:14:98:15 | p3 | deallocation.rs:91:23:91:36 | ...::null | deallocation.rs:98:14:98:15 | p3 | This operation dereferences a pointer that may be $@. | deallocation.rs:91:23:91:36 | ...::null | invalid |
2+
| deallocation.rs:23:13:23:14 | m1 | deallocation.rs:20:23:20:24 | m1 | deallocation.rs:23:13:23:14 | m1 | This operation dereferences a pointer that may be $@. | deallocation.rs:20:23:20:24 | m1 | invalid |
3+
| deallocation.rs:25:12:25:31 | ...::read::<...> | deallocation.rs:20:23:20:24 | m1 | deallocation.rs:25:12:25:31 | ...::read::<...> | This operation dereferences a pointer that may be $@. | deallocation.rs:20:23:20:24 | m1 | invalid |
4+
| deallocation.rs:33:5:33:6 | m1 | deallocation.rs:20:23:20:24 | m1 | deallocation.rs:33:5:33:6 | m1 | This operation dereferences a pointer that may be $@. | deallocation.rs:20:23:20:24 | m1 | invalid |
5+
| deallocation.rs:35:4:35:24 | ...::write::<...> | deallocation.rs:20:23:20:24 | m1 | deallocation.rs:35:4:35:24 | ...::write::<...> | This operation dereferences a pointer that may be $@. | deallocation.rs:20:23:20:24 | m1 | invalid |
6+
| deallocation.rs:97:14:97:15 | p1 | deallocation.rs:90:23:90:40 | ...::dangling | deallocation.rs:97:14:97:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:90:23:90:40 | ...::dangling | invalid |
7+
| deallocation.rs:98:14:98:15 | p2 | deallocation.rs:91:21:91:42 | ...::dangling_mut | deallocation.rs:98:14:98:15 | p2 | This operation dereferences a pointer that may be $@. | deallocation.rs:91:21:91:42 | ...::dangling_mut | invalid |
8+
| deallocation.rs:99:14:99:15 | p3 | deallocation.rs:92:23:92:36 | ...::null | deallocation.rs:99:14:99:15 | p3 | This operation dereferences a pointer that may be $@. | deallocation.rs:92:23:92:36 | ...::null | invalid |
9+
| deallocation.rs:146:14:146:15 | p1 | deallocation.rs:143:27:143:28 | p1 | deallocation.rs:146:14:146:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:143:27:143:28 | p1 | invalid |
510
edges
6-
| deallocation.rs:89:6:89:7 | p1 | deallocation.rs:96:14:96:15 | p1 | provenance | |
7-
| deallocation.rs:89:23:89:40 | ...::dangling | deallocation.rs:89:23:89:42 | ...::dangling(...) | provenance | Src:MaD:1 MaD:1 |
8-
| deallocation.rs:89:23:89:42 | ...::dangling(...) | deallocation.rs:89:6:89:7 | p1 | provenance | |
9-
| deallocation.rs:90:6:90:7 | p2 | deallocation.rs:97:14:97:15 | p2 | provenance | |
10-
| deallocation.rs:90:21:90:42 | ...::dangling_mut | deallocation.rs:90:21:90:44 | ...::dangling_mut(...) | provenance | Src:MaD:2 MaD:2 |
11-
| deallocation.rs:90:21:90:44 | ...::dangling_mut(...) | deallocation.rs:90:6:90:7 | p2 | provenance | |
12-
| deallocation.rs:91:6:91:7 | p3 | deallocation.rs:98:14:98:15 | p3 | provenance | |
13-
| deallocation.rs:91:23:91:36 | ...::null | deallocation.rs:91:23:91:38 | ...::null(...) | provenance | Src:MaD:3 MaD:3 |
14-
| deallocation.rs:91:23:91:38 | ...::null(...) | deallocation.rs:91:6:91:7 | p3 | provenance | |
11+
| deallocation.rs:20:23:20:24 | m1 | deallocation.rs:23:13:23:14 | m1 | provenance | |
12+
| deallocation.rs:20:23:20:24 | m1 | deallocation.rs:25:33:25:34 | m1 | provenance | |
13+
| deallocation.rs:25:33:25:34 | m1 | deallocation.rs:25:12:25:31 | ...::read::<...> | provenance | MaD:1 Sink:MaD:1 |
14+
| deallocation.rs:25:33:25:34 | m1 | deallocation.rs:33:5:33:6 | m1 | provenance | |
15+
| deallocation.rs:25:33:25:34 | m1 | deallocation.rs:33:5:33:6 | m1 | provenance | |
16+
| deallocation.rs:33:5:33:6 | m1 | deallocation.rs:35:26:35:27 | m1 | provenance | |
17+
| deallocation.rs:35:26:35:27 | m1 | deallocation.rs:35:4:35:24 | ...::write::<...> | provenance | MaD:2 Sink:MaD:2 |
18+
| deallocation.rs:90:6:90:7 | p1 | deallocation.rs:97:14:97:15 | p1 | provenance | |
19+
| deallocation.rs:90:23:90:40 | ...::dangling | deallocation.rs:90:23:90:42 | ...::dangling(...) | provenance | Src:MaD:3 MaD:3 |
20+
| deallocation.rs:90:23:90:42 | ...::dangling(...) | deallocation.rs:90:6:90:7 | p1 | provenance | |
21+
| deallocation.rs:91:6:91:7 | p2 | deallocation.rs:98:14:98:15 | p2 | provenance | |
22+
| deallocation.rs:91:21:91:42 | ...::dangling_mut | deallocation.rs:91:21:91:44 | ...::dangling_mut(...) | provenance | Src:MaD:4 MaD:4 |
23+
| deallocation.rs:91:21:91:44 | ...::dangling_mut(...) | deallocation.rs:91:6:91:7 | p2 | provenance | |
24+
| deallocation.rs:92:6:92:7 | p3 | deallocation.rs:99:14:99:15 | p3 | provenance | |
25+
| deallocation.rs:92:23:92:36 | ...::null | deallocation.rs:92:23:92:38 | ...::null(...) | provenance | Src:MaD:5 MaD:5 |
26+
| deallocation.rs:92:23:92:38 | ...::null(...) | deallocation.rs:92:6:92:7 | p3 | provenance | |
27+
| deallocation.rs:143:27:143:28 | p1 | deallocation.rs:146:14:146:15 | p1 | provenance | |
1528
models
16-
| 1 | Source: lang:core; crate::ptr::dangling; pointer-invalidate; ReturnValue |
17-
| 2 | Source: lang:core; crate::ptr::dangling_mut; pointer-invalidate; ReturnValue |
18-
| 3 | Source: lang:core; crate::ptr::null; pointer-invalidate; ReturnValue |
29+
| 1 | Sink: lang:core; crate::ptr::read; pointer-access; Argument[0] |
30+
| 2 | Sink: lang:core; crate::ptr::write; pointer-access; Argument[0] |
31+
| 3 | Source: lang:core; crate::ptr::dangling; pointer-invalidate; ReturnValue |
32+
| 4 | Source: lang:core; crate::ptr::dangling_mut; pointer-invalidate; ReturnValue |
33+
| 5 | Source: lang:core; crate::ptr::null; pointer-invalidate; ReturnValue |
1934
nodes
20-
| deallocation.rs:89:6:89:7 | p1 | semmle.label | p1 |
21-
| deallocation.rs:89:23:89:40 | ...::dangling | semmle.label | ...::dangling |
22-
| deallocation.rs:89:23:89:42 | ...::dangling(...) | semmle.label | ...::dangling(...) |
23-
| deallocation.rs:90:6:90:7 | p2 | semmle.label | p2 |
24-
| deallocation.rs:90:21:90:42 | ...::dangling_mut | semmle.label | ...::dangling_mut |
25-
| deallocation.rs:90:21:90:44 | ...::dangling_mut(...) | semmle.label | ...::dangling_mut(...) |
26-
| deallocation.rs:91:6:91:7 | p3 | semmle.label | p3 |
27-
| deallocation.rs:91:23:91:36 | ...::null | semmle.label | ...::null |
28-
| deallocation.rs:91:23:91:38 | ...::null(...) | semmle.label | ...::null(...) |
29-
| deallocation.rs:96:14:96:15 | p1 | semmle.label | p1 |
30-
| deallocation.rs:97:14:97:15 | p2 | semmle.label | p2 |
31-
| deallocation.rs:98:14:98:15 | p3 | semmle.label | p3 |
35+
| deallocation.rs:20:23:20:24 | m1 | semmle.label | m1 |
36+
| deallocation.rs:23:13:23:14 | m1 | semmle.label | m1 |
37+
| deallocation.rs:25:12:25:31 | ...::read::<...> | semmle.label | ...::read::<...> |
38+
| deallocation.rs:25:33:25:34 | m1 | semmle.label | m1 |
39+
| deallocation.rs:33:5:33:6 | m1 | semmle.label | m1 |
40+
| deallocation.rs:33:5:33:6 | m1 | semmle.label | m1 |
41+
| deallocation.rs:35:4:35:24 | ...::write::<...> | semmle.label | ...::write::<...> |
42+
| deallocation.rs:35:26:35:27 | m1 | semmle.label | m1 |
43+
| deallocation.rs:90:6:90:7 | p1 | semmle.label | p1 |
44+
| deallocation.rs:90:23:90:40 | ...::dangling | semmle.label | ...::dangling |
45+
| deallocation.rs:90:23:90:42 | ...::dangling(...) | semmle.label | ...::dangling(...) |
46+
| deallocation.rs:91:6:91:7 | p2 | semmle.label | p2 |
47+
| deallocation.rs:91:21:91:42 | ...::dangling_mut | semmle.label | ...::dangling_mut |
48+
| deallocation.rs:91:21:91:44 | ...::dangling_mut(...) | semmle.label | ...::dangling_mut(...) |
49+
| deallocation.rs:92:6:92:7 | p3 | semmle.label | p3 |
50+
| deallocation.rs:92:23:92:36 | ...::null | semmle.label | ...::null |
51+
| deallocation.rs:92:23:92:38 | ...::null(...) | semmle.label | ...::null(...) |
52+
| deallocation.rs:97:14:97:15 | p1 | semmle.label | p1 |
53+
| deallocation.rs:98:14:98:15 | p2 | semmle.label | p2 |
54+
| deallocation.rs:99:14:99:15 | p3 | semmle.label | p3 |
55+
| deallocation.rs:143:27:143:28 | p1 | semmle.label | p1 |
56+
| deallocation.rs:146:14:146:15 | p1 | semmle.label | p1 |
3257
subpaths

‎rust/ql/test/query-tests/security/CWE-825/deallocation.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,22 @@ pub fn test_alloc(do_dangerous_writes: bool) {
1717
println!(" v3 = {v3}");
1818
println!(" v4 = {v4}");
1919

20-
std::alloc::dealloc(m1, layout); // m1, m2 are now dangling
20+
std::alloc::dealloc(m1, layout); // $ Source=dealloc
21+
// (m1, m2 are now dangling)
2122

22-
let v5 = *m1; // $ MISSING: Alert
23+
let v5 = *m1; // $ Alert[rust/access-invalid-pointer]=dealloc
2324
let v6 = *m2; // $ MISSING: Alert
24-
let v7 = std::ptr::read::<u8>(m1); // $ MISSING: Alert
25+
let v7 = std::ptr::read::<u8>(m1); // $ Alert[rust/access-invalid-pointer]=dealloc
2526
let v8 = std::ptr::read::<i64>(m2); // $ MISSING: Alert
2627
println!(" v5 = {v5} (!)"); // corrupt in practice
2728
println!(" v6 = {v6} (!)"); // corrupt in practice
2829
println!(" v7 = {v7} (!)"); // corrupt in practice
2930
println!(" v8 = {v8} (!)"); // corrupt in practice
3031

3132
if do_dangerous_writes {
32-
*m1 = 2; // $ MISSING: Alert
33+
*m1 = 2; // $ Alert[rust/access-invalid-pointer]=dealloc
3334
*m2 = 3; // $ MISSING: Alert
34-
std::ptr::write::<u8>(m1, 4); // $ MISSING: Alert
35+
std::ptr::write::<u8>(m1, 4); // $ Alert[rust/access-invalid-pointer]=dealloc
3536
std::ptr::write::<i64>(m2, 5); // $ MISSING: Alert
3637
}
3738
}
@@ -139,9 +140,10 @@ pub fn test_ptr_drop() {
139140
println!(" v1 = {v1}");
140141
println!(" v2 = {v2}");
141142

142-
std::ptr::drop_in_place(p1); // explicitly destructs the pointed-to `m2`
143+
std::ptr::drop_in_place(p1); // $ Source=drop_in_place
144+
// explicitly destructs the pointed-to `m2`
143145

144-
let v3 = (*p1)[0]; // $ MISSING: Alert
146+
let v3 = (*p1)[0]; // $ Alert[rust/access-invalid-pointer]=drop_in_place
145147
let v4 = (*p2)[0]; // $ MISSING: Alert
146148
println!(" v3 = {v3} (!)"); // corrupt in practice
147149
println!(" v4 = {v4} (!)"); // corrupt in practice

0 commit comments

Comments
 (0)