diff --git a/rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml b/rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml index 46eea8f9c4ef..7d1761dd8885 100644 --- a/rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml +++ b/rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml @@ -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 diff --git a/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll b/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll index bddc2d8ee5a9..b8b40ffa2578 100644 --- a/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll @@ -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 @@ -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 { } } diff --git a/rust/ql/lib/codeql/rust/security/Barriers.qll b/rust/ql/lib/codeql/rust/security/Barriers.qll index a55e30aa13e8..fbe3691b4123 100644 --- a/rust/ql/lib/codeql/rust/security/Barriers.qll +++ b/rust/ql/lib/codeql/rust/security/Barriers.qll @@ -7,6 +7,8 @@ import rust 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 /** @@ -40,3 +42,25 @@ class IntegralOrBooleanTypeBarrier extends DataFlow::Node { ) } } + +/** + * Holds if guard expression `g` having result `branch` indicates that the + * sub-expression `e` 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::getABarrierNode() } +} diff --git a/rust/ql/src/change-notes/2025-17-11-access-invalid-pointer.md b/rust/ql/src/change-notes/2025-17-11-access-invalid-pointer.md new file mode 100644 index 000000000000..bc7011dc98a5 --- /dev/null +++ b/rust/ql/src/change-notes/2025-17-11-access-invalid-pointer.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `rust/access-invalid-pointer` query has been improved with new flow sources and barriers. diff --git a/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql b/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql index 5177e1fb0e03..3c10e2b197a4 100644 --- a/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql +++ b/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql @@ -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 diff --git a/rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected b/rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected index 10fa2c751a17..c24c6a728bbf 100644 --- a/rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected +++ b/rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected @@ -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 | | @@ -212,32 +212,32 @@ models | 4 | Summary: ::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 ... | diff --git a/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected b/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected index 1e9bdb3c8bb9..6afe11ad0122 100644 --- a/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected +++ b/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected @@ -13,10 +13,23 @@ | deallocation.rs:130:14:130:15 | p1 | deallocation.rs:123:23:123:40 | ...::dangling | deallocation.rs:130:14:130:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:123:23:123:40 | ...::dangling | invalid | | deallocation.rs:131:14:131:15 | p2 | deallocation.rs:124:21:124:42 | ...::dangling_mut | deallocation.rs:131:14:131:15 | p2 | This operation dereferences a pointer that may be $@. | deallocation.rs:124:21:124:42 | ...::dangling_mut | invalid | | deallocation.rs:132:14:132:15 | p3 | deallocation.rs:125:23:125:36 | ...::null | deallocation.rs:132:14:132:15 | p3 | This operation dereferences a pointer that may be $@. | deallocation.rs:125:23:125:36 | ...::null | invalid | -| deallocation.rs:180:15:180:16 | p1 | deallocation.rs:176:3:176:25 | ...::drop_in_place | deallocation.rs:180:15:180:16 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:176:3:176:25 | ...::drop_in_place | invalid | -| deallocation.rs:180:15:180:16 | p1 | deallocation.rs:176:3:176:25 | ...::drop_in_place | deallocation.rs:180:15:180:16 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:176:3:176:25 | ...::drop_in_place | invalid | -| deallocation.rs:248:18:248:20 | ptr | deallocation.rs:242:3:242:25 | ...::drop_in_place | deallocation.rs:248:18:248:20 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:242:3:242:25 | ...::drop_in_place | invalid | -| deallocation.rs:248:18:248:20 | ptr | deallocation.rs:242:3:242:25 | ...::drop_in_place | deallocation.rs:248:18:248:20 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:242:3:242:25 | ...::drop_in_place | invalid | +| deallocation.rs:163:13:163:15 | ptr | deallocation.rs:159:9:159:26 | ...::null_mut | deallocation.rs:163:13:163:15 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:159:9:159:26 | ...::null_mut | invalid | +| deallocation.rs:166:13:166:15 | ptr | deallocation.rs:159:9:159:26 | ...::null_mut | deallocation.rs:166:13:166:15 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:159:9:159:26 | ...::null_mut | invalid | +| deallocation.rs:175:13:175:15 | ptr | deallocation.rs:171:9:171:26 | ...::null_mut | deallocation.rs:175:13:175:15 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:171:9:171:26 | ...::null_mut | invalid | +| deallocation.rs:178:13:178:15 | ptr | deallocation.rs:171:9:171:26 | ...::null_mut | deallocation.rs:178:13:178:15 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:171:9:171:26 | ...::null_mut | invalid | +| deallocation.rs:186:24:186:26 | ptr | deallocation.rs:183:9:183:26 | ...::null_mut | deallocation.rs:186:24:186:26 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:183:9:183:26 | ...::null_mut | invalid | +| deallocation.rs:190:24:190:26 | ptr | deallocation.rs:183:9:183:26 | ...::null_mut | deallocation.rs:190:24:190:26 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:183:9:183:26 | ...::null_mut | invalid | +| deallocation.rs:194:25:194:27 | ptr | deallocation.rs:183:9:183:26 | ...::null_mut | deallocation.rs:194:25:194:27 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:183:9:183:26 | ...::null_mut | invalid | +| deallocation.rs:202:24:202:26 | ptr | deallocation.rs:183:9:183:26 | ...::null_mut | deallocation.rs:202:24:202:26 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:183:9:183:26 | ...::null_mut | invalid | +| deallocation.rs:202:24:202:26 | ptr | deallocation.rs:199:9:199:26 | ...::null_mut | deallocation.rs:202:24:202:26 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:199:9:199:26 | ...::null_mut | invalid | +| deallocation.rs:210:7:210:9 | ptr | deallocation.rs:183:9:183:26 | ...::null_mut | deallocation.rs:210:7:210:9 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:183:9:183:26 | ...::null_mut | invalid | +| deallocation.rs:210:7:210:9 | ptr | deallocation.rs:199:9:199:26 | ...::null_mut | deallocation.rs:210:7:210:9 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:199:9:199:26 | ...::null_mut | invalid | +| deallocation.rs:210:7:210:9 | ptr | deallocation.rs:207:9:207:26 | ...::null_mut | deallocation.rs:210:7:210:9 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:207:9:207:26 | ...::null_mut | invalid | +| deallocation.rs:226:13:226:21 | const_ptr | deallocation.rs:219:15:219:32 | ...::null_mut | deallocation.rs:226:13:226:21 | const_ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:219:15:219:32 | ...::null_mut | invalid | +| deallocation.rs:274:15:274:16 | p1 | deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:274:15:274:16 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:270:3:270:25 | ...::drop_in_place | invalid | +| deallocation.rs:274:15:274:16 | p1 | deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:274:15:274:16 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:270:3:270:25 | ...::drop_in_place | invalid | +| deallocation.rs:342:18:342:20 | ptr | deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:342:18:342:20 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:336:3:336:25 | ...::drop_in_place | invalid | +| deallocation.rs:342:18:342:20 | ptr | deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:342:18:342:20 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:336:3:336:25 | ...::drop_in_place | invalid | edges | deallocation.rs:20:3:20:21 | ...::dealloc | deallocation.rs:20:23:20:24 | [post] m1 | provenance | Src:MaD:3 MaD:3 | | deallocation.rs:20:23:20:24 | [post] m1 | deallocation.rs:26:15:26:16 | m1 | provenance | | @@ -32,7 +45,7 @@ edges | deallocation.rs:70:23:70:35 | [post] m2 as ... | deallocation.rs:90:7:90:8 | m2 | provenance | | | deallocation.rs:70:23:70:35 | [post] m2 as ... | deallocation.rs:95:33:95:34 | m2 | provenance | | | deallocation.rs:95:33:95:34 | m2 | deallocation.rs:95:5:95:31 | ...::write::<...> | provenance | MaD:2 Sink:MaD:2 | -| deallocation.rs:112:3:112:12 | ...::free | deallocation.rs:112:14:112:40 | [post] my_ptr as ... | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:112:3:112:12 | ...::free | deallocation.rs:112:14:112:40 | [post] my_ptr as ... | provenance | Src:MaD:9 MaD:9 | | deallocation.rs:112:14:112:40 | [post] my_ptr as ... | deallocation.rs:115:13:115:18 | my_ptr | provenance | | | deallocation.rs:123:6:123:7 | p1 | deallocation.rs:130:14:130:15 | p1 | provenance | | | deallocation.rs:123:23:123:40 | ...::dangling | deallocation.rs:123:23:123:42 | ...::dangling(...) | provenance | Src:MaD:4 MaD:4 | @@ -44,12 +57,37 @@ edges | deallocation.rs:125:6:125:7 | p3 | deallocation.rs:132:14:132:15 | p3 | provenance | | | deallocation.rs:125:23:125:36 | ...::null | deallocation.rs:125:23:125:38 | ...::null(...) | provenance | Src:MaD:7 MaD:7 | | deallocation.rs:125:23:125:38 | ...::null(...) | deallocation.rs:125:6:125:7 | p3 | provenance | | -| deallocation.rs:176:3:176:25 | ...::drop_in_place | deallocation.rs:176:27:176:28 | [post] p1 | provenance | Src:MaD:6 MaD:6 | -| deallocation.rs:176:3:176:25 | ...::drop_in_place | deallocation.rs:176:27:176:28 | [post] p1 | provenance | Src:MaD:6 MaD:6 | -| deallocation.rs:176:27:176:28 | [post] p1 | deallocation.rs:180:15:180:16 | p1 | provenance | | -| deallocation.rs:242:3:242:25 | ...::drop_in_place | deallocation.rs:242:27:242:29 | [post] ptr | provenance | Src:MaD:6 MaD:6 | -| deallocation.rs:242:3:242:25 | ...::drop_in_place | deallocation.rs:242:27:242:29 | [post] ptr | provenance | Src:MaD:6 MaD:6 | -| deallocation.rs:242:27:242:29 | [post] ptr | deallocation.rs:248:18:248:20 | ptr | provenance | | +| deallocation.rs:159:3:159:5 | ptr | deallocation.rs:163:13:163:15 | ptr | provenance | | +| deallocation.rs:159:3:159:5 | ptr | deallocation.rs:166:13:166:15 | ptr | provenance | | +| deallocation.rs:159:9:159:26 | ...::null_mut | deallocation.rs:159:9:159:28 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:159:9:159:28 | ...::null_mut(...) | deallocation.rs:159:3:159:5 | ptr | provenance | | +| deallocation.rs:171:3:171:5 | ptr | deallocation.rs:175:13:175:15 | ptr | provenance | | +| deallocation.rs:171:3:171:5 | ptr | deallocation.rs:178:13:178:15 | ptr | provenance | | +| deallocation.rs:171:9:171:26 | ...::null_mut | deallocation.rs:171:9:171:28 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:171:9:171:28 | ...::null_mut(...) | deallocation.rs:171:3:171:5 | ptr | provenance | | +| deallocation.rs:183:3:183:5 | ptr | deallocation.rs:186:24:186:26 | ptr | provenance | | +| deallocation.rs:183:3:183:5 | ptr | deallocation.rs:190:24:190:26 | ptr | provenance | | +| deallocation.rs:183:3:183:5 | ptr | deallocation.rs:194:25:194:27 | ptr | provenance | | +| deallocation.rs:183:3:183:5 | ptr | deallocation.rs:202:24:202:26 | ptr | provenance | | +| deallocation.rs:183:3:183:5 | ptr | deallocation.rs:210:7:210:9 | ptr | provenance | | +| deallocation.rs:183:9:183:26 | ...::null_mut | deallocation.rs:183:9:183:28 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:183:9:183:28 | ...::null_mut(...) | deallocation.rs:183:3:183:5 | ptr | provenance | | +| deallocation.rs:199:3:199:5 | ptr | deallocation.rs:202:24:202:26 | ptr | provenance | | +| deallocation.rs:199:3:199:5 | ptr | deallocation.rs:210:7:210:9 | ptr | provenance | | +| deallocation.rs:199:9:199:26 | ...::null_mut | deallocation.rs:199:9:199:28 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:199:9:199:28 | ...::null_mut(...) | deallocation.rs:199:3:199:5 | ptr | provenance | | +| deallocation.rs:207:3:207:5 | ptr | deallocation.rs:210:7:210:9 | ptr | provenance | | +| deallocation.rs:207:9:207:26 | ...::null_mut | deallocation.rs:207:9:207:28 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:207:9:207:28 | ...::null_mut(...) | deallocation.rs:207:3:207:5 | ptr | provenance | | +| deallocation.rs:219:3:219:11 | const_ptr | deallocation.rs:226:13:226:21 | const_ptr | provenance | | +| deallocation.rs:219:15:219:32 | ...::null_mut | deallocation.rs:219:15:219:34 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 | +| deallocation.rs:219:15:219:34 | ...::null_mut(...) | deallocation.rs:219:3:219:11 | const_ptr | provenance | | +| deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:270:27:270:28 | [post] p1 | provenance | Src:MaD:6 MaD:6 | +| deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:270:27:270:28 | [post] p1 | provenance | Src:MaD:6 MaD:6 | +| deallocation.rs:270:27:270:28 | [post] p1 | deallocation.rs:274:15:274:16 | p1 | provenance | | +| deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:336:27:336:29 | [post] ptr | provenance | Src:MaD:6 MaD:6 | +| deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:336:27:336:29 | [post] ptr | provenance | Src:MaD:6 MaD:6 | +| deallocation.rs:336:27:336:29 | [post] ptr | deallocation.rs:342:18:342:20 | ptr | provenance | | models | 1 | Sink: core::ptr::read; Argument[0]; pointer-access | | 2 | Sink: core::ptr::write; Argument[0]; pointer-access | @@ -58,7 +96,8 @@ models | 5 | Source: core::ptr::dangling_mut; ReturnValue; pointer-invalidate | | 6 | Source: core::ptr::drop_in_place; Argument[0]; pointer-invalidate | | 7 | Source: core::ptr::null; ReturnValue; pointer-invalidate | -| 8 | Source: libc::unix::free; Argument[0]; pointer-invalidate | +| 8 | Source: core::ptr::null_mut; ReturnValue; pointer-invalidate | +| 9 | Source: libc::unix::free; Argument[0]; pointer-invalidate | nodes | deallocation.rs:20:3:20:21 | ...::dealloc | semmle.label | ...::dealloc | | deallocation.rs:20:23:20:24 | [post] m1 | semmle.label | [post] m1 | @@ -92,12 +131,40 @@ nodes | deallocation.rs:130:14:130:15 | p1 | semmle.label | p1 | | deallocation.rs:131:14:131:15 | p2 | semmle.label | p2 | | deallocation.rs:132:14:132:15 | p3 | semmle.label | p3 | -| deallocation.rs:176:3:176:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | -| deallocation.rs:176:3:176:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | -| deallocation.rs:176:27:176:28 | [post] p1 | semmle.label | [post] p1 | -| deallocation.rs:180:15:180:16 | p1 | semmle.label | p1 | -| deallocation.rs:242:3:242:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | -| deallocation.rs:242:3:242:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | -| deallocation.rs:242:27:242:29 | [post] ptr | semmle.label | [post] ptr | -| deallocation.rs:248:18:248:20 | ptr | semmle.label | ptr | +| deallocation.rs:159:3:159:5 | ptr | semmle.label | ptr | +| deallocation.rs:159:9:159:26 | ...::null_mut | semmle.label | ...::null_mut | +| deallocation.rs:159:9:159:28 | ...::null_mut(...) | semmle.label | ...::null_mut(...) | +| deallocation.rs:163:13:163:15 | ptr | semmle.label | ptr | +| deallocation.rs:166:13:166:15 | ptr | semmle.label | ptr | +| deallocation.rs:171:3:171:5 | ptr | semmle.label | ptr | +| deallocation.rs:171:9:171:26 | ...::null_mut | semmle.label | ...::null_mut | +| deallocation.rs:171:9:171:28 | ...::null_mut(...) | semmle.label | ...::null_mut(...) | +| deallocation.rs:175:13:175:15 | ptr | semmle.label | ptr | +| deallocation.rs:178:13:178:15 | ptr | semmle.label | ptr | +| deallocation.rs:183:3:183:5 | ptr | semmle.label | ptr | +| deallocation.rs:183:9:183:26 | ...::null_mut | semmle.label | ...::null_mut | +| deallocation.rs:183:9:183:28 | ...::null_mut(...) | semmle.label | ...::null_mut(...) | +| deallocation.rs:186:24:186:26 | ptr | semmle.label | ptr | +| deallocation.rs:190:24:190:26 | ptr | semmle.label | ptr | +| deallocation.rs:194:25:194:27 | ptr | semmle.label | ptr | +| deallocation.rs:199:3:199:5 | ptr | semmle.label | ptr | +| deallocation.rs:199:9:199:26 | ...::null_mut | semmle.label | ...::null_mut | +| deallocation.rs:199:9:199:28 | ...::null_mut(...) | semmle.label | ...::null_mut(...) | +| deallocation.rs:202:24:202:26 | ptr | semmle.label | ptr | +| deallocation.rs:207:3:207:5 | ptr | semmle.label | ptr | +| deallocation.rs:207:9:207:26 | ...::null_mut | semmle.label | ...::null_mut | +| deallocation.rs:207:9:207:28 | ...::null_mut(...) | semmle.label | ...::null_mut(...) | +| deallocation.rs:210:7:210:9 | ptr | semmle.label | ptr | +| deallocation.rs:219:3:219:11 | const_ptr | semmle.label | const_ptr | +| deallocation.rs:219:15:219:32 | ...::null_mut | semmle.label | ...::null_mut | +| deallocation.rs:219:15:219:34 | ...::null_mut(...) | semmle.label | ...::null_mut(...) | +| deallocation.rs:226:13:226:21 | const_ptr | semmle.label | const_ptr | +| deallocation.rs:270:3:270:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | +| deallocation.rs:270:3:270:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | +| deallocation.rs:270:27:270:28 | [post] p1 | semmle.label | [post] p1 | +| deallocation.rs:274:15:274:16 | p1 | semmle.label | p1 | +| deallocation.rs:336:3:336:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | +| deallocation.rs:336:3:336:25 | ...::drop_in_place | semmle.label | ...::drop_in_place | +| deallocation.rs:336:27:336:29 | [post] ptr | semmle.label | [post] ptr | +| deallocation.rs:342:18:342:20 | ptr | semmle.label | ptr | subpaths diff --git a/rust/ql/test/query-tests/security/CWE-825/CONSISTENCY/PathResolutionConsistency.expected b/rust/ql/test/query-tests/security/CWE-825/CONSISTENCY/PathResolutionConsistency.expected index 81f06adcde33..40e59cf662ab 100644 --- a/rust/ql/test/query-tests/security/CWE-825/CONSISTENCY/PathResolutionConsistency.expected +++ b/rust/ql/test/query-tests/security/CWE-825/CONSISTENCY/PathResolutionConsistency.expected @@ -1,6 +1,14 @@ multipleCallTargets -| deallocation.rs:260:11:260:29 | ...::from(...) | -| deallocation.rs:261:11:261:29 | ...::from(...) | +| deallocation.rs:162:5:162:17 | ptr.is_null() | +| deallocation.rs:174:7:174:19 | ptr.is_null() | +| deallocation.rs:186:5:186:17 | ptr.is_null() | +| deallocation.rs:190:5:190:17 | ptr.is_null() | +| deallocation.rs:194:6:194:18 | ptr.is_null() | +| deallocation.rs:202:5:202:17 | ptr.is_null() | +| deallocation.rs:210:25:210:37 | ptr.is_null() | +| deallocation.rs:225:5:225:23 | const_ptr.is_null() | +| deallocation.rs:354:11:354:29 | ...::from(...) | +| deallocation.rs:355:11:355:29 | ...::from(...) | | lifetime.rs:610:13:610:31 | ...::from(...) | | lifetime.rs:611:13:611:31 | ...::from(...) | | lifetime.rs:628:13:628:31 | ...::from(...) | diff --git a/rust/ql/test/query-tests/security/CWE-825/deallocation.rs b/rust/ql/test/query-tests/security/CWE-825/deallocation.rs index 89ef0470e99d..073d03260b34 100644 --- a/rust/ql/test/query-tests/security/CWE-825/deallocation.rs +++ b/rust/ql/test/query-tests/security/CWE-825/deallocation.rs @@ -137,6 +137,100 @@ pub fn test_ptr_invalid(mode: i32) { } } +struct MyObject { + value: i64 +} + +impl MyObject { + fn is_zero(&self) -> bool { + self.value == 0 + } +} + +pub unsafe fn test_ptr_invalid_conditions(mode: i32) { + let layout = std::alloc::Layout::new::(); + + // --- mutable pointer --- + + let mut ptr = std::alloc::alloc(layout) as *mut MyObject; + (*ptr).value = 0; // good + + if mode == 121 { // (causes a panic below) + ptr = std::ptr::null_mut(); // $ Source[rust/access-invalid-pointer] + } + + if ptr.is_null() { + let v = (*ptr).value; // $ Alert[rust/access-invalid-pointer] + println!(" cond1 v = {v}"); + } else { + let v = (*ptr).value; // $ SPURIOUS: Alert[rust/access-invalid-pointer] good - unreachable with null pointer + println!(" cond2 v = {v}"); + } + + if mode == 122 { // (causes a panic below) + ptr = std::ptr::null_mut(); // $ Source[rust/access-invalid-pointer] + } + + if !(ptr.is_null()) { + let v = (*ptr).value; // $ SPURIOUS: Alert[rust/access-invalid-pointer] good - unreachable with null pointer + println!(" cond3 v = {v}"); + } else { + let v = (*ptr).value; // $ Alert[rust/access-invalid-pointer] + println!(" cond4 v = {v}"); + } + + if mode == 123 { // (causes a panic below) + ptr = std::ptr::null_mut(); // $ Source[rust/access-invalid-pointer] + } + + if ptr.is_null() || (*ptr).value == 0 { // $ SPURIOUS: Alert[rust/access-invalid-pointer] good - deref is protected by short-circuiting + println!(" cond5"); + } + + if ptr.is_null() || (*ptr).is_zero() { // $ SPURIOUS: Alert[rust/access-invalid-pointer] good - deref is protected by short-circuiting + println!(" cond6"); + } + + if !ptr.is_null() || (*ptr).value == 0 { // $ Alert[rust/access-invalid-pointer] + println!(" cond7"); + } + + if mode == 124 { // (causes a panic below) + ptr = std::ptr::null_mut(); // $ Source[rust/access-invalid-pointer] + } + + if ptr.is_null() && (*ptr).is_zero() { // $ Alert[rust/access-invalid-pointer] + println!(" cond8"); + } + + if mode == 125 { // (causes a panic below) + ptr = std::ptr::null_mut(); // $ Source[rust/access-invalid-pointer] + } + + if (*ptr).is_zero() || ptr.is_null() { // $ Alert[rust/access-invalid-pointer] + println!(" cond9"); + } + + // --- immutable pointer --- + + let const_ptr; + + if mode == 126 { // (causes a panic below) + const_ptr = std::ptr::null_mut(); // $ Source[rust/access-invalid-pointer] + } else { + const_ptr = std::alloc::alloc(layout) as *mut MyObject; + (*const_ptr).value = 0; // good + } + + if const_ptr.is_null() { + let v = (*const_ptr).value; // $ Alert[rust/access-invalid-pointer] + println!(" cond10 v = {v}"); + } else { + let v = (*const_ptr).value; // $ good - unreachable with null pointer + println!(" cond11 v = {v}"); + } +} + // --- drop --- struct MyBuffer { diff --git a/rust/ql/test/query-tests/security/CWE-825/main.rs b/rust/ql/test/query-tests/security/CWE-825/main.rs index 8d1f2a421469..d15f595e13c0 100644 --- a/rust/ql/test/query-tests/security/CWE-825/main.rs +++ b/rust/ql/test/query-tests/security/CWE-825/main.rs @@ -126,6 +126,11 @@ fn main() { println!("test_ptr_invalid:"); test_ptr_invalid(mode); + println!("test_ptr_invalid_conditions:"); + unsafe { + test_ptr_invalid_conditions(mode); + } + println!("test_drop:"); test_drop();