Skip to content

Commit bfcbf51

Browse files
hvitvedhvitved
committed
Rust: Speedup AccessAfterLifetime.ql
Before ``` Pipeline standard for AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf@61cb32j5 was evaluated in 30 iterations totaling 44856ms (delta sizes total: 241646328). 241404616 ~1% {2} r1 = SCAN `AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf#prev_delta` OUTPUT In.1, In.0 7379161442 ~1080% {2} | JOIN WITH `_AstNode::AstNode.getEnclosingBlock/0#5c38e65a_AstNode::AstNode.getEnclosingCallable/0#5a548913_Bloc__#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1 333897324 ~40% {2} | AND NOT `AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf#prev`(FIRST 2) 297961888 ~24% {2} | JOIN WITH `project#AccessAfterLifetimeExtensions::AccessAfterLifetime::sourceValueScope/3#d065ba16#2` ON FIRST 1 OUTPUT Lhs.0, Lhs.1 return r1 ```
1 parent 27874ca commit bfcbf51

File tree

2 files changed

+79
-31
lines changed

2 files changed

+79
-31
lines changed

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

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,83 @@ module AccessAfterLifetime {
4747
valueScope(source.getTarget(), target, scope)
4848
}
4949

50-
/**
51-
* Holds if the pair `(source, sink)`, that represents a flow from a
52-
* pointer or reference to a dereference, has its dereference outside the
53-
* lifetime of the target variable `target`.
54-
*/
55-
bindingset[source, sink]
56-
predicate dereferenceAfterLifetime(Source source, Sink sink, Variable target) {
57-
exists(BlockExpr valueScope, BlockExpr accessScope |
58-
sourceValueScope(source, target, valueScope) and
59-
accessScope = sink.asExpr().getEnclosingBlock() and
60-
not mayEncloseOnStack(valueScope, accessScope)
61-
)
50+
signature predicate dereferenceAfterLifetimeCandSig(DataFlow::Node source, DataFlow::Node sink);
51+
52+
module DereferenceAfterLifetime<dereferenceAfterLifetimeCandSig/2 dereferenceAfterLifetimeCand> {
53+
private newtype TTcNode =
54+
TSource(Source s, Variable target) {
55+
dereferenceAfterLifetimeCand(s, _) and sourceValueScope(s, target, _)
56+
} or
57+
TBlockExpr(BlockExpr be) or
58+
TSink(Sink s) { dereferenceAfterLifetimeCand(_, s) }
59+
60+
class TcNode extends TTcNode {
61+
Source asSource(Variable target) { this = TSource(result, target) }
62+
63+
BlockExpr asBlockExpr() { this = TBlockExpr(result) }
64+
65+
Sink asSink() { this = TSink(result) }
66+
67+
string toString() {
68+
result = this.asSource(_).toString()
69+
or
70+
result = this.asBlockExpr().toString()
71+
or
72+
result = this.asSink().toString()
73+
}
74+
75+
Location getLocation() {
76+
result = this.asSource(_).getLocation()
77+
or
78+
result = this.asBlockExpr().getLocation()
79+
or
80+
result = this.asSink().getLocation()
81+
}
82+
}
83+
84+
pragma[nomagic]
85+
private predicate tcStep(TcNode a, TcNode b) {
86+
// `b` is a child of `a`
87+
exists(Source source, Variable target, BlockExpr be |
88+
source = a.asSource(target) and
89+
be = b.asBlockExpr().getEnclosingBlock*() and
90+
sourceValueScope(source, target, be) and
91+
dereferenceAfterLifetimeCand(source, _)
92+
)
93+
or
94+
// propagate through function calls
95+
exists(Call call |
96+
a.asBlockExpr() = call.getEnclosingBlock() and
97+
call.getARuntimeTarget() = b.asBlockExpr().getEnclosingCallable()
98+
)
99+
or
100+
a.asBlockExpr() = b.asSink().asExpr().getEnclosingBlock()
101+
}
102+
103+
predicate isTcSource(TcNode n) { n instanceof TSource }
104+
105+
predicate isTcSink(TcNode n) { n instanceof TSink }
106+
107+
/**
108+
* Holds if block `a` contains block `b`, in the sense that a stack allocated variable in
109+
* `a` may still be on the stack during execution of `b`. This is interprocedural,
110+
* but is an overapproximation that doesn't accurately track call contexts
111+
* (for example if `f` and `g` both call `b`, then then depending on the
112+
* caller a variable in `f` or `g` may or may-not be on the stack during `b`).
113+
*/
114+
predicate mayEncloseOnStack(TcNode a, TcNode b) =
115+
doublyBoundedFastTC(tcStep/2, isTcSource/1, isTcSink/1)(a, b)
116+
117+
/**
118+
* Holds if the pair `(source, sink)`, that represents a flow from a
119+
* pointer or reference to a dereference, has its dereference outside the
120+
* lifetime of the target variable `target`.
121+
*/
122+
predicate dereferenceAfterLifetime(Source source, Sink sink, Variable target) {
123+
dereferenceAfterLifetimeCand(source, sink) and
124+
sourceValueScope(source, target, _) and
125+
not mayEncloseOnStack(TSource(source, target), TSink(sink))
126+
}
62127
}
63128

64129
/**
@@ -88,24 +153,6 @@ module AccessAfterLifetime {
88153
valueScope(value.(FieldExpr).getContainer(), target, scope)
89154
}
90155

91-
/**
92-
* Holds if block `a` contains block `b`, in the sense that a stack allocated variable in
93-
* `a` may still be on the stack during execution of `b`. This is interprocedural,
94-
* but is an overapproximation that doesn't accurately track call contexts
95-
* (for example if `f` and `g` both call `b`, then then depending on the
96-
* caller a variable in `f` or `g` may or may-not be on the stack during `b`).
97-
*/
98-
private predicate mayEncloseOnStack(BlockExpr a, BlockExpr b) {
99-
// `b` is a child of `a`
100-
a = b.getEnclosingBlock*()
101-
or
102-
// propagate through function calls
103-
exists(Call call |
104-
mayEncloseOnStack(a, call.getEnclosingBlock()) and
105-
call.getARuntimeTarget() = b.getEnclosingCallable()
106-
)
107-
}
108-
109156
/**
110157
* A source that is a `RefExpr`.
111158
*/

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ where
6161
// flow from a pointer or reference to the dereference
6262
AccessAfterLifetimeFlow::flowPath(sourceNode, sinkNode) and
6363
// check that the dereference is outside the lifetime of the target
64-
AccessAfterLifetime::dereferenceAfterLifetime(sourceNode.getNode(), sinkNode.getNode(), target)
64+
AccessAfterLifetime::DereferenceAfterLifetime<AccessAfterLifetimeFlow::flow/2>::dereferenceAfterLifetime(sourceNode
65+
.getNode(), sinkNode.getNode(), target)
6566
select sinkNode.getNode(), sourceNode, sinkNode,
6667
"Access of a pointer to $@ after its lifetime has ended.", target, target.toString()

0 commit comments

Comments
 (0)