diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index def578053733..c31ef9e81072 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -112,7 +112,7 @@ module Node { } /** A data flow node that corresponds to a CFG node for an AST node. */ - abstract private class AstCfgFlowNode extends Node { + abstract class AstCfgFlowNode extends Node { AstCfgNode n; override CfgNode getCfgNode() { result = n } @@ -145,7 +145,7 @@ module Node { PatNode() { this = TPatNode(n) } - /** Gets the Pat in the AST that this node corresponds to. */ + /** Gets the `Pat` in the AST that this node corresponds to. */ Pat getPat() { result = n.getPat() } } @@ -282,6 +282,12 @@ module LocalFlow { nodeFrom.getCfgNode().getAstNode() = s.getInitializer() and nodeTo.getCfgNode().getAstNode() = s.getPat() ) + or + // An edge from a pattern/expression to its corresponding SSA definition. + nodeFrom.(Node::AstCfgFlowNode).getCfgNode() = + nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode() + or + SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _) } } @@ -395,7 +401,10 @@ module RustDataFlow implements InputSig { * Holds if there is a simple local flow step from `node1` to `node2`. These * are the value-preserving intra-callable flow steps. */ - predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) { none() } + predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) { + LocalFlow::localFlowStepCommon(nodeFrom, nodeTo) and + model = "" + } /** * Holds if data can flow from `node1` to `node2` through a non-local step @@ -517,8 +526,6 @@ private module Cached { cached predicate localFlowStepImpl(Node::Node nodeFrom, Node::Node nodeTo) { LocalFlow::localFlowStepCommon(nodeFrom, nodeTo) - or - SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _) } } diff --git a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected index c59ea4cc6703..a54c75d0c17f 100644 --- a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected +++ b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected @@ -1,10 +1,16 @@ models edges +| main.rs:21:13:21:21 | CallExpr : unit | main.rs:22:10:22:10 | s | provenance | | +| main.rs:32:13:32:21 | CallExpr : unit | main.rs:33:10:33:10 | s | provenance | | nodes | main.rs:17:10:17:18 | CallExpr | semmle.label | CallExpr | +| main.rs:21:13:21:21 | CallExpr : unit | semmle.label | CallExpr : unit | +| main.rs:22:10:22:10 | s | semmle.label | s | +| main.rs:32:13:32:21 | CallExpr : unit | semmle.label | CallExpr : unit | +| main.rs:33:10:33:10 | s | semmle.label | s | subpaths testFailures -| main.rs:22:14:22:32 | Comment | Missing result: hasValueFlow=1 | -| main.rs:33:14:33:32 | Comment | Missing result: hasValueFlow=1 | #select | main.rs:17:10:17:18 | CallExpr | main.rs:17:10:17:18 | CallExpr | main.rs:17:10:17:18 | CallExpr | $@ | main.rs:17:10:17:18 | CallExpr | CallExpr | +| main.rs:22:10:22:10 | s | main.rs:21:13:21:21 | CallExpr : unit | main.rs:22:10:22:10 | s | $@ | main.rs:21:13:21:21 | CallExpr : unit | CallExpr : unit | +| main.rs:33:10:33:10 | s | main.rs:32:13:32:21 | CallExpr : unit | main.rs:33:10:33:10 | s | $@ | main.rs:32:13:32:21 | CallExpr : unit | CallExpr : unit | diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected index a9631f78affe..8cc12598f62b 100644 --- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected +++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected @@ -1,31 +1,85 @@ -| main.rs:2:9:2:9 | [SSA] s | main.rs:3:33:3:33 | s | -| main.rs:2:13:2:19 | "Hello" | main.rs:2:9:2:9 | s | -| main.rs:6:18:6:21 | [SSA] cond | main.rs:9:16:9:19 | cond | -| main.rs:7:9:7:9 | [SSA] a | main.rs:10:9:10:9 | a | -| main.rs:7:13:7:13 | 1 | main.rs:7:9:7:9 | a | -| main.rs:8:9:8:9 | [SSA] b | main.rs:12:9:12:9 | b | -| main.rs:8:13:8:13 | 2 | main.rs:8:9:8:9 | b | -| main.rs:9:9:9:9 | [SSA] c | main.rs:14:5:14:5 | c | -| main.rs:9:13:13:5 | IfExpr | main.rs:9:9:9:9 | c | -| main.rs:9:21:11:5 | BlockExpr | main.rs:9:13:13:5 | IfExpr | -| main.rs:10:9:10:9 | a | main.rs:9:21:11:5 | BlockExpr | -| main.rs:11:12:13:5 | BlockExpr | main.rs:9:13:13:5 | IfExpr | -| main.rs:12:9:12:9 | b | main.rs:11:12:13:5 | BlockExpr | -| main.rs:14:5:14:5 | c | main.rs:6:37:15:1 | BlockExpr | -| main.rs:18:9:18:9 | [SSA] a | main.rs:20:15:20:15 | a | -| main.rs:18:13:18:13 | 1 | main.rs:18:9:18:9 | a | -| main.rs:19:9:19:9 | [SSA] b | main.rs:22:5:22:5 | b | -| main.rs:19:13:21:5 | LoopExpr | main.rs:19:9:19:9 | b | -| main.rs:20:9:20:15 | BreakExpr | main.rs:19:13:21:5 | LoopExpr | -| main.rs:20:15:20:15 | a | main.rs:20:9:20:15 | BreakExpr | -| main.rs:22:5:22:5 | b | main.rs:17:29:23:1 | BlockExpr | -| main.rs:26:17:26:17 | 1 | main.rs:26:9:26:13 | i | -| main.rs:27:5:27:5 | [SSA] i | main.rs:28:5:28:5 | i | -| main.rs:27:5:27:5 | i | main.rs:27:5:27:5 | [SSA] i | -| main.rs:28:5:28:5 | i | main.rs:25:24:29:1 | BlockExpr | -| main.rs:31:21:31:21 | [SSA] a | main.rs:33:20:33:20 | a | -| main.rs:31:29:31:29 | [SSA] b | main.rs:34:17:34:17 | b | -| main.rs:31:37:31:37 | [SSA] c | main.rs:32:11:32:11 | c | -| main.rs:32:5:35:5 | MatchExpr | main.rs:31:60:36:1 | BlockExpr | -| main.rs:33:20:33:20 | a | main.rs:32:5:35:5 | MatchExpr | -| main.rs:34:17:34:17 | b | main.rs:32:5:35:5 | MatchExpr | +| main.rs:3:11:3:11 | [SSA] i | main.rs:4:12:4:12 | i | +| main.rs:3:11:3:11 | i | main.rs:3:11:3:11 | [SSA] i | +| main.rs:4:5:4:12 | ... + ... | main.rs:3:26:5:1 | BlockExpr | +| main.rs:7:9:7:9 | [SSA] s | main.rs:8:20:8:20 | s | +| main.rs:7:9:7:9 | s | main.rs:7:9:7:9 | [SSA] s | +| main.rs:19:9:19:9 | [SSA] s | main.rs:20:10:20:10 | s | +| main.rs:19:9:19:9 | s | main.rs:19:9:19:9 | [SSA] s | +| main.rs:19:13:19:21 | CallExpr | main.rs:19:9:19:9 | s | +| main.rs:23:18:23:21 | [SSA] cond | main.rs:26:16:26:19 | cond | +| main.rs:23:18:23:21 | cond | main.rs:23:18:23:21 | [SSA] cond | +| main.rs:24:9:24:9 | [SSA] a | main.rs:26:23:26:23 | a | +| main.rs:24:9:24:9 | a | main.rs:24:9:24:9 | [SSA] a | +| main.rs:24:13:24:21 | CallExpr | main.rs:24:9:24:9 | a | +| main.rs:25:9:25:9 | [SSA] b | main.rs:26:34:26:34 | b | +| main.rs:25:9:25:9 | b | main.rs:25:9:25:9 | [SSA] b | +| main.rs:25:13:25:13 | 2 | main.rs:25:9:25:9 | b | +| main.rs:26:9:26:9 | [SSA] c | main.rs:27:10:27:10 | c | +| main.rs:26:9:26:9 | c | main.rs:26:9:26:9 | [SSA] c | +| main.rs:26:13:26:36 | IfExpr | main.rs:26:9:26:9 | c | +| main.rs:26:21:26:25 | BlockExpr | main.rs:26:13:26:36 | IfExpr | +| main.rs:26:23:26:23 | a | main.rs:26:21:26:25 | BlockExpr | +| main.rs:26:32:26:36 | BlockExpr | main.rs:26:13:26:36 | IfExpr | +| main.rs:26:34:26:34 | b | main.rs:26:32:26:36 | BlockExpr | +| main.rs:30:21:30:21 | [SSA] m | main.rs:32:19:32:19 | m | +| main.rs:30:21:30:21 | m | main.rs:30:21:30:21 | [SSA] m | +| main.rs:31:9:31:9 | [SSA] a | main.rs:33:20:33:20 | a | +| main.rs:31:9:31:9 | a | main.rs:31:9:31:9 | [SSA] a | +| main.rs:31:13:31:21 | CallExpr | main.rs:31:9:31:9 | a | +| main.rs:32:9:32:9 | [SSA] b | main.rs:36:10:36:10 | b | +| main.rs:32:9:32:9 | b | main.rs:32:9:32:9 | [SSA] b | +| main.rs:32:13:35:5 | MatchExpr | main.rs:32:9:32:9 | b | +| main.rs:33:20:33:20 | a | main.rs:32:13:35:5 | MatchExpr | +| main.rs:34:17:34:17 | 0 | main.rs:32:13:35:5 | MatchExpr | +| main.rs:40:9:40:9 | [SSA] a | main.rs:43:10:43:10 | a | +| main.rs:40:9:40:9 | a | main.rs:40:9:40:9 | [SSA] a | +| main.rs:40:13:42:5 | LoopExpr | main.rs:40:9:40:9 | a | +| main.rs:41:9:41:15 | BreakExpr | main.rs:40:13:42:5 | LoopExpr | +| main.rs:41:15:41:15 | 1 | main.rs:41:9:41:15 | BreakExpr | +| main.rs:44:9:44:9 | [SSA] b | main.rs:47:10:47:10 | b | +| main.rs:44:9:44:9 | b | main.rs:44:9:44:9 | [SSA] b | +| main.rs:44:13:46:5 | LoopExpr | main.rs:44:9:44:9 | b | +| main.rs:45:9:45:23 | BreakExpr | main.rs:44:13:46:5 | LoopExpr | +| main.rs:45:15:45:23 | CallExpr | main.rs:45:9:45:23 | BreakExpr | +| main.rs:51:9:51:13 | [SSA] i | main.rs:52:10:52:10 | i | +| main.rs:51:9:51:13 | i | main.rs:51:9:51:13 | [SSA] i | +| main.rs:51:17:51:17 | 1 | main.rs:51:9:51:13 | i | +| main.rs:53:5:53:5 | [SSA] i | main.rs:54:10:54:10 | i | +| main.rs:53:5:53:5 | i | main.rs:53:5:53:5 | [SSA] i | +| main.rs:61:9:61:9 | [SSA] i | main.rs:62:11:62:11 | i | +| main.rs:61:9:61:9 | i | main.rs:61:9:61:9 | [SSA] i | +| main.rs:61:13:61:31 | CallExpr | main.rs:61:9:61:9 | i | +| main.rs:66:9:66:9 | [SSA] a | main.rs:67:10:67:10 | a | +| main.rs:66:9:66:9 | a | main.rs:66:9:66:9 | [SSA] a | +| main.rs:66:13:66:26 | TupleExpr | main.rs:66:9:66:9 | a | +| main.rs:67:10:67:10 | a | main.rs:68:10:68:10 | a | +| main.rs:78:9:78:9 | [SSA] p | main.rs:83:10:83:10 | p | +| main.rs:78:9:78:9 | p | main.rs:78:9:78:9 | [SSA] p | +| main.rs:78:13:82:5 | RecordExpr | main.rs:78:9:78:9 | p | +| main.rs:83:10:83:10 | p | main.rs:84:10:84:10 | p | +| main.rs:84:10:84:10 | p | main.rs:85:10:85:10 | p | +| main.rs:92:9:92:9 | [SSA] p | main.rs:97:38:97:38 | p | +| main.rs:92:9:92:9 | p | main.rs:92:9:92:9 | [SSA] p | +| main.rs:92:13:96:5 | RecordExpr | main.rs:92:9:92:9 | p | +| main.rs:97:20:97:20 | [SSA] a | main.rs:98:10:98:10 | a | +| main.rs:97:20:97:20 | a | main.rs:97:20:97:20 | [SSA] a | +| main.rs:97:26:97:26 | [SSA] b | main.rs:99:10:99:10 | b | +| main.rs:97:26:97:26 | b | main.rs:97:26:97:26 | [SSA] b | +| main.rs:97:32:97:32 | [SSA] c | main.rs:100:10:100:10 | c | +| main.rs:97:32:97:32 | c | main.rs:97:32:97:32 | [SSA] c | +| main.rs:97:38:97:38 | p | main.rs:97:9:97:34 | RecordPat | +| main.rs:104:9:104:10 | [SSA] s1 | main.rs:106:11:106:12 | s1 | +| main.rs:104:9:104:10 | s1 | main.rs:104:9:104:10 | [SSA] s1 | +| main.rs:104:14:104:28 | CallExpr | main.rs:104:9:104:10 | s1 | +| main.rs:105:9:105:10 | [SSA] s2 | main.rs:110:11:110:12 | s2 | +| main.rs:105:9:105:10 | s2 | main.rs:105:9:105:10 | [SSA] s2 | +| main.rs:105:14:105:20 | CallExpr | main.rs:105:9:105:10 | s2 | +| main.rs:107:14:107:14 | [SSA] n | main.rs:107:25:107:25 | n | +| main.rs:107:14:107:14 | n | main.rs:107:14:107:14 | [SSA] n | +| main.rs:107:20:107:26 | CallExpr | main.rs:106:5:109:5 | MatchExpr | +| main.rs:108:17:108:23 | CallExpr | main.rs:106:5:109:5 | MatchExpr | +| main.rs:110:5:113:5 | MatchExpr | main.rs:103:27:114:1 | BlockExpr | +| main.rs:111:14:111:14 | [SSA] n | main.rs:111:25:111:25 | n | +| main.rs:111:14:111:14 | n | main.rs:111:14:111:14 | [SSA] n | +| main.rs:111:20:111:26 | CallExpr | main.rs:110:5:113:5 | MatchExpr | +| main.rs:112:17:112:23 | CallExpr | main.rs:110:5:113:5 | MatchExpr | diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected new file mode 100644 index 000000000000..72273334c6f5 --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected @@ -0,0 +1,24 @@ +models +edges +| main.rs:19:13:19:21 | CallExpr : unit | main.rs:20:10:20:10 | s | provenance | | +| main.rs:24:13:24:21 | CallExpr : unit | main.rs:27:10:27:10 | c | provenance | | +| main.rs:31:13:31:21 | CallExpr : unit | main.rs:36:10:36:10 | b | provenance | | +| main.rs:45:15:45:23 | CallExpr : unit | main.rs:47:10:47:10 | b | provenance | | +nodes +| main.rs:15:10:15:18 | CallExpr | semmle.label | CallExpr | +| main.rs:19:13:19:21 | CallExpr : unit | semmle.label | CallExpr : unit | +| main.rs:20:10:20:10 | s | semmle.label | s | +| main.rs:24:13:24:21 | CallExpr : unit | semmle.label | CallExpr : unit | +| main.rs:27:10:27:10 | c | semmle.label | c | +| main.rs:31:13:31:21 | CallExpr : unit | semmle.label | CallExpr : unit | +| main.rs:36:10:36:10 | b | semmle.label | b | +| main.rs:45:15:45:23 | CallExpr : unit | semmle.label | CallExpr : unit | +| main.rs:47:10:47:10 | b | semmle.label | b | +subpaths +testFailures +#select +| main.rs:15:10:15:18 | CallExpr | main.rs:15:10:15:18 | CallExpr | main.rs:15:10:15:18 | CallExpr | $@ | main.rs:15:10:15:18 | CallExpr | CallExpr | +| main.rs:20:10:20:10 | s | main.rs:19:13:19:21 | CallExpr : unit | main.rs:20:10:20:10 | s | $@ | main.rs:19:13:19:21 | CallExpr : unit | CallExpr : unit | +| main.rs:27:10:27:10 | c | main.rs:24:13:24:21 | CallExpr : unit | main.rs:27:10:27:10 | c | $@ | main.rs:24:13:24:21 | CallExpr : unit | CallExpr : unit | +| main.rs:36:10:36:10 | b | main.rs:31:13:31:21 | CallExpr : unit | main.rs:36:10:36:10 | b | $@ | main.rs:31:13:31:21 | CallExpr : unit | CallExpr : unit | +| main.rs:47:10:47:10 | b | main.rs:45:15:45:23 | CallExpr : unit | main.rs:47:10:47:10 | b | $@ | main.rs:45:15:45:23 | CallExpr : unit | CallExpr : unit | diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.ql b/rust/ql/test/library-tests/dataflow/local/inline-flow.ql new file mode 100644 index 000000000000..ad553fe548dc --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/local/inline-flow.ql @@ -0,0 +1,12 @@ +/** + * @kind path-problem + */ + +import rust +import utils.InlineFlowTest +import DefaultFlowTest +import ValueFlow::PathGraph + +from ValueFlow::PathNode source, ValueFlow::PathNode sink +where ValueFlow::flowPath(source, sink) +select sink, source, sink, "$@", source, source.toString() diff --git a/rust/ql/test/library-tests/dataflow/local/main.rs b/rust/ql/test/library-tests/dataflow/local/main.rs index e607ad357591..fb79baa70bdb 100644 --- a/rust/ql/test/library-tests/dataflow/local/main.rs +++ b/rust/ql/test/library-tests/dataflow/local/main.rs @@ -1,41 +1,128 @@ -fn variable() { - let s = "Hello"; - println!("{:?} data flow!", s); +// Tests for intraprocedural data flow. + +fn source(i: i64) -> i64 { + 1000 + i +} + +fn sink(s: i64) { + println!("{}", s); +} + +// ----------------------------------------------------------------------------- +// Simple data flow through expressions and assignments + +fn direct() { + sink(source(1)); // $ hasValueFlow=1 +} + +fn variable_usage() { + let s = source(1); + sink(s); // $ hasValueFlow=1 } -fn if_expression(cond: bool) -> i64 { - let a = 1; +fn if_expression(cond: bool) { + let a = source(1); let b = 2; - let c = if cond { - a - } else { - b + let c = if cond { a } else { b }; + sink(c); // $ hasValueFlow=1 +} + +fn match_expression(m: Option) { + let a = source(1); + let b = match m { + Some(_) => a, + None => 0, }; - c + sink(b); // $ hasValueFlow=1 } -fn loop_expression() -> i64 { - let a = 1; +fn loop_with_break() { + let a = loop { + break 1; + }; + sink(a); let b = loop { - break a; + break source(1); }; - b + sink(b); // $ hasValueFlow=1 } -fn assignment() -> i64 { +fn assignment() { let mut i = 1; - i = 2; - i + sink(i); + i = source(2); + sink(i); // $ MISSING: hasValueFlow=2 } -fn match_expression(a: i64, b: i64, c: Option) -> i64 { - match c { - Some(_) => a, - None => b, +// ----------------------------------------------------------------------------- +// Data flow through data structures by writing and reading + +fn box_deref() { + let i = Box::new(source(1)); + sink(*i); // $ MISSING: hasValueFlow=1 +} + +fn tuple() { + let a = (source(1), 2); + sink(a.0); // $ MISSING: hasValueFlow=1 + sink(a.1); +} + +struct Point { + x: i64, + y: i64, + z: i64, +} + +fn struct_field() { + let p = Point { + x: source(1), + y: 2, + z: source(3), + }; + sink(p.x); // MISSING: hasValueFlow=1 + sink(p.y); + sink(p.z); // MISSING: hasValueFlow=3 +} + +// ----------------------------------------------------------------------------- +// Data flow through data structures by pattern matching + +fn struct_pattern_match() { + let p = Point { + x: source(1), + y: 2, + z: source(3), + }; + let Point { x: a, y: b, z: c } = p; + sink(a); // MISSING: hasValueFlow=1 + sink(b); + sink(c); // MISSING: hasValueFlow=3 +} + +fn option_pattern_match() { + let s1 = Some(source(1)); + let s2 = Some(2); + match s1 { + Some(n) => sink(n), // MISSING: hasValueFlow=3 + None => sink(0), + } + match s2 { + Some(n) => sink(n), + None => sink(0), } } fn main() { - variable(); + direct(); + variable_usage(); if_expression(true); + match_expression(Some(0)); + loop_with_break(); + assignment(); + box_deref(); + tuple(); + struct_field(); + struct_pattern_match(); + option_pattern_match(); } diff --git a/rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected new file mode 100644 index 000000000000..8d729bb5fd93 --- /dev/null +++ b/rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected @@ -0,0 +1,4 @@ +uniqueEnclosingCallable +| main.rs:5:29:5:33 | width | Node should have one enclosing callable but has 0. | +| main.rs:5:36:5:44 | precision | Node should have one enclosing callable but has 0. | +| main.rs:9:22:9:27 | people | Node should have one enclosing callable but has 0. | diff --git a/rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected new file mode 100644 index 000000000000..5bd870a72056 --- /dev/null +++ b/rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected @@ -0,0 +1,5 @@ +uniqueEnclosingCallable +| main.rs:5:29:5:33 | width | Node should have one enclosing callable but has 0. | +| main.rs:5:36:5:44 | precision | Node should have one enclosing callable but has 0. | +| main.rs:16:22:16:27 | people | Node should have one enclosing callable but has 0. | +| main.rs:27:23:27:27 | width | Node should have one enclosing callable but has 0. | diff --git a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected new file mode 100644 index 000000000000..0a61a151c204 --- /dev/null +++ b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected @@ -0,0 +1,5 @@ +uniqueEnclosingCallable +| main.rs:194:25:194:25 | x | Node should have one enclosing callable but has 0. | +| main.rs:198:28:198:28 | x | Node should have one enclosing callable but has 0. | +| main.rs:202:28:202:28 | x | Node should have one enclosing callable but has 0. | +| main.rs:206:28:206:28 | x | Node should have one enclosing callable but has 0. |