Skip to content

Commit 34024ad

Browse files
hvitvedhvitved
committed
Rust: Track closure types in data flow
1 parent 2f964f8 commit 34024ad

File tree

4 files changed

+109
-26
lines changed

4 files changed

+109
-26
lines changed

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll‎

Lines changed: 101 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,7 @@ module LocalFlow {
193193
}
194194

195195
pragma[nomagic]
196-
predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
197-
nodeFrom.asExpr() = getALastEvalNode(nodeTo.asExpr())
198-
or
196+
predicate localMustFlowStep(Node nodeFrom, Node nodeTo) {
199197
// An edge from the right-hand side of a let statement to the left-hand side.
200198
exists(LetStmt s |
201199
nodeFrom.asExpr() = s.getInitializer() and
@@ -238,6 +236,15 @@ module LocalFlow {
238236
nodeTo.asPat() = match.getAnArm().getPat()
239237
)
240238
or
239+
nodeFrom.asExpr() = nodeTo.asExpr().(ParenExpr).getExpr()
240+
}
241+
242+
pragma[nomagic]
243+
predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
244+
localMustFlowStep(nodeFrom, nodeTo)
245+
or
246+
nodeFrom.asExpr() = getALastEvalNode(nodeTo.asExpr())
247+
or
241248
nodeFrom.asPat().(OrPat).getAPat() = nodeTo.asPat()
242249
or
243250
nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr() =
@@ -263,10 +270,84 @@ predicate lambdaCallExpr(CallExprImpl::DynamicCallExpr call, LambdaCallKind kind
263270
exists(kind)
264271
}
265272

273+
// NOTE: We do not yet track type information, except for closures where
274+
// we use the closure itself to represent the unique type.
275+
final class DataFlowType extends TDataFlowType {
276+
Expr asClosureExpr() { this = TClosureExprType(result) }
277+
278+
predicate isUnknownType() { this = TUnknownType() }
279+
280+
predicate isSourceContextParameterType() { this = TSourceContextParameterType() }
281+
282+
string toString() {
283+
exists(this.asClosureExpr()) and
284+
result = "... => .."
285+
or
286+
this.isUnknownType() and
287+
result = ""
288+
or
289+
this.isSourceContextParameterType() and
290+
result = "<source context parameter type>"
291+
}
292+
}
293+
294+
pragma[nomagic]
295+
private predicate compatibleTypesSourceContextParameterTypeLeft(DataFlowType t1, DataFlowType t2) {
296+
t1.isSourceContextParameterType() and not exists(t2.asClosureExpr())
297+
}
298+
299+
pragma[nomagic]
300+
private predicate compatibleTypesLeft(DataFlowType t1, DataFlowType t2) {
301+
t1.isUnknownType()
302+
or
303+
t1.asClosureExpr() = t2.asClosureExpr()
304+
or
305+
compatibleTypesSourceContextParameterTypeLeft(t1, t2)
306+
}
307+
308+
predicate compatibleTypes(DataFlowType t1, DataFlowType t2) {
309+
compatibleTypesLeft(t1, t2)
310+
or
311+
compatibleTypesLeft(t2, t1)
312+
}
313+
314+
pragma[nomagic]
315+
predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) {
316+
exists(t1) and t2.isUnknownType()
317+
or
318+
compatibleTypesSourceContextParameterTypeLeft(t1, t2)
319+
}
320+
321+
DataFlowType getNodeType(NodePublic node) {
322+
result.asClosureExpr() = node.asExpr()
323+
or
324+
result.asClosureExpr() = node.(ClosureParameterNode).getCfgScope()
325+
or
326+
exists(VariableCapture::Flow::SynthesizedCaptureNode scn |
327+
scn = node.(CaptureNode).getSynthesizedCaptureNode() and
328+
if scn.isInstanceAccess()
329+
then result.asClosureExpr() = scn.getEnclosingCallable()
330+
else result.isUnknownType()
331+
)
332+
or
333+
not lambdaCreationExpr(node.asExpr()) and
334+
not node instanceof ClosureParameterNode and
335+
not node instanceof CaptureNode and
336+
result.isUnknownType()
337+
}
338+
266339
// Defines a set of aliases needed for the `RustDataFlow` module
267340
private module Aliases {
268341
class DataFlowCallableAlias = DataFlowCallable;
269342

343+
class DataFlowTypeAlias = DataFlowType;
344+
345+
predicate compatibleTypesAlias = compatibleTypes/2;
346+
347+
predicate typeStrongerThanAlias = typeStrongerThan/2;
348+
349+
predicate getNodeTypeAlias = getNodeType/1;
350+
270351
class ReturnKindAlias = ReturnKind;
271352

272353
class DataFlowCallAlias = DataFlowCall;
@@ -398,8 +479,6 @@ module RustDataFlowGen<RustDataFlowInputSig Input> implements InputSig<Location>
398479
result = node.(Node::Node).getEnclosingCallable()
399480
}
400481

