Skip to content

Commit 79f8584

Browse files
committed
Rust: Fix spurious results involving closures.
1 parent bf4ea02 commit 79f8584

File tree

3 files changed

+10
-10
lines changed

3 files changed

+10
-10
lines changed

rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ module AccessAfterLifetime {
4747
exists(BlockExpr valueScope, BlockExpr accessScope |
4848
valueScope(source.getTargetValue(), valueScope) and
4949
accessScope = sink.asExpr().getExpr().getEnclosingBlock() and
50-
not maybeOnStack(valueScope, accessScope)
50+
not maybeOnStack(valueScope, accessScope) and
51+
// exclude results where the access is in a closure, since we don't
52+
// model where a closure is actually called here.
53+
not accessScope.getEnclosingBlock*() = any(ClosureExpr ce).getBody()
5154
)
5255
}
5356

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
| lifetime.rs:433:7:433:8 | p1 | lifetime.rs:383:31:383:37 | &raw mut my_pair | lifetime.rs:433:7:433:8 | p1 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:383:31:383:37 | my_pair | my_pair |
1818
| lifetime.rs:459:13:459:14 | p1 | lifetime.rs:442:17:442:23 | &my_val | lifetime.rs:459:13:459:14 | p1 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:442:18:442:23 | my_val | my_val |
1919
| lifetime.rs:460:13:460:31 | get_ptr_from_ref(...) | lifetime.rs:442:17:442:23 | &my_val | lifetime.rs:460:13:460:31 | get_ptr_from_ref(...) | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:442:18:442:23 | my_val | my_val |
20-
| lifetime.rs:520:14:520:15 | p3 | lifetime.rs:542:26:542:35 | &my_local3 | lifetime.rs:520:14:520:15 | p3 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:542:27:542:35 | my_local3 | my_local3 |
21-
| lifetime.rs:521:14:521:15 | p4 | lifetime.rs:543:4:543:13 | &my_local4 | lifetime.rs:521:14:521:15 | p4 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:543:5:543:13 | my_local4 | my_local4 |
22-
| lifetime.rs:553:14:553:15 | p2 | lifetime.rs:534:3:534:12 | &my_local5 | lifetime.rs:553:14:553:15 | p2 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:534:4:534:12 | my_local5 | my_local5 |
2320
| lifetime.rs:659:15:659:18 | ref1 | lifetime.rs:654:31:654:35 | &str1 | lifetime.rs:659:15:659:18 | ref1 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:654:32:654:35 | str1 | str1 |
2421
| lifetime.rs:667:14:667:17 | ref1 | lifetime.rs:654:31:654:35 | &str1 | lifetime.rs:667:14:667:17 | ref1 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:654:32:654:35 | str1 | str1 |
2522
| lifetime.rs:667:14:667:17 | ref1 | lifetime.rs:655:11:655:25 | &raw const str2 | lifetime.rs:667:14:667:17 | ref1 | Access of a pointer to $@ after it's lifetime has ended. | lifetime.rs:655:22:655:25 | str2 | str2 |

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,8 @@ fn get_closure(p3: *const i64, p4: *const i64) -> impl FnOnce() {
517517
unsafe {
518518
let v1 = *p1; // $ MISSING: Alert[rust/access-after-lifetime-ended]=local1
519519
let v2 = *p2; // GOOD
520-
let v3 = *p3; // $ SPURIOUS: Alert[rust/access-after-lifetime-ended]=local3
521-
let v4 = *p4; // $ Alert[rust/access-after-lifetime-ended]=local4
520+
let v3 = *p3; // GOOD
521+
let v4 = *p4; // $ MISSING: Alert[rust/access-after-lifetime-ended]=local4
522522
println!(" v1 = {v1} (!)"); // corrupt in practice
523523
println!(" v2 = {v2}");
524524
println!(" v3 = {v3}");
@@ -531,16 +531,16 @@ fn with_closure(ptr: *const i64, closure: fn(*const i64, *const i64)) {
531531
let my_local5: i64 = 5;
532532

533533
closure(ptr,
534-
&my_local5); // $ SPURIOUS: Source[rust/access-after-lifetime-ended]=local5
534+
&my_local5);
535535
}
536536

537537
pub fn test_closures() {
538538
let closure;
539539
let my_local3: i64 = 3;
540540
{
541541
let my_local4: i64 = 4;
542-
closure = get_closure( &my_local3, // $ SPURIOUS: Source[rust/access-after-lifetime-ended]=local3
543-
&my_local4); // $ Source[rust/access-after-lifetime-ended]=local4
542+
closure = get_closure( &my_local3,
543+
&my_local4); // $ MISSING: Source[rust/access-after-lifetime-ended]=local4
544544
} // (`my_local4` goes out of scope, so `p4` is dangling)
545545

546546
use_the_stack();
@@ -550,7 +550,7 @@ pub fn test_closures() {
550550
with_closure(&my_local3, |p1, p2| {
551551
unsafe {
552552
let v5 = *p1; // GOOD
553-
let v6 = *p2; // $ SPURIOUS: Alert[rust/access-after-lifetime-ended]=local5
553+
let v6 = *p2; // $ GOOD
554554
println!(" v5 = {v5}");
555555
println!(" v6 = {v6}");
556556
}

0 commit comments

Comments
 (0)