Skip to content

Rust: Test and model some string and iterator methods #18701

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.

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

Merged
merged 23 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6966c96
Rust: Add a test case for parse on a command line arg.
geoffw0 Feb 5, 2025
c597818
Rust: Add a flow test for to_string() and parse().
geoffw0 Feb 6, 2025
1ff7a52
Rust: Add a flow test for some iterator methods.
geoffw0 Feb 6, 2025
d0f5aad
Rust: Model to_string.
geoffw0 Feb 6, 2025
a8a0512
Rust: Model parse.
geoffw0 Feb 5, 2025
bce4735
Rust: Additional test case suggested by copilot.
geoffw0 Feb 6, 2025
78e3c89
Rust: Accept changes to integration tests.
geoffw0 Feb 6, 2025
f350181
Merge branch 'main' into nth
geoffw0 Feb 7, 2025
f5b9691
Rust: Accept fixed result.
geoffw0 Feb 7, 2025
e7fdfd0
Merge branch 'main' into nth
geoffw0 Feb 11, 2025
4f73429
Rust: Accept test changes after merging latest main.
geoffw0 Feb 11, 2025
f5ab6a6
Rust: Accept integration test changes.
geoffw0 Feb 11, 2025
edda26c
Merge branch 'main' into nth
geoffw0 Feb 12, 2025
e9b8ec9
Rust: Accept integration test changes (again).
geoffw0 Feb 12, 2025
c07a57b
Rust: Accept spurious test results (we need a barrier for numeric typ…
geoffw0 Feb 13, 2025
6c31473
Rust: Accept changes to the summary stats query .expected.
geoffw0 Feb 13, 2025
048f7db
Merge branch 'main' into nth
geoffw0 Feb 17, 2025
79525fa
Rust: Variant -> Field.
geoffw0 Feb 17, 2025
8024fb6
Rust: Add more models for Iterator.
geoffw0 Feb 18, 2025
12d5a30
Rust: Add a test of mutable iterators as well.
geoffw0 Feb 18, 2025
1a6c6a4
Rust: Effect on integration tests.
geoffw0 Feb 18, 2025
fdc76dd
Merge branch 'main' into nth
geoffw0 Feb 24, 2025
6cb8f65
Rust: Fix up .expected after merge.
geoffw0 Feb 24, 2025
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
2 changes: 1 addition & 1 deletion rust/ql/integration-tests/hello-project/summary.expected
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
| Macro calls - resolved | 2 |
| Macro calls - total | 2 |
| Macro calls - unresolved | 0 |
| Taint edges - number of edges | 4 |
| Taint edges - number of edges | 10 |
| Taint reach - nodes tainted | 0 |
| Taint reach - per million nodes | 0 |
| Taint sinks - cryptographic operations | 0 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
| Macro calls - resolved | 2 |
| Macro calls - total | 2 |
| Macro calls - unresolved | 0 |
| Taint edges - number of edges | 4 |
| Taint edges - number of edges | 10 |
| Taint reach - nodes tainted | 0 |
| Taint reach - per million nodes | 0 |
| Taint sinks - cryptographic operations | 0 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
| Macro calls - resolved | 2 |
| Macro calls - total | 2 |
| Macro calls - unresolved | 0 |
| Taint edges - number of edges | 4 |
| Taint edges - number of edges | 10 |
| Taint reach - nodes tainted | 0 |
| Taint reach - per million nodes | 0 |
| Taint sinks - cryptographic operations | 0 |
Expand Down
14 changes: 14 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,19 @@ extensions:
# Hint
- ["lang:core", "crate::hint::must_use", "Argument[0]", "ReturnValue", "value", "manual"]
# Iterator
- ["lang:core", "<[_]>::iter", "Argument[Self].Element", "ReturnValue.Element", "value", "manual"]
- ["lang:core", "<[_]>::iter_mut", "Argument[Self].Element", "ReturnValue.Element", "value", "manual"]
- ["lang:core", "<[_]>::into_iter", "Argument[Self].Element", "ReturnValue.Element", "value", "manual"]
- ["lang:core", "crate::iter::traits::iterator::Iterator::nth", "Argument[self].Element", "ReturnValue.Field[crate::option::Option::Some(0)]", "value", "manual"]
- ["lang:core", "crate::iter::traits::iterator::Iterator::next", "Argument[self].Element", "ReturnValue.Field[crate::option::Option::Some(0)]", "value", "manual"]
- ["lang:core", "crate::iter::traits::iterator::Iterator::collect", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
- ["lang:core", "crate::iter::traits::iterator::Iterator::map", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
- ["lang:core", "crate::iter::traits::iterator::Iterator::for_each", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
- ["lang:core", "<crate::slice::iter::Iter as crate::iter::traits::iterator::Iterator>::nth", "Argument[self].Element", "ReturnValue.Field[crate::option::Option::Some(0)]", "value", "manual"]
- ["lang:core", "<crate::slice::iter::Iter as crate::iter::traits::iterator::Iterator>::next", "Argument[self].Element", "ReturnValue.Field[crate::option::Option::Some(0)]", "value", "manual"]
- ["lang:core", "<crate::slice::iter::Iter as crate::iter::traits::iterator::Iterator>::collect", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
- ["lang:core", "<crate::slice::iter::Iter as crate::iter::traits::iterator::Iterator>::map", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
- ["lang:core", "<crate::slice::iter::Iter as crate::iter::traits::iterator::Iterator>::for_each", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
# Option
- ["lang:core", "<crate::option::Option>::expect", "Argument[self].Field[crate::option::Option::Some(0)]", "ReturnValue", "value", "manual"]
# Result
Expand All @@ -24,6 +35,9 @@ extensions:
- ["lang:core", "<crate::result::Result>::unwrap_err_unchecked", "Argument[self].Field[crate::result::Result::Err(0)]", "ReturnValue", "value", "manual"]
- ["lang:core", "<crate::result::Result>::expect", "Argument[self].Field[crate::result::Result::Ok(0)]", "ReturnValue", "value", "manual"]
- ["lang:core", "<crate::result::Result>::expect_err", "Argument[self].Field[crate::result::Result::Err(0)]", "ReturnValue", "value", "manual"]
# Str
- ["lang:core", "<str>::parse", "Argument[self]", "ReturnValue.Field[crate::result::Result::Ok(0)]", "taint", "manual"]
# String
- ["lang:alloc", "<crate::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "manual"]
- ["lang:alloc", "<crate::string::String>::as_bytes", "Argument[self]", "ReturnValue", "taint", "manual"]
- ["lang:alloc", "<_ as crate::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
202 changes: 172 additions & 30 deletions rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected

Large diffs are not rendered by default.

63 changes: 43 additions & 20 deletions rust/ql/test/library-tests/dataflow/local/inline-flow.expected
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,23 @@ edges
| main.rs:421:10:421:16 | mut_arr [element] | main.rs:421:10:421:19 | mut_arr[0] | provenance | |
| main.rs:444:9:444:9 | s | main.rs:445:10:445:10 | s | provenance | |
| main.rs:444:25:444:26 | source(...) | main.rs:444:9:444:9 | s | provenance | |
| main.rs:449:9:449:9 | a | main.rs:454:10:454:10 | a | provenance | |
| main.rs:449:13:449:22 | source(...) | main.rs:449:9:449:9 | a | provenance | |
| main.rs:451:9:451:9 | c | main.rs:452:18:452:18 | c | provenance | |
| main.rs:451:13:451:22 | source(...) | main.rs:451:9:451:9 | c | provenance | |
| main.rs:452:9:452:13 | c_ref [&ref] | main.rs:457:11:457:15 | c_ref [&ref] | provenance | |
| main.rs:452:17:452:18 | &c [&ref] | main.rs:452:9:452:13 | c_ref [&ref] | provenance | |
| main.rs:452:18:452:18 | c | main.rs:452:17:452:18 | &c [&ref] | provenance | |
| main.rs:457:11:457:15 | c_ref [&ref] | main.rs:457:10:457:15 | * ... | provenance | |
| main.rs:453:9:453:9 | a | main.rs:458:10:458:10 | a | provenance | |
| main.rs:453:13:453:22 | source(...) | main.rs:453:9:453:9 | a | provenance | |
| main.rs:465:9:465:10 | vs [element] | main.rs:467:10:467:11 | vs [element] | provenance | |
| main.rs:465:9:465:10 | vs [element] | main.rs:471:14:471:15 | vs [element] | provenance | |
| main.rs:465:14:465:34 | [...] [element] | main.rs:465:9:465:10 | vs [element] | provenance | |
| main.rs:465:15:465:24 | source(...) | main.rs:465:14:465:34 | [...] [element] | provenance | |
| main.rs:467:10:467:11 | vs [element] | main.rs:467:10:467:14 | vs[0] | provenance | |
| main.rs:471:9:471:9 | v | main.rs:472:14:472:14 | v | provenance | |
| main.rs:471:14:471:15 | vs [element] | main.rs:471:9:471:9 | v | provenance | |
| main.rs:502:9:502:9 | a | main.rs:507:10:507:10 | a | provenance | |
| main.rs:502:13:502:22 | source(...) | main.rs:502:9:502:9 | a | provenance | |
| main.rs:504:9:504:9 | c | main.rs:505:18:505:18 | c | provenance | |
| main.rs:504:13:504:22 | source(...) | main.rs:504:9:504:9 | c | provenance | |
| main.rs:505:9:505:13 | c_ref [&ref] | main.rs:510:11:510:15 | c_ref [&ref] | provenance | |
| main.rs:505:17:505:18 | &c [&ref] | main.rs:505:9:505:13 | c_ref [&ref] | provenance | |
| main.rs:505:18:505:18 | c | main.rs:505:17:505:18 | &c [&ref] | provenance | |
| main.rs:510:11:510:15 | c_ref [&ref] | main.rs:510:10:510:15 | * ... | provenance | |
nodes
| main.rs:18:10:18:18 | source(...) | semmle.label | source(...) |
| main.rs:22:9:22:9 | s | semmle.label | s |
Expand Down Expand Up @@ -411,16 +420,27 @@ nodes
| main.rs:444:9:444:9 | s | semmle.label | s |
| main.rs:444:25:444:26 | source(...) | semmle.label | source(...) |
| main.rs:445:10:445:10 | s | semmle.label | s |
| main.rs:449:9:449:9 | a | semmle.label | a |
| main.rs:449:13:449:22 | source(...) | semmle.label | source(...) |
| main.rs:451:9:451:9 | c | semmle.label | c |
| main.rs:451:13:451:22 | source(...) | semmle.label | source(...) |
| main.rs:452:9:452:13 | c_ref [&ref] | semmle.label | c_ref [&ref] |
| main.rs:452:17:452:18 | &c [&ref] | semmle.label | &c [&ref] |
| main.rs:452:18:452:18 | c | semmle.label | c |
| main.rs:454:10:454:10 | a | semmle.label | a |
| main.rs:457:10:457:15 | * ... | semmle.label | * ... |
| main.rs:457:11:457:15 | c_ref [&ref] | semmle.label | c_ref [&ref] |
| main.rs:453:9:453:9 | a | semmle.label | a |
| main.rs:453:13:453:22 | source(...) | semmle.label | source(...) |
| main.rs:458:10:458:10 | a | semmle.label | a |
| main.rs:465:9:465:10 | vs [element] | semmle.label | vs [element] |
| main.rs:465:14:465:34 | [...] [element] | semmle.label | [...] [element] |
| main.rs:465:15:465:24 | source(...) | semmle.label | source(...) |
| main.rs:467:10:467:11 | vs [element] | semmle.label | vs [element] |
| main.rs:467:10:467:14 | vs[0] | semmle.label | vs[0] |
| main.rs:471:9:471:9 | v | semmle.label | v |
| main.rs:471:14:471:15 | vs [element] | semmle.label | vs [element] |
| main.rs:472:14:472:14 | v | semmle.label | v |
| main.rs:502:9:502:9 | a | semmle.label | a |
| main.rs:502:13:502:22 | source(...) | semmle.label | source(...) |
| main.rs:504:9:504:9 | c | semmle.label | c |
| main.rs:504:13:504:22 | source(...) | semmle.label | source(...) |
| main.rs:505:9:505:13 | c_ref [&ref] | semmle.label | c_ref [&ref] |
| main.rs:505:17:505:18 | &c [&ref] | semmle.label | &c [&ref] |
| main.rs:505:18:505:18 | c | semmle.label | c |
| main.rs:507:10:507:10 | a | semmle.label | a |
| main.rs:510:10:510:15 | * ... | semmle.label | * ... |
| main.rs:510:11:510:15 | c_ref [&ref] | semmle.label | c_ref [&ref] |
subpaths
testFailures
#select
Expand Down Expand Up @@ -466,5 +486,8 @@ testFailures
| main.rs:420:10:420:10 | d | main.rs:418:18:418:27 | source(...) | main.rs:420:10:420:10 | d | $@ | main.rs:418:18:418:27 | source(...) | source(...) |
| main.rs:421:10:421:19 | mut_arr[0] | main.rs:418:18:418:27 | source(...) | main.rs:421:10:421:19 | mut_arr[0] | $@ | main.rs:418:18:418:27 | source(...) | source(...) |
| main.rs:445:10:445:10 | s | main.rs:444:25:444:26 | source(...) | main.rs:445:10:445:10 | s | $@ | main.rs:444:25:444:26 | source(...) | source(...) |
| main.rs:454:10:454:10 | a | main.rs:449:13:449:22 | source(...) | main.rs:454:10:454:10 | a | $@ | main.rs:449:13:449:22 | source(...) | source(...) |
| main.rs:457:10:457:15 | * ... | main.rs:451:13:451:22 | source(...) | main.rs:457:10:457:15 | * ... | $@ | main.rs:451:13:451:22 | source(...) | source(...) |
| main.rs:458:10:458:10 | a | main.rs:453:13:453:22 | source(...) | main.rs:458:10:458:10 | a | $@ | main.rs:453:13:453:22 | source(...) | source(...) |
| main.rs:467:10:467:14 | vs[0] | main.rs:465:15:465:24 | source(...) | main.rs:467:10:467:14 | vs[0] | $@ | main.rs:465:15:465:24 | source(...) | source(...) |
| main.rs:472:14:472:14 | v | main.rs:465:15:465:24 | source(...) | main.rs:472:14:472:14 | v | $@ | main.rs:465:15:465:24 | source(...) | source(...) |
| main.rs:507:10:507:10 | a | main.rs:502:13:502:22 | source(...) | main.rs:507:10:507:10 | a | $@ | main.rs:502:13:502:22 | source(...) | source(...) |
| main.rs:510:10:510:15 | * ... | main.rs:504:13:504:22 | source(...) | main.rs:510:10:510:15 | * ... | $@ | main.rs:504:13:504:22 | source(...) | source(...) |
55 changes: 55 additions & 0 deletions rust/ql/test/library-tests/dataflow/local/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,59 @@ fn macro_invocation() {
sink(s); // $ hasValueFlow=37
}

fn sink_string(s: String) {
println!("{}", s);
}

fn parse() {
let a = source(90);
let b = a.to_string();
let c = b.parse::<i64>().unwrap();
let d : i64 = b.parse().unwrap();

sink(a); // $ hasValueFlow=90
sink_string(b); // $ hasTaintFlow=90
sink(c); // $ hasTaintFlow=90
sink(d); // $ hasTaintFlow=90
}

fn iterators() {
let vs = [source(91), 2, 3, 4];

sink(vs[0]); // $ hasValueFlow=91
sink(*vs.iter().next().unwrap()); // $ MISSING: hasValueFlow=91
sink(*vs.iter().nth(0).unwrap()); // $ MISSING: hasValueFlow=91

for v in vs {
sink(v); // $ hasValueFlow=91
}
for &v in vs.iter() {
sink(v); // $ MISSING: hasValueFlow=91
}

let vs2 : Vec<&i64> = vs.iter().collect();
for &v in vs2 {
sink(v); // $ MISSING: hasValueFlow=91
}

vs.iter().map(|x| sink(*x)); // $ MISSING: hasValueFlow=91
vs.iter().for_each(|x| sink(*x)); // $ MISSING: hasValueFlow=91

for v in vs.into_iter() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this call to into_iter() (from the IntoIter trait), we don't seem to have a result for resolvable_resolved_paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hvitved would you mind taking a quick look?

Copy link
Contributor

Choose a reason for hiding this comment

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

resolvable_resolved_paths is populated by the extractor, so I think @redsun82 or @aibaars can better explain this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've created an issue to track this (to separate this concern from merging the PR). I'm keen to hear what people think we need to do.

sink(v); // $ MISSING: hasValueFlow=91
}

let mut vs_mut = [source(92), 2, 3, 4];

sink(vs_mut[0]); // $ MISSING: hasValueFlow=92
sink(*vs_mut.iter().next().unwrap()); // $ MISSING: hasValueFlow=92
sink(*vs_mut.iter().nth(0).unwrap()); // $ MISSING: hasValueFlow=92

for &mut v in vs_mut.iter_mut() {
sink(v); // $ MISSING: hasValueFlow=92
}
}

fn references() {
let a = source(40);
let b = source(41);
Expand Down Expand Up @@ -494,5 +547,7 @@ fn main() {
array_assignment();
captured_variable_and_continue(vec![]);
macro_invocation();
parse();
iterators();
references();
}
31 changes: 16 additions & 15 deletions rust/ql/test/library-tests/dataflow/sources/TaintSources.expected
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@
| test.rs:29:29:29:42 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:32:16:32:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:33:16:33:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:40:16:40:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:44:16:44:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:50:15:50:35 | ...::current_dir | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:51:15:51:35 | ...::current_exe | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:52:16:52:33 | ...::home_dir | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:60:26:60:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:63:26:63:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:66:26:66:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:69:26:69:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:72:26:72:37 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:75:26:75:37 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:78:24:78:35 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:110:35:110:46 | send_request | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:117:31:117:42 | send_request | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:201:16:201:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:34:16:34:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:42:16:42:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:46:16:46:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:52:15:52:35 | ...::current_dir | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:53:15:53:35 | ...::current_exe | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:54:16:54:33 | ...::home_dir | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:62:26:62:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:65:26:65:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:68:26:68:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:71:26:71:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:74:26:74:37 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:77:26:77:37 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:80:24:80:35 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:112:35:112:46 | send_request | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:119:31:119:42 | send_request | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:203:16:203:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. |
2 changes: 2 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ fn test_env_args() {
let arg1 = &args[1];
let arg2 = std::env::args().nth(2).unwrap(); // $ Alert[rust/summary/taint-sources]
let arg3 = std::env::args_os().nth(3).unwrap(); // $ Alert[rust/summary/taint-sources]
let arg4 = std::env::args().nth(4).unwrap().parse::<usize>().unwrap(); // $ Alert[rust/summary/taint-sources]

sink(my_path); // $ hasTaintFlow
sink(arg1); // $ hasTaintFlow
sink(arg2); // $ hasTaintFlow
sink(arg3); // $ hasTaintFlow
sink(arg4); // $ hasTaintFlow

for arg in std::env::args() { // $ Alert[rust/summary/taint-sources]
sink(arg); // $ hasTaintFlow
Expand Down
Loading