Skip to content

Rust: Add taint model for add on String #20559

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

Merged
paldepind merged 2 commits into from
Oct 1, 2025
Merged

Rust: Add taint model for add on String #20559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49efd57
Rust: Add taint model for add on `String`
paldepind Sep 30, 2025
19871a2
Rust: Accept test changes
paldepind Sep 30, 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: 2 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/lang-alloc.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,7 @@ extensions:
- ["<alloc::string::String as alloc::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
- ["<core::str>::parse", "Argument[self]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
- ["<core::str>::trim", "Argument[self]", "ReturnValue.Reference", "taint", "manual"]
- ["<alloc::string::String as core::ops::arith::Add>::add", "Argument[self]", "ReturnValue", "taint", "manual"]
- ["<alloc::string::String as core::ops::arith::Add>::add", "Argument[0].Reference", "ReturnValue", "taint", "manual"]
Comment on lines +49 to +50
Copy link
Contributor

@hvitved hvitved Oct 1, 2025

Choose a reason for hiding this comment

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

Should we instead lift this to <_ as core::ops::arith::Add>::add?

Copy link
Contributor

@geoffw0 geoffw0 Oct 1, 2025

Choose a reason for hiding this comment

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

If we do this as follow-up, we can include other similar traits (e.g. Sub) as well.

Copy link
Contributor

@geoffw0 geoffw0 Oct 1, 2025

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 for this.

# Vec
- ["alloc::vec::from_elem", "Argument[0]", "ReturnValue.Element", "value", "manual"]
41 changes: 26 additions & 15 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,11 +1,12 @@
models
| 1 | Summary: <_ as core::convert::From>::from; Argument[0]; ReturnValue; taint |
| 2 | Summary: <alloc::string::String as core::convert::From>::from; Argument[0].Reference; ReturnValue; value |
| 3 | Summary: <alloc::string::String as core::ops::arith::Add>::add; Argument[self]; ReturnValue; value |
| 4 | Summary: <alloc::string::String>::as_str; Argument[self]; ReturnValue; value |
| 5 | Summary: <core::str>::as_str; Argument[self]; ReturnValue; value |
| 6 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
| 7 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
| 3 | Summary: <alloc::string::String as core::ops::arith::Add>::add; Argument[0].Reference; ReturnValue; taint |
| 4 | Summary: <alloc::string::String as core::ops::arith::Add>::add; Argument[self]; ReturnValue; value |
| 5 | Summary: <alloc::string::String>::as_str; Argument[self]; ReturnValue; value |
| 6 | Summary: <core::str>::as_str; Argument[self]; ReturnValue; value |
| 7 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
| 8 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
edges
| 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 | |
Expand All @@ -16,47 +17,51 @@ edges
| 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:3 |
| main.rs:35:14:35:15 | s1 | main.rs:35:14:35:20 | ... + ... | provenance | MaD:4 |
| 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: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:1 |
| main.rs:52:27:52:28 | s1 | main.rs:52:14:52:29 | ...::from(...) | provenance | MaD:2 |
| main.rs:63:9:63:9 | s | main.rs:64:16:64:16 | s | provenance | |
| main.rs:63:9:63:9 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:4 |
| main.rs:63:9:63:9 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:5 |
| main.rs:63:9:63:9 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:6 |
| 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:4 |
| main.rs:64:16:64:16 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:5 |
| main.rs:64:16:64:16 | s | main.rs:64:16:64:25 | s.as_str() | provenance | MaD:6 |
| 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:6 |
| main.rs:70:34:70:61 | MacroExpr | main.rs:70:22:70:62 | ...::format(...) | provenance | MaD:7 |
| 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:6 |
| main.rs:73:34:73:59 | MacroExpr | main.rs:73:22:73:60 | ...::format(...) | provenance | MaD:7 |
| 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:6 |
| main.rs:77:34:77:74 | MacroExpr | main.rs:77:22:77:75 | ...::format(...) | provenance | MaD:7 |
| 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:10:86:16 | res | main.rs:86:18:86:25 | { ... } | provenance | |
| main.rs:86:18:86:25 | ...::format(...) | main.rs:86:10:86:16 | res | 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:6 |
| main.rs:86:18:86:25 | { ... } | main.rs:86:18:86:25 | ...::must_use(...) | provenance | MaD:7 |
| main.rs:86:18:86:25 | MacroExpr | main.rs:86:18:86:25 | ...::format(...) | provenance | MaD:7 |
| main.rs:86:18:86:25 | { ... } | main.rs:86:18:86:25 | ...::must_use(...) | provenance | MaD:8 |
| main.rs:87:10:87:16 | res | main.rs:87:18:87:32 | { ... } | provenance | |
| main.rs:87:18:87:32 | ...::format(...) | main.rs:87:10:87:16 | res | 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:6 |
| main.rs:87:18:87:32 | { ... } | main.rs:87:18:87:32 | ...::must_use(...) | provenance | MaD:7 |
| main.rs:87:18:87:32 | MacroExpr | main.rs:87:18:87:32 | ...::format(...) | provenance | MaD:7 |
| main.rs:87:18:87:32 | { ... } | main.rs:87:18:87:32 | ...::must_use(...) | provenance | MaD:8 |
nodes
| main.rs:26:9:26:9 | s | semmle.label | s |
| main.rs:26:13:26:22 | source(...) | semmle.label | source(...) |
Expand All @@ -70,6 +75,11 @@ nodes
| main.rs:35:14:35:15 | s1 | semmle.label | s1 |
| main.rs:35:14:35:20 | ... + ... | semmle.label | ... + ... |
| main.rs:38:10:38:11 | s4 | semmle.label | s4 |
| main.rs:43:9:43:10 | s1 | semmle.label | s1 |
| main.rs:43:14:43:23 | source(...) | semmle.label | source(...) |
| main.rs:46:10:46:35 | ... + ... | semmle.label | ... + ... |
| main.rs:46:33:46:35 | &s1 [&ref] | semmle.label | &s1 [&ref] |
| main.rs:46:34:46:35 | s1 | semmle.label | s1 |
| main.rs:51:9:51:10 | s1 | semmle.label | s1 |
| main.rs:51:14:51:29 | source_slice(...) | semmle.label | source_slice(...) |
| main.rs:52:9:52:10 | s2 | semmle.label | s2 |
Expand Down Expand Up @@ -115,6 +125,7 @@ testFailures
#select
| main.rs:28:16:28:21 | sliced | main.rs:26:13:26:22 | source(...) | main.rs:28:16:28:21 | sliced | $@ | main.rs:26:13:26:22 | source(...) | source(...) |
| main.rs:38:10:38:11 | s4 | main.rs:32:14:32:23 | source(...) | main.rs:38:10:38:11 | s4 | $@ | main.rs:32:14:32:23 | source(...) | source(...) |
| main.rs:46:10:46:35 | ... + ... | main.rs:43:14:43:23 | source(...) | main.rs:46:10:46:35 | ... + ... | $@ | main.rs:43:14:43:23 | source(...) | source(...) |
| main.rs:53:10:53:11 | s2 | main.rs:51:14:51:29 | source_slice(...) | main.rs:53:10:53:11 | s2 | $@ | main.rs:51:14:51:29 | source_slice(...) | source_slice(...) |
| main.rs:64:16:64:25 | s.as_str() | main.rs:63:13:63:22 | source(...) | main.rs:64:16:64:25 | s.as_str() | $@ | main.rs:63:13:63:22 | source(...) | source(...) |
| main.rs:71:10:71:19 | formatted1 | main.rs:68:13:68:22 | source(...) | main.rs:71:10:71:19 | formatted1 | $@ | main.rs:68:13:68:22 | source(...) | source(...) |
Expand Down
4 changes: 2 additions & 2 deletions 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 @@ -43,7 +43,7 @@ fn string_add_reference() {
let s1 = source(37);
let s2 = "1".to_string();

sink("Hello ".to_string() + &s1); // $ MISSING: hasTaintFlow=37
sink("Hello ".to_string() + &s1); // $ hasTaintFlow=37
sink("Hello ".to_string() + &s2);
}

Expand All @@ -56,7 +56,7 @@ fn string_from() {
fn string_to_string() {
let s1 = source_slice(22);
let s2 = s1.to_string();
sink(s2); // $ MISSING: hasTaintFlow=22 - we are not currently able to resolve the `to_string` call above, which comes from `impl<T: fmt::Display + ?Sized> ToString for T`
sink(s2); // $ MISSING: hasTaintFlow=22
Copy link
Contributor Author

@paldepind paldepind Sep 30, 2025

Choose a reason for hiding this comment

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

The explanation here was outdated. We actually resolve the call now, but we can't model the method in a blanket implementation.

}

fn as_str() {
Expand Down
Loading
Loading