Skip to content

Commit 632cde6

Browse files
hvitvedhvitved
authored
Merge pull request #19702 from geoffw0/lifetime
Rust: New query rust/access-after-lifetime-ended
2 parents e3a61f5 + 006f0e8 commit 632cde6

20 files changed

+1363
-70
lines changed

‎rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
1616
ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
1717
ql/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
1818
ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
19+
ql/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql
1920
ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
2021
ql/rust/ql/src/queries/summary/LinesOfCode.ql
2122
ql/rust/ql/src/queries/summary/LinesOfUserCode.ql

‎rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
1515
ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
1616
ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
1717
ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
18+
ql/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql
1819
ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
1920
ql/rust/ql/src/queries/summary/LinesOfCode.ql
2021
ql/rust/ql/src/queries/summary/LinesOfUserCode.ql

‎rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ module Impl {
5959
)
6060
}
6161

62+
/** Gets the block that encloses this node, if any. */
63+
cached
64+
BlockExpr getEnclosingBlock() {
65+
exists(AstNode p | p = this.getParentNode() |
66+
result = p
67+
or
68+
not p instanceof BlockExpr and
69+
result = p.getEnclosingBlock()
70+
)
71+
}
72+
6273
/** Holds if this node is inside a macro expansion. */
6374
predicate isInMacroExpansion() { MacroCallImpl::isInMacroExpansion(_, this) }
6475