401-
DataFlowType getNodeType(Node node) { any() }
402-
403482
predicate nodeIsHidden(Node node) {
404483
node instanceof SsaNode or
405484
node.(FlowSummaryNode).getSummaryNode().isHidden() or
@@ -486,15 +565,17 @@ module RustDataFlowGen<RustDataFlowInputSig Input> implements InputSig<Location>
486565
*/
487566
OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) { call = result.getCall(kind) }
488567

489-
// NOTE: For now we use the type `Unit` and do not benefit from type
490-
// information in the data flow analysis.
491-
final class DataFlowType extends Unit {
492-
string toString() { result = "" }
493-
}
568+
class DataFlowType = DataFlowTypeAlias;
494569

495-
predicate compatibleTypes(DataFlowType t1, DataFlowType t2) { any() }
570+
predicate compatibleTypes = compatibleTypesAlias/2;
496571

497-
predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
572+
predicate typeStrongerThan = typeStrongerThanAlias/2;
573+
574+
DataFlowType getSourceContextParameterNodeType(Node p) {
575+
exists(p) and result.isSourceContextParameterType()
576+
}
577+
578+
predicate getNodeType = getNodeTypeAlias/1;
498579

499580
class Content = ContentAlias;
500581

@@ -897,6 +978,8 @@ module RustDataFlowGen<RustDataFlowInputSig Input> implements InputSig<Location>
897978
predicate localMustFlowStep(Node node1, Node node2) {
898979
SsaFlow::localMustFlowStep(node1, node2)
899980
or
981+
LocalFlow::localMustFlowStep(node1, node2)
982+
or
900983
FlowSummaryImpl::Private::Steps::summaryLocalMustFlowStep(node1
901984
.(FlowSummaryNode)
902985
.getSummaryNode(), node2.(FlowSummaryNode).getSummaryNode())
@@ -1110,6 +1193,12 @@ private module Cached {
11101193
TCfgScope(CfgScope scope) or
11111194
TSummarizedCallable(SummarizedCallable c)
11121195

1196+
cached
1197+
newtype TDataFlowType =
1198+
TClosureExprType(Expr e) { lambdaCreationExpr(e) } or
1199+
TUnknownType() or
1200+
TSourceContextParameterType()
1201+
11131202
/** This is the local flow predicate that is exposed. */
11141203
cached
11151204
predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) {

‎rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ private module Debug {
280280
private import Content
281281
private import codeql.rust.dataflow.internal.DataFlowImpl
282282
private import codeql.rust.internal.typeinference.TypeMention
283-
private import codeql.rust.internal.typeinference.Type
283+
private import codeql.rust.internal.typeinference.Type as Type
284284

285285
private predicate relevantManualModel(SummarizedCallableImpl sc, string can) {
286286
exists(Provenance manual |
@@ -298,7 +298,7 @@ private module Debug {
298298
sc.propagatesFlow(input, _, _, _, _, _) and
299299
input.head() = SummaryComponent::argument(pos) and
300300
p = pos.getParameterIn(sc.getParamList()) and
301-
tm.getType() instanceof RefType and
301+
tm.getType() instanceof Type::RefType and
302302
not input.tail().head() = SummaryComponent::content(TSingletonContentSet(TReferenceContent()))
303303
|
304304
tm = p.getTypeRepr()
@@ -313,7 +313,7 @@ private module Debug {
313313
exists(TypeMention tm |
314314
relevantManualModel(sc, can) and
315315
sc.propagatesFlow(_, output, _, _, _, _) and
316-
tm.getType() instanceof RefType and
316+
tm.getType() instanceof Type::RefType and
317317
output.head() = SummaryComponent::return(_) and
318318
not output.tail().head() =
319319
SummaryComponent::content(TSingletonContentSet(TReferenceContent())) and

‎rust/ql/test/library-tests/dataflow/lambdas/inline-flow.expected‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ edges
1313
| main.rs:29:13:29:22 | f(...) | main.rs:29:9:29:9 | b | provenance | |
1414
| main.rs:29:21:29:21 | a | main.rs:27:20:27:23 | ... | provenance | |
1515
| main.rs:29:21:29:21 | a | main.rs:29:13:29:22 | f(...) | provenance | |
16-
| main.rs:37:16:37:25 | source(...) | main.rs:39:5:39:5 | [post] f [captured capt] | provenance | |
17-
| main.rs:39:5:39:5 | [post] f [captured capt] | main.rs:40:10:40:13 | capt | provenance | |
18-
| main.rs:39:5:39:5 | [post] f [captured capt] | main.rs:44:5:44:5 | g [captured capt] | provenance | |
16+
| main.rs:37:16:37:25 | source(...) | main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | provenance | |
17+
| main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | main.rs:40:10:40:13 | capt | provenance | |
18+
| main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | main.rs:44:5:44:5 | g [captured capt] | provenance | |
1919
| main.rs:44:5:44:5 | g [captured capt] | main.rs:42:14:42:17 | capt | provenance | |
2020
| main.rs:47:29:49:1 | { ... } | main.rs:57:10:57:12 | f(...) | provenance | |
2121
| main.rs:48:5:48:14 | source(...) | main.rs:47:29:49:1 | { ... } | provenance | |
@@ -37,7 +37,6 @@ edges
3737
| main.rs:82:7:82:7 | x | main.rs:89:12:89:12 | ... | provenance | |
3838
| main.rs:82:7:82:7 | x | main.rs:99:17:99:17 | ... | provenance | |
3939
| main.rs:82:7:82:7 | x | main.rs:101:17:101:17 | ... | provenance | |
40-
| main.rs:82:7:82:7 | x | main.rs:102:17:102:17 | ... | provenance | |
4140
| main.rs:86:9:86:9 | a | main.rs:87:24:87:24 | a | provenance | |
4241
| main.rs:86:13:86:22 | source(...) | main.rs:86:9:86:9 | a | provenance | |
4342
| main.rs:87:12:87:12 | ... | main.rs:87:20:87:20 | x | provenance | |
@@ -56,7 +55,6 @@ edges
5655
| main.rs:100:13:100:22 | source(...) | main.rs:100:9:100:9 | b | provenance | |
5756
| main.rs:101:17:101:17 | ... | main.rs:101:25:101:25 | x | provenance | |
5857
| main.rs:101:29:101:29 | b | main.rs:93:33:93:38 | ...: i64 | provenance | |
59-
| main.rs:102:17:102:17 | ... | main.rs:102:25:102:25 | x | provenance | |
6058
nodes
6159
| main.rs:10:20:10:52 | if cond {...} else {...} | semmle.label | if cond {...} else {...} |
6260
| main.rs:10:30:10:39 | source(...) | semmle.label | source(...) |
@@ -75,7 +73,7 @@ nodes
7573
| main.rs:29:21:29:21 | a | semmle.label | a |
7674
| main.rs:30:10:30:10 | b | semmle.label | b |
7775
| main.rs:37:16:37:25 | source(...) | semmle.label | source(...) |
78-
| main.rs:39:5:39:5 | [post] f [captured capt] | semmle.label | [post] f [captured capt] |
76+
| main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | semmle.label | [post] f : ... => .. [captured capt] |
7977
| main.rs:40:10:40:13 | capt | semmle.label | capt |
8078
| main.rs:42:14:42:17 | capt | semmle.label | capt |
8179
| main.rs:44:5:44:5 | g [captured capt] | semmle.label | g [captured capt] |
@@ -123,8 +121,6 @@ nodes
123121
| main.rs:101:17:101:17 | ... | semmle.label | ... |
124122
| main.rs:101:25:101:25 | x | semmle.label | x |
125123
| main.rs:101:29:101:29 | b | semmle.label | b |
126-
| main.rs:102:17:102:17 | ... | semmle.label | ... |
127-
| main.rs:102:25:102:25 | x | semmle.label | x |
128124
subpaths
129125
| main.rs:29:21:29:21 | a | main.rs:27:20:27:23 | ... | main.rs:27:26:27:52 | if cond {...} else {...} | main.rs:29:13:29:22 | f(...) |
130126
| main.rs:77:21:77:21 | a | main.rs:66:24:66:32 | ...: i64 | main.rs:66:42:72:1 | { ... } | main.rs:77:13:77:22 | f(...) |
@@ -144,5 +140,3 @@ testFailures
144140
| main.rs:99:25:99:25 | x | main.rs:100:13:100:22 | source(...) | main.rs:99:25:99:25 | x | $@ | main.rs:100:13:100:22 | source(...) | source(...) |
145141
| main.rs:101:25:101:25 | x | main.rs:98:13:98:22 | source(...) | main.rs:101:25:101:25 | x | $@ | main.rs:98:13:98:22 | source(...) | source(...) |
146142
| main.rs:101:25:101:25 | x | main.rs:100:13:100:22 | source(...) | main.rs:101:25:101:25 | x | $@ | main.rs:100:13:100:22 | source(...) | source(...) |
147-
| main.rs:102:25:102:25 | x | main.rs:98:13:98:22 | source(...) | main.rs:102:25:102:25 | x | $@ | main.rs:98:13:98:22 | source(...) | source(...) |
148-
| main.rs:102:25:102:25 | x | main.rs:100:13:100:22 | source(...) | main.rs:102:25:102:25 | x | $@ | main.rs:100:13:100:22 | source(...) | source(...) |

‎rust/ql/test/library-tests/dataflow/lambdas/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ fn test_apply_wrap() {
9999
apply_wrap(|x| sink(x), a); // $ hasValueFlow=79 $ SPURIOUS: hasValueFlow=80
100100
let b = source(80);
101101
apply_wrap(|x| sink(x), b); // $ hasValueFlow=80 $ SPURIOUS: hasValueFlow=79
102-
apply_wrap(|x| sink(x), 0); // $ SPURIOUS: hasValueFlow=79 hasValueFlow=80
102+
apply_wrap(|x| sink(x), 0);
103103
}
104104

105105
fn main() {

0 commit comments

Comments
 (0)