Skip to content

Rust: Refactor MaD provenance-based filtering #21072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hvitved wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Rust: Refactor MaD provenance-based filtering #21072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 2 additions & 20 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ final class DataFlowCallable extends TDataFlowCallable {

/** Gets a textual representation of this callable. */
string toString() {
result = [this.asCfgScope().toString(), this.asSummarizedCallable().toString()]
result = [this.asCfgScope().toString(), "[summarized] " + this.asSummarizedCallable()]
}

/** Gets the location of this callable. */
Expand Down Expand Up @@ -443,25 +443,7 @@ module RustDataFlow implements InputSig<Location> {
exists(Call c | c = call.asCall() |
result.asCfgScope() = c.getARuntimeTarget()
or
exists(SummarizedCallable sc, Function staticTarget |
staticTarget = getStaticTargetExt(c) and
sc = result.asSummarizedCallable() and
// Only use summarized callables with generated summaries in case
// the static call target is not in the source code.
// Note that if `applyGeneratedModel` holds it implies that there doesn't
// exist a manual model.
not (
staticTarget.fromSource() and
sc.applyGeneratedModel()
)
|
sc = staticTarget
or
// only apply trait models to concrete implementations when they are not
// defined in source code
staticTarget.implements(sc) and
not staticTarget.fromSource()
)
result.asSummarizedCallable() = getStaticTargetExt(c)
)
}

Expand Down
60 changes: 47 additions & 13 deletions rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,27 +111,61 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
)
}

