Skip to content

Commit 0d19fb6

Browse files
paldepindpaldepind
committed
Rust: Add taint from children of format_args to format_args
1 parent 2ef9339 commit 0d19fb6

File tree

4 files changed

+67
-3
lines changed

4 files changed

+67
-3
lines changed

‎rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,32 @@ final class MethodCallExprCfgNode extends CallExprBaseCfgNode, Nodes::MethodCall
173173
*/
174174
final class CallExprCfgNode extends CallExprBaseCfgNode, Nodes::CallExprCfgNode { }
175175

176+
/**
177+
* A FormatArgsExpr. For example:
178+
* ```rust
179+
* format_args!("no args");
180+
* format_args!("{} foo {:?}", 1, 2);
181+
* format_args!("{b} foo {a:?}", a=1, b=2);
182+
* let (x, y) = (1, 42);
183+
* format_args!("{x}, {y}");
184+
* ```
185+
*/
186+
final class FormatArgsExprCfgNode extends Nodes::FormatArgsExprCfgNode {
187+
private FormatArgsExprChildMapping node;
188+
189+
ExprCfgNode getArgumentExpr(int i) {
190+
any(ChildMapping mapping).hasCfgChild(node, node.getArg(i).getExpr(), this, result)
191+
}
192+
193+
FormatTemplateVariableAccessCfgNode getFormatTemplateVariableAccess(int i) {
194+
exists(FormatTemplateVariableAccess v |
195+
v.getArgument() = node.getFormat(i).getArgument() and
196+
result.getFormatTemplateVariableAccess() = v and
197+
any(ChildMapping mapping).hasCfgChild(node, v, this, result)
198+
)
199+
}
200+
}
201+
176202
final class MacroCallCfgNode extends Nodes::MacroCallCfgNode {
177203
private MacroCallChildMapping node;
178204

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
4646
RustDataFlow::readStep(pred, cs, succ) and
4747
cs.getContent() instanceof ArrayElementContent
4848
)
49+
or
50+
exists(FormatArgsExprCfgNode format | succ.asExpr() = format |
51+
pred.asExpr() = [format.getArgumentExpr(_), format.getFormatTemplateVariableAccess(_)]
52+
)
4953
)
5054
or
5155
FlowSummaryImpl::Private::Steps::summaryLocalStep(pred.(Node::FlowSummaryNode).getSummaryNode(),

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
models
22
| 1 | Summary: lang:alloc; <crate::string::String>::as_str; Argument[self]; ReturnValue; taint |
3+
| 2 | Summary: lang:alloc; crate::fmt::format; Argument[0]; ReturnValue; taint |
34
edges
45
| main.rs:26:9:26:9 | s | main.rs:27:19:27:25 | s[...] | provenance | |
56
| main.rs:26:13:26:22 | source(...) | main.rs:26:9:26:9 | s | provenance | |
@@ -12,6 +13,20 @@ edges
1213
| main.rs:63:9:63:9 | s | main.rs:64:16:64:16 | s | provenance | |
1314
| main.rs:63:13:63:22 | source(...) | main.rs:63:9:63:9 | s | provenance | |
1415
| main.rs:64:16:64:16 | s | main.rs:64:16:64:25 | s.as_str(...) | provenance | MaD:1 |
16+
| main.rs:68:9:68:9 | s | main.rs:70:34:70:61 | MacroExpr | provenance | |
17+
| main.rs:68:9:68:9 | s | main.rs:73:34:73:59 | MacroExpr | provenance | |
18+
| main.rs:68:13:68:22 | source(...) | main.rs:68:9:68:9 | s | provenance | |
19+
| main.rs:70:9:70:18 | formatted1 | main.rs:71:10:71:19 | formatted1 | provenance | |
20+
| main.rs:70:22:70:62 | ...::format(...) | main.rs:70:9:70:18 | formatted1 | provenance | |
21+
| main.rs:70:34:70:61 | MacroExpr | main.rs:70:22:70:62 | ...::format(...) | provenance | MaD:2 |
22+
| main.rs:73:9:73:18 | formatted2 | main.rs:74:10:74:19 | formatted2 | provenance | |
23+
| main.rs:73:22:73:60 | ...::format(...) | main.rs:73:9:73:18 | formatted2 | provenance | |
24+
| main.rs:73:34:73:59 | MacroExpr | main.rs:73:22:73:60 | ...::format(...) | provenance | MaD:2 |
25+
| main.rs:76:9:76:13 | width | main.rs:77:34:77:74 | MacroExpr | provenance | |
26+
| main.rs:76:17:76:32 | source_usize(...) | main.rs:76:9:76:13 | width | provenance | |
27+
| main.rs:77:9:77:18 | formatted3 | main.rs:78:10:78:19 | formatted3 | provenance | |
28+
| main.rs:77:22:77:75 | ...::format(...) | main.rs:77:9:77:18 | formatted3 | provenance | |
29+
| main.rs:77:34:77:74 | MacroExpr | main.rs:77:22:77:75 | ...::format(...) | provenance | MaD:2 |
1530
nodes
1631
| main.rs:26:9:26:9 | s | semmle.label | s |
1732
| main.rs:26:13:26:22 | source(...) | semmle.label | source(...) |
@@ -27,9 +42,28 @@ nodes
2742
| main.rs:63:13:63:22 | source(...) | semmle.label | source(...) |
2843
| main.rs:64:16:64:16 | s | semmle.label | s |
2944
| main.rs:64:16:64:25 | s.as_str(...) | semmle.label | s.as_str(...) |
45+
| main.rs:68:9:68:9 | s | semmle.label | s |
46+
| main.rs:68:13:68:22 | source(...) | semmle.label | source(...) |
47+
| main.rs:70:9:70:18 | formatted1 | semmle.label | formatted1 |
48+
| main.rs:70:22:70:62 | ...::format(...) | semmle.label | ...::format(...) |
49+
| main.rs:70:34:70:61 | MacroExpr | semmle.label | MacroExpr |
50+
| main.rs:71:10:71:19 | formatted1 | semmle.label | formatted1 |
51+
| main.rs:73:9:73:18 | formatted2 | semmle.label | formatted2 |
52+
| main.rs:73:22:73:60 | ...::format(...) | semmle.label | ...::format(...) |
53+
| main.rs:73:34:73:59 | MacroExpr | semmle.label | MacroExpr |
54+
| main.rs:74:10:74:19 | formatted2 | semmle.label | formatted2 |
55+
| main.rs:76:9:76:13 | width | semmle.label | width |
56+
| main.rs:76:17:76:32 | source_usize(...) | semmle.label | source_usize(...) |
57+
| main.rs:77:9:77:18 | formatted3 | semmle.label | formatted3 |
58+
| main.rs:77:22:77:75 | ...::format(...) | semmle.label | ...::format(...) |
59+
| main.rs:77:34:77:74 | MacroExpr | semmle.label | MacroExpr |
60+
| main.rs:78:10:78:19 | formatted3 | semmle.label | formatted3 |
3061
subpaths
3162
testFailures
3263
#select
3364
| 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(...) |
3465
| 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(...) |
3566
| 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(...) |
67+
| 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(...) |
68+
| main.rs:74:10:74:19 | formatted2 | main.rs:68:13:68:22 | source(...) | main.rs:74:10:74:19 | formatted2 | $@ | main.rs:68:13:68:22 | source(...) | source(...) |
69+
| main.rs:78:10:78:19 | formatted3 | main.rs:76:17:76:32 | source_usize(...) | main.rs:78:10:78:19 | formatted3 | $@ | main.rs:76:17:76:32 | source_usize(...) | source_usize(...) |

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,14 @@ fn format_args_built_in() {
6868
let s = source(88);
6969

7070
let formatted1 = fmt::format(format_args!("Hello {}!", s));
71-
sink(formatted1); // $ MISSING: hasTaintFlow=88
71+
sink(formatted1); // $ hasTaintFlow=88
7272

7373
let formatted2 = fmt::format(format_args!("Hello {s}!"));
74-
sink(formatted2); // $ MISSING: hasTaintFlow=88
74+
sink(formatted2); // $ hasTaintFlow=88
7575

7676
let width = source_usize(10);
7777
let formatted3 = fmt::format(format_args!("Hello {:width$}!", "World"));
78-
sink(formatted3); // $ MISSING: hasTaintFlow=10
78+
sink(formatted3); // $ hasTaintFlow=10
7979
}
8080

8181
fn format_macro() {

0 commit comments

Comments
 (0)