Skip to content

Commit 5523b5e

Browse files
authored
Merge pull request #21271 from geoffw0/neutralmodels
Rust: Add support for neutral models.
2 parents 4e4d055 + ccc3181 commit 5523b5e

File tree

9 files changed

+300
-93
lines changed

9 files changed

+300
-93
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added support for neutral models (`extensible: neutralModel`) to control where generated source, sink and flow summary models apply.

rust/ql/lib/codeql/files/FileSystem.qll

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,22 @@ class Folder = Impl::Folder;
3636

3737
module Folder = Impl::Folder;
3838

39+
/**
40+
* Holds if the file identified by `relativePath` should be treated as though it is external
41+
* to the target project, even though it is within the source code directory. This is used for
42+
* testing.
43+
*/
44+
extensible predicate additionalExternalFile(string relativePath);
45+
3946
/** A file. */
4047
class File extends Container, Impl::File {
4148
/**
4249
* Holds if this file was extracted from the source code of the target project
4350
* (rather than another location such as inside a dependency).
4451
*/
4552
predicate fromSource() {
46-
exists(ExtractorStep s | s.getAction() = "Extract" and s.getFile() = this)
53+
exists(ExtractorStep s | s.getAction() = "Extract" and s.getFile() = this) and
54+
not additionalExternalFile(this.getRelativePath())
4755
}
4856

4957
/**

rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import codeql.rust.dataflow.internal.DataFlowImpl
99
private import codeql.rust.internal.PathResolution
1010
private import codeql.rust.dataflow.FlowSummary
1111
private import codeql.rust.dataflow.Ssa
12+
private import codeql.rust.dataflow.internal.ModelsAsData
1213
private import Content
1314

1415
predicate encodeContentTupleField(TupleFieldContent c, string arg) {
@@ -46,6 +47,16 @@ module Input implements InputSig<Location, RustDataFlow> {
4647

4748
abstract class SinkBase extends SourceSinkBase { }
4849

50+
predicate neutralElement(
51+
Input::SummarizedCallableBase c, string kind, string provenance, boolean isExact
52+
) {
53+
exists(string path |
54+
neutralModel(path, kind, provenance, _) and
55+
c.getCanonicalPath() = path and
56+
isExact = true
57+
)
58+
}
59+
4960
private class CallExprFunction extends SourceBase, SinkBase {
5061
private CallExpr call;
5162

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,15 @@ extensible predicate summaryModel(
8989
QlBuiltins::ExtensionId madId
9090
);
9191

92+
/**
93+
* Holds if a neutral model exists for the function with canonical path `path`. The only
94+
* effect of a neutral model is to prevent generated and inherited models of the corresponding
95+
* `kind` (`source`, `sink` or `summary`) from being applied to that function.
96+
*/
97+
extensible predicate neutralModel(
98+
string path, string kind, string provenance, QlBuiltins::ExtensionId madId
99+
);
100+
92101
/**
93102
* Holds if the given extension tuple `madId` should pretty-print as `model`.
94103
*
@@ -109,6 +118,11 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
109118
summaryModel(path, input, output, kind, _, madId) and
110119
model = "Summary: " + path + "; " + input + "; " + output + "; " + kind
111120
)
121+
or
122+
exists(string path, string kind |
123+
neutralModel(path, kind, _, madId) and
124+
model = "Neutral: " + path + "; " + kind
125+
)
112126
}
113127

114128
private class SummarizedCallableFromModel extends SummarizedCallable::Range {
@@ -124,7 +138,9 @@ private class SummarizedCallableFromModel extends SummarizedCallable::Range {
124138
summaryModel(path, input_, output_, kind, p, madId) and
125139
f.getCanonicalPath() = path
126140
|
127-
this = f and isExact_ = true and p_ = p
141+
this = f and
142+
isExact_ = true and
143+
p_ = p
128144
or
129145
this.implements(f) and
130146
isExact_ = false and
@@ -158,6 +174,12 @@ private class FlowSourceFromModel extends FlowSource::Range {
158174
exists(QlBuiltins::ExtensionId madId |
159175
sourceModel(path, output, kind, provenance, madId) and
160176
model = "MaD:" + madId.toString()
177+
) and
178+
// Only apply generated models when no neutral model exists
179+
// (the shared code only applies neutral models to summaries at present)
180+
not (
181+
provenance.isGenerated() and
182+
neutralModel(path, "source", _, _)
161183
)
162184
}
163185
}
@@ -174,6 +196,12 @@ private class FlowSinkFromModel extends FlowSink::Range {
174196
exists(QlBuiltins::ExtensionId madId |
175197
sinkModel(path, input, kind, provenance, madId) and
176198
model = "MaD:" + madId.toString()
199+
) and
200+
// Only apply generated models when no neutral model exists
201+
// (the shared code only applies neutral models to summaries at present)
202+
not (
203+
provenance.isGenerated() and
204+
neutralModel(path, "sink", _, _)
177205
)
178206
}
179207
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
extensions:
2+
# Make sure that the extensible model predicates have at least one definition
3+
# to avoid errors about undefined extensionals.
4+
- addsTo:
5+
pack: codeql/rust-all
6+
extensible: sourceModel
7+
data: []
8+
- addsTo:
9+
pack: codeql/rust-all
10+
extensible: sinkModel
11+
data: []
12+
- addsTo:
13+
pack: codeql/rust-all
14+
extensible: summaryModel
15+
data: []
16+
- addsTo:
17+
pack: codeql/rust-all
18+
extensible: excludeFieldTaintStep
19+
data: []
20+
- addsTo:
21+
pack: codeql/rust-all
22+
extensible: neutralModel
23+
data: []
24+
- addsTo:
25+
pack: codeql/rust-all
26+
extensible: additionalExternalFile
27+
data: []
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
2+
pub fn generated_source(i: i64) -> i64 {
3+
0
4+
}
5+
6+
pub fn neutral_generated_source(i: i64) -> i64 {
7+
0
8+
}
9+
10+
pub fn neutral_manual_source(i: i64) -> i64 {
11+
0
12+
}
13+
14+
pub fn generated_sink(i: i64) {}
15+
16+
pub fn neutral_generated_sink(i: i64) {}
17+
18+
pub fn neutral_manual_sink(i: i64) {}
19+
20+
pub fn generated_summary(i: i64) -> i64 {
21+
0
22+
}
23+
24+
pub fn neutral_generated_summary(i: i64) -> i64 {
25+
0
26+
}
27+
28+
pub fn neutral_manual_summary(i: i64) -> i64 {
29+
0
30+
}

rust/ql/test/library-tests/dataflow/models/main.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,26 @@ fn test_trait_model<T: Ord>(x: T) {
410410
sink(x7);
411411
}
412412

413+
mod external_file;
414+
use external_file::*;
415+
416+
fn test_neutrals() {
417+
// neutral models should cause corresponding generated models to be ignored.
418+
// Thus the `neutral_generated_source`, `neutral_generated_sink` and
419+
// `neutral_generated_summary`, which have both a generated and a neutral
420+
// model, should not have flow.
421+
422+
sink(generated_source(1)); // $ hasValueFlow=1
423+
sink(neutral_generated_source(2));
424+
sink(neutral_manual_source(3)); // $ hasValueFlow=3
425+
generated_sink(source(4)); // $ hasValueFlow=4
426+
neutral_generated_sink(source(5));
427+
neutral_manual_sink(source(6)); // $ hasValueFlow=6
428+
sink(generated_summary(source(7))); // $ hasValueFlow=7
429+
sink(neutral_generated_summary(source(8)));
430+
sink(neutral_manual_summary(source(9))); // $ hasValueFlow=9
431+
}
432+
413433
#[tokio::main]
414434
async fn main() {
415435
test_identify();
@@ -431,5 +451,6 @@ async fn main() {
431451
test_simple_sink();
432452
test_get_async_number().await;
433453
test_arg_source();
454+
test_neutrals();
434455
let dummy = Some(0); // ensure that the the `lang:core` crate is extracted
435456
}

0 commit comments

Comments
 (0)