‎rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ module Impl {
127127
*/
128128
Name getName() { variableDecl(definingNode, result, text) }
129129

130+
/** Gets the block that encloses this variable, if any. */
131+
BlockExpr getEnclosingBlock() { result = definingNode.getEnclosingBlock() }
132+
130133
/** Gets the `self` parameter that declares this variable, if any. */
131134
SelfParam getSelfParam() { result.getName() = this.getName() }
132135

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Provides classes and predicates for reasoning about accesses to a pointer
3+
* after its lifetime has ended.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.security.AccessInvalidPointerExtensions
9+
private import codeql.rust.internal.Type
10+
private import codeql.rust.internal.TypeInference as TypeInference
11+
12+
/**
13+
* Provides default sources, sinks and barriers for detecting accesses to a
14+
* pointer after its lifetime has ended, as well as extension points for
15+
* adding your own. Note that a particular `(source, sink)` pair must be
16+
* checked with `dereferenceAfterLifetime` to determine if it is a result.
17+
*/
18+
module AccessAfterLifetime {
19+
/**
20+
* A data flow source for accesses to a pointer after its lifetime has ended,
21+
* that is, creation of a pointer or reference.
22+
*/
23+
abstract class Source extends DataFlow::Node {
24+
/**
25+
* Gets the value this pointer or reference points to.
26+
*/
27+
abstract Expr getTarget();
28+
}
29+
30+
/**
31+
* A data flow sink for accesses to a pointer after its lifetime has ended,
32+
* that is, a dereference. We re-use the same sinks as for the accesses to
33+
* invalid pointers query.
34+
*/
35+
class Sink = AccessInvalidPointer::Sink;
36+
37+
/**
38+
* A barrier for accesses to a pointer after its lifetime has ended.
39+
*/
40+
abstract class Barrier extends DataFlow::Node { }
41+
42+
/**
43+
* Holds if the pair `(source, sink)`, that represents a flow from a
44+
* pointer or reference to a dereference, has its dereference outside the
45+
* lifetime of the target variable `target`.
46+
*/
47+
bindingset[source, sink]
48+
predicate dereferenceAfterLifetime(Source source, Sink sink, Variable target) {
49+
exists(BlockExpr valueScope, BlockExpr accessScope |
50+
valueScope(source.getTarget(), target, valueScope) and
51+
accessScope = sink.asExpr().getExpr().getEnclosingBlock() and
52+
not mayEncloseOnStack(valueScope, accessScope)
53+
)
54+
}
55+
56+
/**
57+
* Holds if `var` has scope `scope`.
58+
*/
59+
private predicate variableScope(Variable var, BlockExpr scope) {
60+
// local variable
61+
scope = var.getEnclosingBlock()
62+
or
63+
// parameter
64+
exists(Callable c |
65+
var.getParameter().getEnclosingCallable() = c and
66+
scope.getParentNode() = c
67+
)
68+
}
69+
70+
/**
71+
* Holds if `value` accesses a variable `target` with scope `scope`.
72+
*/
73+
private predicate valueScope(Expr value, Variable target, BlockExpr scope) {
74+
// variable access (to a non-reference)
75+
target = value.(VariableAccess).getVariable() and
76+
variableScope(target, scope) and
77+
not TypeInference::inferType(value) instanceof RefType
78+
or
79+
// field access
80+
valueScope(value.(FieldExpr).getContainer(), target, scope)
81+
}
82+
83+
/**
84+
* Holds if block `a` contains block `b`, in the sense that a stack allocated variable in
85+
* `a` may still be on the stack during execution of `b`. This is interprocedural,
86+
* but is an overapproximation that doesn't accurately track call contexts
87+
* (for example if `f` and `g` both call `b`, then then depending on the
88+
* caller a variable in `f` or `g` may or may-not be on the stack during `b`).
89+
*/
90+
private predicate mayEncloseOnStack(BlockExpr a, BlockExpr b) {
91+
// `b` is a child of `a`
92+
a = b.getEnclosingBlock*()
93+
or
94+
// propagate through function calls
95+
exists(CallExprBase ce |
96+
mayEncloseOnStack(a, ce.getEnclosingBlock()) and
97+
ce.getStaticTarget() = b.getEnclosingCallable()
98+
)
99+
}
100+
101+
/**
102+
* A source that is a `RefExpr`.
103+
*/
104+
private class RefExprSource extends Source {
105+
Expr targetValue;
106+
107+
RefExprSource() { this.asExpr().getExpr().(RefExpr).getExpr() = targetValue }
108+
109+
override Expr getTarget() { result = targetValue }
110+
}
111+
112+
/**
113+
* A barrier for nodes inside closures, as we don't model lifetimes of
114+
* variables through closures properly.
115+
*/
116+
private class ClosureBarrier extends Barrier {
117+
ClosureBarrier() { this.asExpr().getExpr().getEnclosingCallable() instanceof ClosureExpr }
118+
}
119+
}

‎rust/ql/src/change-notes/2025-06-24-access-after-lifetime-ended.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `rust/access-after-lifetime-ended`, for detecting pointer dereferences after the lifetime of the pointed-to object has ended.

‎rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.qhelp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
7+
<p>
8+
Dereferencing a pointer after the lifetime of its target has ended causes undefined behavior. Memory
9+
may be corrupted, causing the program to crash or behave incorrectly, in some cases exposing the program
10+
to potential attacks.
11+
</p>
12+
13+
</overview>
14+
<recommendation>
15+
16+
<p>
17+
When dereferencing a pointer in <code>unsafe</code> code, take care that the pointer is still valid
18+
at the time it is dereferenced. Code may need to be rearranged or changed to extend lifetimes. If
19+
possible, rewrite the code using safe Rust types to avoid this kind of problem altogether.
20+
</p>
21+
22+
</recommendation>
23+
<example>
24+
25+
<p>
26+
In the following example, <code>val</code> is local to <code>get_pointer</code> so its lifetime
27+
ends when that function returns. However, a pointer to <code>val</code> is returned and dereferenced
28+
after that lifetime has ended, causing undefined behavior:
29+
</p>
30+
31+
<sample src="AccessAfterLifetimeBad.rs" />
32+
33+
<p>
34+
One way to fix this is to change the return type of the function from a pointer to a <code>Box</code>,
35+
which ensures that the value it points to remains on the heap for the lifetime of the <code>Box</code>
36+
itself. Note that there is no longer a need for an <code>unsafe</code> block as the code no longer
37+
handles pointers directly:
38+
</p>
39+
40+
<sample src="AccessAfterLifetimeGood.rs" />
41+
42+
</example>
43+
<references>
44+
45+
<li>Rust Documentation: <a href="https://doc.rust-lang.org/reference/behavior-considered-undefined.html#dangling-pointers">Behavior considered undefined &gt;&gt; Dangling pointers</a>.</li>
46+
<li>Rust Documentation: <a href="https://doc.rust-lang.org/std/ptr/index.html#safety">Module ptr - Safety</a>.</li>
47+
<li>Massachusetts Institute of Technology: <a href="https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer">Unsafe Rust - Dereferencing a Raw Pointer</a>.</li>
48+
49+
</references>
50+
</qhelp>

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* @name Access of a pointer after its lifetime has ended
3+
* @description Dereferencing a pointer after the lifetime of its target has ended
4+
* causes undefined behavior and may result in memory corruption.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.8
8+
* @precision medium
9+
* @id rust/access-after-lifetime-ended
10+
* @tags reliability
11+
* security
12+
* external/cwe/cwe-825
13+
*/
14+
15+
import rust
16+
import codeql.rust.dataflow.DataFlow
17+
import codeql.rust.dataflow.TaintTracking
18+
import codeql.rust.security.AccessAfterLifetimeExtensions
19+
import AccessAfterLifetimeFlow::PathGraph
20+
21+
/**
22+
* A data flow configuration for detecting accesses to a pointer after its
23+
* lifetime has ended.
24+
*/
25+
module AccessAfterLifetimeConfig implements DataFlow::ConfigSig {
26+
predicate isSource(DataFlow::Node node) { node instanceof AccessAfterLifetime::Source }
27+
28+
predicate isSink(DataFlow::Node node) { node instanceof AccessAfterLifetime::Sink }
29+
30+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof AccessAfterLifetime::Barrier }
31+
}
32+
33+
module AccessAfterLifetimeFlow = TaintTracking::Global<AccessAfterLifetimeConfig>;
34+
35+
from
36+
AccessAfterLifetimeFlow::PathNode sourceNode, AccessAfterLifetimeFlow::PathNode sinkNode,
37+
Variable target
38+
where
39+
// flow from a pointer or reference to the dereference
40+
AccessAfterLifetimeFlow::flowPath(sourceNode, sinkNode) and
41+
// check that the dereference is outside the lifetime of the target
42+
AccessAfterLifetime::dereferenceAfterLifetime(sourceNode.getNode(), sinkNode.getNode(), target) and
43+
// include only results inside `unsafe` blocks, as other results tend to be false positives
44+
(
45+
sinkNode.getNode().asExpr().getExpr().getEnclosingBlock*().isUnsafe() or
46+
sinkNode.getNode().asExpr().getExpr().getEnclosingCallable().(Function).isUnsafe()
47+
) and
48+
// exclude cases with sources / sinks in macros, since these results are difficult to interpret
49+
not sourceNode.getNode().asExpr().getExpr().isFromMacroExpansion() and
50+
not sinkNode.getNode().asExpr().getExpr().isFromMacroExpansion()
51+
select sinkNode.getNode(), sourceNode, sinkNode,
52+
"Access of a pointer to $@ after its lifetime has ended.", target, target.toString()

‎rust/ql/src/queries/security/CWE-825/AccessAfterLifetimeBad.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
2+
fn get_pointer() -> *const i64 {
3+
let val = 123;
4+
5+
&val
6+
} // lifetime of `val` ends here, the pointer becomes dangling
7+
8+
fn example() {
9+
let ptr = get_pointer();
10+
let dereferenced_ptr;
11+
12+
// ...
13+
14+
unsafe {
15+
dereferenced_ptr = *ptr; // BAD: dereferences `ptr` after the lifetime of `val` has ended
16+
}
17+
18+
// ...
19+
}

‎rust/ql/src/queries/security/CWE-825/AccessAfterLifetimeGood.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
2+
fn get_box() -> Box<i64> {
3+
let val = 123;
4+
5+
Box::new(val) // copies `val` onto the heap, where it remains for the lifetime of the `Box`.
6+
}
7+
8+
fn example() {
9+
let ptr = get_box();
10+
let dereferenced_ptr;
11+
12+
// ...
13+
14+
dereferenced_ptr = *ptr; // GOOD
15+
16+
// ...
17+
}

0 commit comments

Comments
 (0)