Skip to content

Commit e481c01

Browse files
hvitvedhvitved
authored
Merge pull request #22287 from hvitved/rust/default-callback-models
Rust: Assume callbacks will be invoked in library functions
2 parents c914268 + 52cbb67 commit e481c01

8 files changed

Lines changed: 92 additions & 2 deletions

File tree

‎rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ final class CallableScope extends CfgScopeImpl, Callable {
3535
CallableScope() {
3636
// A function without a body corresponds to a trait method signature and
3737
// should not have a CFG scope.
38-
this.hasBody()
38+
this.hasBody() and
39+
this.fromSource() // exclude stubs in tests defined using `additionalExternalFile`
3940
}
4041

4142
override predicate scopeFirst(AstNode first) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ module RustDataFlowGen<RustDataFlowInputSig Input> implements InputSig<Location>
440440
or
441441
result = "self" and this.isSelf()
442442
or
443-
result = "closure self" and this.isClosureSelf()
443+
result = "closure-self" and this.isClosureSelf()
444444
}
445445

446446
ParamBase getParameterIn(ParamList ps) {

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ private import codeql.rust.dataflow.FlowBarrier
5858
private import codeql.rust.dataflow.FlowSummary
5959
private import codeql.rust.dataflow.FlowSource
6060
private import codeql.rust.dataflow.FlowSink
61+
private import codeql.rust.internal.CachedStages
62+
private import codeql.rust.internal.typeinference.FunctionType
63+
private import codeql.rust.internal.typeinference.TypeMention
64+
private import codeql.rust.frameworks.stdlib.Stdlib
6165

6266
/**
6367
* Holds if in a call to the function with canonical path `path`, the value referred
@@ -207,6 +211,39 @@ private class SummarizedCallableFromModel extends SummarizedCallable::Range {
207211
}
208212
}
209213

214+
/**
215+
* Holds if library function `f` has a callback at position `n`. In this case we
216+
* add a flow model that achieves the effect of simulating that the callback is
217+
* invoked, which is needed for flow through captured variables to work.
218+
*/
219+
cached
220+
predicate mayInvokeCallback(Function f, int n) {
221+
Stages::TypeInferenceStage::ref() and
222+
exists(TypeMention tm, Trait trait |
223+
tm = f.getParam(n).getTypeRepr() and
224+
trait = getALookupTrait(f, tm.getType()) and
225+
trait.getSupertrait*() instanceof FnOnceTrait and
226+
not f.fromSource()
227+
)
228+
}
229+
230+
private class SummarizedCallableWithCallback extends SummarizedCallable::Range {
231+
private int pos;
232+
233+
SummarizedCallableWithCallback() { mayInvokeCallback(this, pos) }
234+
235+
override predicate propagatesFlow(
236+
string input, string output, boolean preservesValue, Provenance p, boolean isExact, string model
237+
) {
238+
input = "Argument[" + pos + "]" and
239+
output = "Argument[" + pos + "].Parameter[closure-self]" and
240+
preservesValue = true and
241+
p = "hq-generated" and
242+
isExact = true and
243+
model = "heuristic-callback"
244+
}
245+
}
246+
210247
private class FlowSourceFromModel extends FlowSource::Range {
211248
private string path;
212249

‎rust/ql/lib/codeql/rust/internal/CachedStages.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ module Stages {
130130
module TypeInferenceStage {
131131
private import codeql.rust.internal.typeinference.Type
132132
private import codeql.rust.internal.typeinference.TypeInference
133+
private import codeql.rust.dataflow.internal.ModelsAsData
133134

134135
/**
135136
* Always holds.
@@ -150,6 +151,8 @@ module Stages {
150151
exists(Type t)
151152
or
152153
exists(inferType(_))
154+
or
155+
mayInvokeCallback(_, _)
153156
}
154157
}
155158

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
pub fn may_invoke_callback1<F: Fn(i64)>(f: F) {}
2+
3+
pub fn may_invoke_callback2<F: FnOnce(i64)>(f: F) {}
4+
5+
pub fn may_invoke_callback3(f: impl Fn(i64)) {}
6+
7+
pub fn may_invoke_callback4<T>(f: T)
8+
where
9+
T: for<'a> FnOnce(&'a mut i64),
10+
{
11+
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ edges
5555
| main.rs:100:13:100:22 | source(...) | main.rs:100:9:100:9 | b | provenance | |
5656
| main.rs:101:17:101:17 | ... | main.rs:101:25:101:25 | x | provenance | |
5757
| main.rs:101:29:101:29 | b | main.rs:93:33:93:38 | ...: i64 | provenance | |
58+
| main.rs:109:13:109:22 | source(...) | main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | provenance | |
59+
| main.rs:109:13:109:22 | source(...) | main.rs:111:26:111:36 | \|...\| ... : ... => .. [captured a] | provenance | |
60+
| main.rs:109:13:109:22 | source(...) | main.rs:112:26:112:36 | \|...\| ... : ... => .. [captured a] | provenance | |
61+
| main.rs:109:13:109:22 | source(...) | main.rs:113:26:113:36 | \|...\| ... : ... => .. [captured a] | provenance | |
62+
| main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | main.rs:110:35:110:35 | a | provenance | heuristic-callback |
63+
| main.rs:111:26:111:36 | \|...\| ... : ... => .. [captured a] | main.rs:111:35:111:35 | a | provenance | heuristic-callback |
64+
| main.rs:112:26:112:36 | \|...\| ... : ... => .. [captured a] | main.rs:112:35:112:35 | a | provenance | heuristic-callback |
65+
| main.rs:113:26:113:36 | \|...\| ... : ... => .. [captured a] | main.rs:113:35:113:35 | a | provenance | heuristic-callback |
5866
nodes
5967
| main.rs:10:20:10:52 | if cond {...} else {...} | semmle.label | if cond {...} else {...} |
6068
| main.rs:10:30:10:39 | source(...) | semmle.label | source(...) |
@@ -121,6 +129,15 @@ nodes
121129
| main.rs:101:17:101:17 | ... | semmle.label | ... |
122130
| main.rs:101:25:101:25 | x | semmle.label | x |
123131
| main.rs:101:29:101:29 | b | semmle.label | b |
132+
| main.rs:109:13:109:22 | source(...) | semmle.label | source(...) |
133+
| main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | semmle.label | \|...\| ... : ... => .. [captured a] |
134+
| main.rs:110:35:110:35 | a | semmle.label | a |
135+
| main.rs:111:26:111:36 | \|...\| ... : ... => .. [captured a] | semmle.label | \|...\| ... : ... => .. [captured a] |
136+
| main.rs:111:35:111:35 | a | semmle.label | a |
137+
| main.rs:112:26:112:36 | \|...\| ... : ... => .. [captured a] | semmle.label | \|...\| ... : ... => .. [captured a] |
138+
| main.rs:112:35:112:35 | a | semmle.label | a |
139+
| main.rs:113:26:113:36 | \|...\| ... : ... => .. [captured a] | semmle.label | \|...\| ... : ... => .. [captured a] |
140+
| main.rs:113:35:113:35 | a | semmle.label | a |
124141
subpaths
125142
| 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(...) |
126143
| 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(...) |
@@ -140,3 +157,7 @@ testFailures
140157
| 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(...) |
141158
| 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(...) |
142159
| 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(...) |
160+
| main.rs:110:35:110:35 | a | main.rs:109:13:109:22 | source(...) | main.rs:110:35:110:35 | a | $@ | main.rs:109:13:109:22 | source(...) | source(...) |
161+
| main.rs:111:35:111:35 | a | main.rs:109:13:109:22 | source(...) | main.rs:111:35:111:35 | a | $@ | main.rs:109:13:109:22 | source(...) | source(...) |
162+
| main.rs:112:35:112:35 | a | main.rs:109:13:109:22 | source(...) | main.rs:112:35:112:35 | a | $@ | main.rs:109:13:109:22 | source(...) | source(...) |
163+
| main.rs:113:35:113:35 | a | main.rs:109:13:109:22 | source(...) | main.rs:113:35:113:35 | a | $@ | main.rs:109:13:109:22 | source(...) | source(...) |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/rust-all
4+
extensible: additionalExternalFile
5+
data:
6+
- ["external_file.rs"]

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ fn test_apply_wrap() {
102102
apply_wrap(|x| sink(x), 0);
103103
}
104104

105+
mod external_file;
106+
use external_file::*;
107+
108+
fn test_external_call() {
109+
let a = source(81);
110+
may_invoke_callback1(|x| sink(a)); // $ hasValueFlow=81
111+
may_invoke_callback2(|x| sink(a)); // $ hasValueFlow=81
112+
may_invoke_callback3(|x| sink(a)); // $ hasValueFlow=81
113+
may_invoke_callback4(|x| sink(a)); // $ hasValueFlow=81
114+
}
115+
105116
fn main() {
106117
closure_flow_out();
107118
closure_flow_in();

0 commit comments

Comments
 (0)