private class SummarizedCallableFromModel extends SummarizedCallable::Range {
private string path;
private predicate summaryModel(
Function f, string input, string output, string kind, Provenance provenance, boolean isExact,
QlBuiltins::ExtensionId madId
) {
exists(string path, Function f0 |
summaryModel(path, input, output, kind, provenance, madId) and
f0.getCanonicalPath() = path
|
f = f0 and
isExact = true
or
f.implements(f0) and
isExact = false
)
}

SummarizedCallableFromModel() {
summaryModel(path, _, _, _, _, _) and
this.getCanonicalPath() = path
}
private predicate summaryModelRelevant(
Function f, string input, string output, string kind, Provenance provenance,
QlBuiltins::ExtensionId madId
) {
exists(boolean isExact | summaryModel(f, input, output, kind, provenance, isExact, madId) |
(
provenance.isManual()
or
// only apply generated models to functions not defined in source code, and
// when there are no exact manual models for the functions
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment uses plural "functions" but should use singular "function" since it refers to a specific function f.

Copilot uses AI. Check for mistakes.
provenance.isGenerated() and
not any(Provenance manual | summaryModel(f, _, _, _, manual, true, _)).isManual() and
not f.fromSource()
) and
(
isExact = true
or
// only apply trait models to concrete implementations when they are not
// defined in source code, and when there are no exact model for the functions
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "when there are no exact model for the functions" but should be clearer about whether this means "no exact model with any provenance" or "no exact model with the same provenance". Consider rephrasing for clarity.

Suggested change
// defined in source code, and when there are no exact model for the functions
// defined in source code, and when there is no exact model with the same provenance for the function

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment uses plural "functions" but should use singular "function" since it refers to a specific function f.

Suggested change
// defined in source code, and when there are no exact model for the functions
// defined in source code, and when there is no exact model for the function

Copilot uses AI. Check for mistakes.
isExact = false and
not summaryModel(f, _, _, _, provenance, true, _) and
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition checks for the absence of an exact model with the same provenance. However, this means a generated trait model could still be applied even when a manual specific implementation model exists (since they have different provenance values). Based on the comment stating "when there are no exact model for the functions" (line 148), this should likely check for any exact model regardless of provenance: not summaryModel(f, _, _, _, _, true, _)

Suggested change
not summaryModel(f, _, _, _, provenance, true, _) and
not summaryModel(f, _, _, _, _, true, _) and

Copilot uses AI. Check for mistakes.
not f.fromSource()
)
)
}

private class SummarizedCallableFromModel extends SummarizedCallable::Range {
SummarizedCallableFromModel() { summaryModelRelevant(this, _, _, _, _, _) }

override predicate hasProvenance(Provenance provenance) {
summaryModel(path, _, _, _, provenance, _)
summaryModelRelevant(this, _, _, _, provenance, _)
}

private predicate hasManualModel() { summaryModel(path, _, _, _, "manual", _) }

override predicate propagatesFlow(
string input, string output, boolean preservesValue, string model
) {
exists(string kind, string provenance, QlBuiltins::ExtensionId madId |
summaryModel(path, input, output, kind, provenance, madId) and
model = "MaD:" + madId.toString() and
(provenance = "manual" or not this.hasManualModel())
exists(string kind, QlBuiltins::ExtensionId madId |
summaryModelRelevant(this, input, output, kind, _, madId) and
model = "MaD:" + madId.toString()
|
kind = "value" and
preservesValue = true
Expand Down
3 changes: 3 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/alloc.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,8 @@ extensions:
- ["<alloc::string::String>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
- ["<alloc::string::String>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]
- ["<_ as alloc::string::ToString>::to_string", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
# Overwrite generated model
- ["<alloc::string::String as core::ops::arith::Add>::add", "Argument[self,0]", "ReturnValue", "taint", "manual"]
- ["<alloc::string::String as core::ops::arith::Add>::add", "Argument[0].Reference", "ReturnValue", "taint", "manual"]
# Vec
- ["alloc::vec::from_elem", "Argument[0]", "ReturnValue.Element", "value", "manual"]
12 changes: 6 additions & 6 deletions rust/ql/test/library-tests/dataflow/global/viableCallable.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
| main.rs:2:5:2:12 | ... + ... | {EXTERNAL LOCATION} | fn add |
| main.rs:2:5:2:12 | ... + ... | {EXTERNAL LOCATION} | [summarized] fn add |
| main.rs:13:5:13:13 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:17:13:17:23 | get_data(...) | main.rs:12:1:14:1 | fn get_data |
| main.rs:18:5:18:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
Expand Down Expand Up @@ -60,7 +60,7 @@
| main.rs:228:13:228:34 | ...::new(...) | main.rs:221:5:224:5 | fn new |
| main.rs:228:24:228:33 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:230:5:230:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:252:11:252:15 | * ... | {EXTERNAL LOCATION} | fn deref |
| main.rs:252:11:252:15 | * ... | {EXTERNAL LOCATION} | [summarized] fn deref |
| main.rs:258:28:258:36 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:260:13:260:17 | ... + ... | main.rs:236:5:239:5 | fn add |
| main.rs:261:5:261:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
Expand All @@ -77,7 +77,7 @@
| main.rs:282:5:282:10 | ... *= ... | main.rs:243:5:245:5 | fn mul_assign |
| main.rs:283:5:283:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:286:28:286:37 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:288:13:288:29 | * ... | {EXTERNAL LOCATION} | fn deref |
| main.rs:288:13:288:29 | * ... | {EXTERNAL LOCATION} | [summarized] fn deref |
| main.rs:288:14:288:29 | ...::deref(...) | main.rs:251:5:253:5 | fn deref |
| main.rs:289:5:289:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:291:28:291:37 | source(...) | main.rs:1:1:3:1 | fn source |
Expand All @@ -101,14 +101,14 @@
| main.rs:346:17:346:25 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:347:9:347:15 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:350:5:350:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:354:13:354:55 | ...::block_on(...) | {EXTERNAL LOCATION} | fn block_on |
| main.rs:354:13:354:55 | ...::block_on(...) | {EXTERNAL LOCATION} | [summarized] fn block_on |
| main.rs:354:41:354:54 | async_source(...) | main.rs:335:1:339:1 | fn async_source |
| main.rs:355:5:355:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:357:5:357:62 | ...::block_on(...) | {EXTERNAL LOCATION} | fn block_on |
| main.rs:357:5:357:62 | ...::block_on(...) | {EXTERNAL LOCATION} | [summarized] fn block_on |
| main.rs:357:33:357:61 | test_async_await_async_part(...) | main.rs:341:1:351:1 | fn test_async_await_async_part |
| main.rs:367:13:367:29 | self.get_number() | main.rs:378:9:380:9 | fn get_number |
| main.rs:367:13:367:29 | self.get_number() | main.rs:386:9:388:9 | fn get_number |
| main.rs:367:13:367:33 | ... * ... | {EXTERNAL LOCATION} | fn mul |
| main.rs:367:13:367:33 | ... * ... | {EXTERNAL LOCATION} | [summarized] fn mul |
| main.rs:371:13:371:21 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:379:13:379:21 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:391:13:391:22 | source(...) | main.rs:1:1:3:1 | fn source |
Expand Down
3 changes: 3 additions & 0 deletions rust/ql/test/library-tests/dataflow/models/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ fn test_trait_model<T: Ord>(x: T) {

let x6 = source(27) < 1;
sink(x6); // $ hasTaintFlow=27

let x7 = (source(28) as i32) < 1;
sink(x7);
}

#[tokio::main]
Expand Down
2 changes: 2 additions & 0 deletions rust/ql/test/library-tests/dataflow/models/models.ext.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ extensions:
- ["main::apply", "Argument[1].ReturnValue", "ReturnValue", "value", "manual"]
- ["main::get_async_number", "Argument[0]", "ReturnValue.Future", "value", "manual"]
- ["<_ as core::cmp::PartialOrd>::lt", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
# Overwrites the generic trait model for i32
- ["<core::i32 as core::cmp::PartialOrd>::lt", "Argument[0]", "ReturnValue", "taint", "manual"]
40 changes: 19 additions & 21 deletions rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,71 +1,69 @@
models
| 1 | Summary: <_ as alloc::string::ToString>::to_string; Argument[self].Reference; ReturnValue; taint |
| 2 | Summary: <_ as core::convert::From>::from; Argument[0]; ReturnValue; taint |
| 3 | Summary: <_ as core::ops::arith::Add>::add; Argument[0].Reference; ReturnValue; taint |
| 4 | Summary: <_ as core::ops::arith::Add>::add; Argument[self]; ReturnValue; taint |
| 5 | Summary: <_ as core::ops::index::Index>::index; Argument[self].Reference.Element; ReturnValue.Reference; value |
| 6 | Summary: <alloc::string::String as core::convert::From>::from; Argument[0].Reference; ReturnValue; value |
| 7 | Summary: <alloc::string::String as core::ops::arith::Add>::add; Argument[self]; ReturnValue; value |
| 8 | Summary: <alloc::string::String>::as_str; Argument[self]; ReturnValue; value |
| 9 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
| 10 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
| 3 | Summary: <_ as core::ops::index::Index>::index; Argument[self].Reference.Element; ReturnValue.Reference; value |
| 4 | Summary: <alloc::string::String as core::convert::From>::from; Argument[0].Reference; ReturnValue; value |
| 5 | Summary: <alloc::string::String as core::ops::arith::Add>::add; Argument[0].Reference; ReturnValue; taint |
| 6 | Summary: <alloc::string::String as core::ops::arith::Add>::add; Argument[self,0]; ReturnValue; taint |
| 7 | Summary: <alloc::string::String>::as_str; Argument[self]; ReturnValue; value |
| 8 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
| 9 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
edges
| main.rs:26:9:26:9 | s | main.rs:27:19:27:19 | s | provenance | |
| main.rs:26:9:26:9 | s | main.rs:27:19:27:25 | s[...] | provenance | |
| main.rs:26:13:26:22 | source(...) | main.rs:26:9:26:9 | s | provenance | |
| main.rs:27:9:27:14 | sliced [&ref] | main.rs:28:16:28:21 | sliced | provenance | |
| main.rs:27:18:27:25 | &... [&ref] | main.rs:27:9:27:14 | sliced [&ref] | provenance | |
| main.rs:27:19:27:19 | s | main.rs:27:19:27:25 | s[...] | provenance | MaD:5 |
| main.rs:27:19:27:19 | s | main.rs:27:19:27:25 | s[...] | provenance | MaD:3 |
| main.rs:27:19:27:25 | s[...] | main.rs:27:18:27:25 | &... [&ref] | provenance | |
| main.rs:32:9:32:10 | s1 | main.rs:35:14:35:15 | s1 | provenance | |
| main.rs:32:14:32:23 | source(...) | main.rs:32:9:32:10 | s1 | provenance | |
| main.rs:35:9:35:10 | s4 | main.rs:38:10:38:11 | s4 | provenance | |
| main.rs:35:14:35:15 | s1 | main.rs:35:14:35:20 | ... + ... | provenance | MaD:4 |
| main.rs:35:14:35:15 | s1 | main.rs:35:14:35:20 | ... + ... | provenance | MaD:7 |
| main.rs:35:14:35:15 | s1 | main.rs:35:14:35:20 | ... + ... | provenance | MaD:6 |
| main.rs:35:14:35:20 | ... + ... | main.rs:35:9:35:10 | s4 | provenance | |
| main.rs:43:9:43:10 | s1 | main.rs:46:34:46:35 | s1 | provenance | |
| main.rs:43:14:43:23 | source(...) | main.rs:43:9:43:10 | s1 | provenance | |
| main.rs:46:33:46:35 | &s1 [&ref] | main.rs:46:10:46:35 | ... + ... | provenance | MaD:3 |
| main.rs:46:33:46:35 | &s1 [&ref] | main.rs:46:10:46:35 | ... + ... | provenance | MaD:5 |
| main.rs:46:34:46:35 | s1 | main.rs:46:33:46:35 | &s1 [&ref] | provenance | |
| main.rs:51:9:51:10 | s1 | main.rs:52:27:52:28 | s1 | provenance | |
| main.rs:51:14:51:29 | source_slice(...) | main.rs:51:9:51:10 | s1 | provenance | |
| main.rs:52:9:52:10 | s2 | main.rs:53:10:53:11 | s2 | provenance | |
| main.rs:52:14:52:29 | ...::from(...) | main.rs:52:9:52:10 | s2 | provenance | |
| main.rs:52:27:52:28 | s1 | main.rs:52:14:52:29 | ...::from(...) | provenance | MaD:2 |
| main.rs:52:27:52:28 | s1 | main.rs:52:14:52:29 | ...::from(...) | provenance | MaD:6 |
| main.rs:52:27:52:28 | s1 | main.rs:52:14:52:29 | ...::from(...) | provenance | MaD:4 |
| main.rs:57:9:57:10 | s1 | main.rs:58:14:58:15 | s1 | provenance | |
| main.rs:57:14:57:29 | source_slice(...) | main.rs:57:9:57:10 | s1 | provenance | |
| main.rs:58:9:58:10 | s2 | main.rs:59:10:59:11 | s2 | provenance | |
| main.rs:58:14:58:15 | s1 | main.rs:58:14:58:27 | s1.to_string() | provenance | MaD:1 |
| main.rs:58:14:58:27 | s1.to_string() | main.rs:58:9:58:10 | s2 | provenance | |
| main.rs:63:9:63:9 | s | main.rs:64:16:64:16 | s | provenance | |
| main.rs:63:13:63:22 | source(...) | main.rs:63:9:63:9 | s | provenance | |
| main.rs:64:16:64:16 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:8 |
| main.rs:64:16:64:16 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:7 |
| main.rs:68:9:68:9 | s | main.rs:70:34:70:61 | MacroExpr | provenance | |
| main.rs:68:9:68:9 | s | main.rs:73:34:73:59 | MacroExpr | provenance | |
| main.rs:68:13:68:22 | source(...) | main.rs:68:9:68:9 | s | provenance | |
| main.rs:70:9:70:18 | formatted1 | main.rs:71:10:71:19 | formatted1 | provenance | |
| main.rs:70:22:70:62 | ...::format(...) | main.rs:70:9:70:18 | formatted1 | provenance | |
| main.rs:70:34:70:61 | MacroExpr | main.rs:70:22:70:62 | ...::format(...) | provenance | MaD:9 |
| main.rs:70:34:70:61 | MacroExpr | main.rs:70:22:70:62 | ...::format(...) | provenance | MaD:8 |
| main.rs:73:9:73:18 | formatted2 | main.rs:74:10:74:19 | formatted2 | provenance | |
| main.rs:73:22:73:60 | ...::format(...) | main.rs:73:9:73:18 | formatted2 | provenance | |
| main.rs:73:34:73:59 | MacroExpr | main.rs:73:22:73:60 | ...::format(...) | provenance | MaD:9 |
| main.rs:73:34:73:59 | MacroExpr | main.rs:73:22:73:60 | ...::format(...) | provenance | MaD:8 |
| main.rs:76:9:76:13 | width | main.rs:77:34:77:74 | MacroExpr | provenance | |
| main.rs:76:17:76:32 | source_usize(...) | main.rs:76:9:76:13 | width | provenance | |
| main.rs:77:9:77:18 | formatted3 | main.rs:78:10:78:19 | formatted3 | provenance | |
| main.rs:77:22:77:75 | ...::format(...) | main.rs:77:9:77:18 | formatted3 | provenance | |
| main.rs:77:34:77:74 | MacroExpr | main.rs:77:22:77:75 | ...::format(...) | provenance | MaD:9 |
| main.rs:77:34:77:74 | MacroExpr | main.rs:77:22:77:75 | ...::format(...) | provenance | MaD:8 |
| main.rs:82:9:82:10 | s1 | main.rs:86:18:86:25 | MacroExpr | provenance | |
| main.rs:82:9:82:10 | s1 | main.rs:87:18:87:32 | MacroExpr | provenance | |
| main.rs:82:14:82:23 | source(...) | main.rs:82:9:82:10 | s1 | provenance | |
| main.rs:86:18:86:25 | ...::format(...) | main.rs:86:18:86:25 | { ... } | provenance | |
| main.rs:86:18:86:25 | ...::must_use(...) | main.rs:86:10:86:26 | MacroExpr | provenance | |
| main.rs:86:18:86:25 | MacroExpr | main.rs:86:18:86:25 | ...::format(...) | provenance | MaD:9 |
| main.rs:86:18:86:25 | { ... } | main.rs:86:18:86:25 | ...::must_use(...) | provenance | MaD:10 |
| main.rs:86:18:86:25 | MacroExpr | main.rs:86:18:86:25 | ...::format(...) | provenance | MaD:8 |
| main.rs:86:18:86:25 | { ... } | main.rs:86:18:86:25 | ...::must_use(...) | provenance | MaD:9 |
| main.rs:87:18:87:32 | ...::format(...) | main.rs:87:18:87:32 | { ... } | provenance | |
| main.rs:87:18:87:32 | ...::must_use(...) | main.rs:87:10:87:33 | MacroExpr | provenance | |
| main.rs:87:18:87:32 | MacroExpr | main.rs:87:18:87:32 | ...::format(...) | provenance | MaD:9 |
| main.rs:87:18:87:32 | { ... } | main.rs:87:18:87:32 | ...::must_use(...) | provenance | MaD:10 |
| main.rs:87:18:87:32 | MacroExpr | main.rs:87:18:87:32 | ...::format(...) | provenance | MaD:8 |
| main.rs:87:18:87:32 | { ... } | main.rs:87:18:87:32 | ...::must_use(...) | provenance | MaD:9 |
nodes
| main.rs:26:9:26:9 | s | semmle.label | s |
| main.rs:26:13:26:22 | source(...) | semmle.label | source(...) |
Expand Down
2 changes: 1 addition & 1 deletion rust/ql/test/library-tests/dataflow/strings/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn string_add() {
let s4 = s1 + s3;
let s5 = s2 + s3;

sink(s4); // $ SPURIOUS: hasValueFlow=83 MISSING: hasTaintFlow=83
sink(s4); // $ hasTaintFlow=83
sink(s5);
}

Expand Down
Loading