Rust: Data flow through tuple and struct fields

github · Nov 27, 2024 · d89678f · d89678f
1 parent 9ec9d79
commit d89678f
Show file tree

Hide file tree

Showing 4 changed files with 220 additions and 29 deletions.
diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
@@ -252,10 +252,10 @@ module Node {
    * Nodes corresponding to AST elements, for example `ExprNode`, usually refer
    * to the value before the update.
    */
-  final class PostUpdateNode extends Node, TArgumentPostUpdateNode {
+  final class PostUpdateNode extends Node, TExprPostUpdateNode {
     private ExprCfgNode n;
 
-    PostUpdateNode() { this = TArgumentPostUpdateNode(n) }
+    PostUpdateNode() { this = TExprPostUpdateNode(n) }
 
     /** Gets the node before the state update. */
     Node getPreUpdateNode() { result = TExprNode(n) }
@@ -449,6 +449,49 @@ private class VariantFieldContent extends VariantContent, TVariantFieldContent {
   }
 }
 
+/** A canonical path pointing to a struct. */
+private class StructCanonicalPath extends MkStructCanonicalPath {
+  CrateOriginOption crate;
+  string path;
+
+  StructCanonicalPath() { this = MkStructCanonicalPath(crate, path) }
+
+  /** Gets the underlying struct. */
+  Struct getStruct() { hasExtendedCanonicalPath(result, crate, path) }
+
+  string toString() { result = this.getStruct().getName().getText() }
+
+  Location getLocation() { result = this.getStruct().getLocation() }
+}
+
+/** Content stored in a field on a struct. */
+private class StructFieldContent extends VariantContent, TStructFieldContent {
+  private StructCanonicalPath s;
+  private string field_;
+
+  StructFieldContent() { this = TStructFieldContent(s, field_) }
+
+  StructCanonicalPath getStructCanonicalPath(string field) { result = s and field = field_ }
+
+  override string toString() { result = s.toString() + "." + field_.toString() }
+}
+
+/**
+ * Content stored at a position in a tuple.
+ *
+ * NOTE: Unlike `struct`s and `enum`s tuples are structural and not nominal,
+ * hence we don't store a canonical path for them.
+ */
+private class TuplePositionContent extends VariantContent, TTuplePositionContent {
+  private int pos;
+
+  TuplePositionContent() { this = TTuplePositionContent(pos) }
+
+  int getPosition() { result = pos }
+
+  override string toString() { result = "tuple." + pos.toString() }
+}
+
 /** A value that represents a set of `Content`s. */
 abstract class ContentSet extends TContentSet {
   /** Gets a textual representation of this element. */
@@ -608,6 +651,14 @@ module RustDataFlow implements InputSig<Location> {
    */
   predicate jumpStep(Node node1, Node node2) { none() }
 
+  /** Holds if path `p` resolves to struct `s`. */
+  private predicate pathResolveToStructCanonicalPath(Path p, StructCanonicalPath s) {
+    exists(CrateOriginOption crate, string path |
+      resolveExtendedCanonicalPath(p, crate, path) and
+      s = MkStructCanonicalPath(crate, path)
+    )
+  }
+
   /** Holds if path `p` resolves to variant `v`. */
   private predicate pathResolveToVariantCanonicalPath(Path p, VariantCanonicalPath v) {
     exists(CrateOriginOption crate, string path |
@@ -636,6 +687,12 @@ module RustDataFlow implements InputSig<Location> {
     pathResolveToVariantCanonicalPath(p.getPath(), v)
   }
 
+  /** Holds if `p` destructs an struct `s`. */
+  pragma[nomagic]
+  private predicate structDestruction(RecordPat p, StructCanonicalPath s) {
+    pathResolveToStructCanonicalPath(p.getPath(), s)
+  }
+
   /**
    * Holds if data can flow from `node1` to `node2` via a read of `c`.  Thus,
    * `node1` references an object with a content `c.getAReadContent()` whose
@@ -652,10 +709,24 @@ module RustDataFlow implements InputSig<Location> {
       or
       exists(RecordPatCfgNode pat, string field |
         pat = node1.asPat() and
-        recordVariantDestruction(pat.getPat(),
-          c.(VariantFieldContent).getVariantCanonicalPath(field)) and
+        (
+          // Pattern destructs a struct-like variant.
+          recordVariantDestruction(pat.getPat(),
+            c.(VariantFieldContent).getVariantCanonicalPath(field))
+          or
+          // Pattern destructs a struct.
+          structDestruction(pat.getPat(), c.(StructFieldContent).getStructCanonicalPath(field))
+        ) and
         node2.asPat() = pat.getFieldPat(field)
       )
+      or
+      exists(FieldExprCfgNode access |
+        // Read of a tuple entry
+        access.getNameRef().getText().toInt() = c.(TuplePositionContent).getPosition() and
+        // TODO: Handle read of a struct field.
+        node1.asExpr() = access.getExpr() and
+        node2.asExpr() = access
+      )
     )
   }
 
@@ -671,30 +742,44 @@ module RustDataFlow implements InputSig<Location> {
     pathResolveToVariantCanonicalPath(re.getPath(), v)
   }
 
+  /** Holds if `re` constructs a struct value of type `v`. */
+  pragma[nomagic]
+  private predicate structConstruction(RecordExpr re, StructCanonicalPath s) {
+    pathResolveToStructCanonicalPath(re.getPath(), s)
+  }
+
   /**
    * Holds if data can flow from `node1` to `node2` via a store into `c`.  Thus,
    * `node2` references an object with a content `c.getAStoreContent()` that
    * contains the value of `node1`.
    */
   predicate storeStep(Node node1, ContentSet cs, Node node2) {
     exists(Content c | c = cs.(SingletonContentSet).getContent() |
-      node2.asExpr() =
-        any(CallExprCfgNode call, int pos |
-          tupleVariantConstruction(call.getCallExpr(),
-            c.(VariantPositionContent).getVariantCanonicalPath(pos)) and
-          node1.asExpr() = call.getArgument(pos)
-        |
-          call
-        )
+      exists(CallExprCfgNode call, int pos |
+        tupleVariantConstruction(call.getCallExpr(),
+          c.(VariantPositionContent).getVariantCanonicalPath(pos)) and
+        node1.asExpr() = call.getArgument(pos) and
+        node2.asExpr() = call
+      )
       or
-      node2.asExpr() =
-        any(RecordExprCfgNode re, string field |
+      exists(RecordExprCfgNode re, string field |
+        (
+          // Expression is for a struct-like enum variant.
           recordVariantConstruction(re.getRecordExpr(),
-            c.(VariantFieldContent).getVariantCanonicalPath(field)) and
-          node1.asExpr() = re.getFieldExpr(field)
-        |
-          re
-        )
+            c.(VariantFieldContent).getVariantCanonicalPath(field))
+          or
+          // Expression is for a struct.
+          structConstruction(re.getRecordExpr(),
+            c.(StructFieldContent).getStructCanonicalPath(field))
+        ) and
+        node1.asExpr() = re.getFieldExpr(field) and
+        node2.asExpr() = re
+      )
+      or
+      exists(TupleExprCfgNode tuple |
+        node1.asExpr() = tuple.getField(c.(TuplePositionContent).getPosition()) and
+        node2.asExpr() = tuple
+      )
     )
   }
 
@@ -703,7 +788,14 @@ module RustDataFlow implements InputSig<Location> {
    * any value stored inside `f` is cleared at the pre-update node associated with `x`
    * in `x.f = newValue`.
    */
-  predicate clearsContent(Node n, ContentSet c) { none() }
+  predicate clearsContent(Node n, ContentSet c) {
+    exists(AssignmentExprCfgNode assignment, FieldExprCfgNode access |
+      assignment.getLhs() = access and
+      n.asExpr() = access.getExpr() and
+      access.getNameRef().getText().toInt() =
+        c.(SingletonContentSet).getContent().(TuplePositionContent).getPosition()
+    )
+  }
 
   /**
    * Holds if the value that is being tracked is expected to be stored inside content `c`
@@ -773,7 +865,9 @@ private module Cached {
     TExprNode(ExprCfgNode n) or
     TParameterNode(ParamBaseCfgNode p) or
     TPatNode(PatCfgNode p) or
-    TArgumentPostUpdateNode(ExprCfgNode e) { isArgumentForCall(e, _, _) } or
+    TExprPostUpdateNode(ExprCfgNode e) {
+      isArgumentForCall(e, _, _) or e = any(FieldExprCfgNode access).getExpr()
+    } or
     TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node)
 
   cached
@@ -811,6 +905,12 @@ private module Cached {
       name = ["Ok", "Err"]
     }
 
+  cached
+  newtype TStructCanonicalPath =
+    MkStructCanonicalPath(CrateOriginOption crate, string path) {
+      exists(Struct s | hasExtendedCanonicalPath(s, crate, path))
+    }
+
   cached
   newtype TContent =
     TVariantPositionContent(VariantCanonicalPath v, int pos) {
@@ -826,6 +926,16 @@ private module Cached {
     } or
     TVariantFieldContent(VariantCanonicalPath v, string field) {
       field = v.getVariant().getFieldList().(RecordFieldList).getAField().getName().getText()
+    } or
+    TTuplePositionContent(int pos) {
+      pos in [0 .. max([
+                any(TuplePat pat).getNumberOfFields(),
+                any(FieldExpr access).getNameRef().getText().toInt()
+              ]
+          )]
+    } or
+    TStructFieldContent(StructCanonicalPath s, string field) {
+      field = s.getStruct().getFieldList().(RecordFieldList).getAField().getName().getText()
     }
 
   cached

diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
@@ -103,9 +103,9 @@ localStep
 | main.rs:109:10:109:10 | a | main.rs:110:10:110:10 | a |
 | main.rs:110:10:110:10 | a | main.rs:111:5:111:5 | a |
 | main.rs:111:5:111:5 | a | main.rs:112:5:112:5 | a |
-| main.rs:111:11:111:11 | 2 | main.rs:111:5:111:7 | a.1 |
+| main.rs:111:11:111:20 | source(...) | main.rs:111:5:111:7 | a.0 |
 | main.rs:112:5:112:5 | a | main.rs:113:10:113:10 | a |
-| main.rs:112:11:112:20 | source(...) | main.rs:112:5:112:7 | a.0 |
+| main.rs:112:11:112:11 | 2 | main.rs:112:5:112:7 | a.1 |
 | main.rs:113:10:113:10 | a | main.rs:114:10:114:10 | a |
 | main.rs:118:9:118:9 | [SSA] a | main.rs:119:14:119:14 | a |
 | main.rs:118:9:118:9 | a | main.rs:118:9:118:9 | [SSA] a |
@@ -329,6 +329,31 @@ localStep
 | main.rs:304:22:304:22 | n | main.rs:304:22:304:22 | [SSA] n |
 | main.rs:304:29:304:35 | sink(...) | main.rs:302:5:305:5 | match s2 { ... } |
 storeStep
+| main.rs:94:14:94:22 | source(...) | tuple.0 | main.rs:94:13:94:26 | TupleExpr |
+| main.rs:94:25:94:25 | 2 | tuple.1 | main.rs:94:13:94:26 | TupleExpr |
+| main.rs:100:14:100:14 | 2 | tuple.0 | main.rs:100:13:100:30 | TupleExpr |
+| main.rs:100:17:100:26 | source(...) | tuple.1 | main.rs:100:13:100:30 | TupleExpr |
+| main.rs:100:29:100:29 | 2 | tuple.2 | main.rs:100:13:100:30 | TupleExpr |
+| main.rs:108:18:108:18 | 2 | tuple.0 | main.rs:108:17:108:31 | TupleExpr |
+| main.rs:108:21:108:30 | source(...) | tuple.1 | main.rs:108:17:108:31 | TupleExpr |
+| main.rs:118:14:118:14 | 3 | tuple.0 | main.rs:118:13:118:27 | TupleExpr |
+| main.rs:118:17:118:26 | source(...) | tuple.1 | main.rs:118:13:118:27 | TupleExpr |
+| main.rs:119:14:119:14 | a | tuple.0 | main.rs:119:13:119:18 | TupleExpr |
+| main.rs:119:17:119:17 | 3 | tuple.1 | main.rs:119:13:119:18 | TupleExpr |
+| main.rs:135:12:135:20 | source(...) | Point.x | main.rs:134:13:137:5 | Point {...} |
+| main.rs:136:12:136:12 | 2 | Point.y | main.rs:134:13:137:5 | Point {...} |
+| main.rs:144:12:144:20 | source(...) | Point.x | main.rs:143:17:146:5 | Point {...} |
+| main.rs:145:12:145:12 | 2 | Point.y | main.rs:143:17:146:5 | Point {...} |
+| main.rs:154:12:154:21 | source(...) | Point.x | main.rs:153:13:156:5 | Point {...} |
+| main.rs:155:12:155:12 | 2 | Point.y | main.rs:153:13:156:5 | Point {...} |
+| main.rs:169:16:172:9 | Point {...} | Point3D.plane | main.rs:168:13:174:5 | Point3D {...} |
+| main.rs:170:16:170:16 | 2 | Point.x | main.rs:169:16:172:9 | Point {...} |
+| main.rs:171:16:171:25 | source(...) | Point.y | main.rs:169:16:172:9 | Point {...} |
+| main.rs:173:12:173:12 | 4 | Point3D.z | main.rs:168:13:174:5 | Point3D {...} |
+| main.rs:182:16:185:9 | Point {...} | Point3D.plane | main.rs:181:13:187:5 | Point3D {...} |
+| main.rs:183:16:183:16 | 2 | Point.x | main.rs:182:16:185:9 | Point {...} |
+| main.rs:184:16:184:25 | source(...) | Point.y | main.rs:182:16:185:9 | Point {...} |
+| main.rs:186:12:186:12 | 4 | Point3D.z | main.rs:181:13:187:5 | Point3D {...} |
 | main.rs:214:19:214:28 | source(...) | Some | main.rs:214:14:214:29 | Some(...) |
 | main.rs:215:19:215:19 | 2 | Some | main.rs:215:14:215:20 | Some(...) |
 | main.rs:232:29:232:38 | source(...) | A | main.rs:232:14:232:39 | ...::A(...) |
@@ -338,6 +363,22 @@ storeStep
 | main.rs:312:27:312:27 | 0 | Some | main.rs:312:22:312:28 | Some(...) |
 readStep
 | main.rs:33:9:33:15 | TupleStructPat | Some | main.rs:33:14:33:14 | _ |
+| main.rs:95:10:95:10 | a | tuple.0 | main.rs:95:10:95:12 | a.0 |
+| main.rs:96:10:96:10 | a | tuple.1 | main.rs:96:10:96:12 | a.1 |
+| main.rs:109:10:109:10 | a | tuple.0 | main.rs:109:10:109:12 | a.0 |
+| main.rs:110:10:110:10 | a | tuple.1 | main.rs:110:10:110:12 | a.1 |
+| main.rs:111:5:111:5 | a | tuple.0 | main.rs:111:5:111:7 | a.0 |
+| main.rs:112:5:112:5 | a | tuple.1 | main.rs:112:5:112:7 | a.1 |
+| main.rs:113:10:113:10 | a | tuple.0 | main.rs:113:10:113:12 | a.0 |
+| main.rs:114:10:114:10 | a | tuple.1 | main.rs:114:10:114:12 | a.1 |
+| main.rs:120:10:120:10 | b | tuple.0 | main.rs:120:10:120:12 | b.0 |
+| main.rs:120:10:120:12 | b.0 | tuple.0 | main.rs:120:10:120:14 | ... .0 |
+| main.rs:121:10:121:10 | b | tuple.0 | main.rs:121:10:121:12 | b.0 |
+| main.rs:121:10:121:12 | b.0 | tuple.1 | main.rs:121:10:121:14 | ... .1 |
+| main.rs:122:10:122:10 | b | tuple.1 | main.rs:122:10:122:12 | b.1 |
+| main.rs:157:9:157:28 | Point {...} | Point.x | main.rs:157:20:157:20 | a |
+| main.rs:157:9:157:28 | Point {...} | Point.y | main.rs:157:26:157:26 | b |
+| main.rs:189:9:189:45 | Point3D {...} | Point3D.plane | main.rs:189:26:189:39 | Point {...} |
 | main.rs:217:9:217:15 | TupleStructPat | Some | main.rs:217:14:217:14 | n |
 | main.rs:221:9:221:15 | TupleStructPat | Some | main.rs:221:14:221:14 | n |
 | main.rs:235:9:235:25 | TupleStructPat | A | main.rs:235:24:235:24 | n |

diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
@@ -5,6 +5,22 @@ edges
 | main.rs:31:13:31:21 | source(...) | main.rs:36:10:36:10 | b | provenance |  |
 | main.rs:45:15:45:23 | source(...) | main.rs:47:10:47:10 | b | provenance |  |
 | main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | provenance |  |
+| main.rs:94:13:94:26 | TupleExpr [tuple.0] | main.rs:95:10:95:10 | a [tuple.0] | provenance |  |
+| main.rs:94:14:94:22 | source(...) | main.rs:94:13:94:26 | TupleExpr [tuple.0] | provenance |  |
+| main.rs:95:10:95:10 | a [tuple.0] | main.rs:95:10:95:12 | a.0 | provenance |  |
+| main.rs:108:17:108:31 | TupleExpr [tuple.1] | main.rs:110:10:110:10 | a [tuple.1] | provenance |  |
+| main.rs:108:21:108:30 | source(...) | main.rs:108:17:108:31 | TupleExpr [tuple.1] | provenance |  |
+| main.rs:110:10:110:10 | a [tuple.1] | main.rs:110:10:110:12 | a.1 | provenance |  |
+| main.rs:118:13:118:27 | TupleExpr [tuple.1] | main.rs:119:14:119:14 | a [tuple.1] | provenance |  |
+| main.rs:118:17:118:26 | source(...) | main.rs:118:13:118:27 | TupleExpr [tuple.1] | provenance |  |
+| main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | main.rs:121:10:121:10 | b [tuple.0, tuple.1] | provenance |  |
+| main.rs:119:14:119:14 | a [tuple.1] | main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | provenance |  |
+| main.rs:121:10:121:10 | b [tuple.0, tuple.1] | main.rs:121:10:121:12 | b.0 [tuple.1] | provenance |  |
+| main.rs:121:10:121:12 | b.0 [tuple.1] | main.rs:121:10:121:14 | ... .1 | provenance |  |
+| main.rs:153:13:156:5 | Point {...} [Point.x] | main.rs:157:9:157:28 | Point {...} [Point.x] | provenance |  |
+| main.rs:154:12:154:21 | source(...) | main.rs:153:13:156:5 | Point {...} [Point.x] | provenance |  |
+| main.rs:157:9:157:28 | Point {...} [Point.x] | main.rs:157:20:157:20 | a | provenance |  |
+| main.rs:157:20:157:20 | a | main.rs:158:10:158:10 | a | provenance |  |
 | main.rs:214:14:214:29 | Some(...) [Some] | main.rs:217:9:217:15 | TupleStructPat [Some] | provenance |  |
 | main.rs:214:19:214:28 | source(...) | main.rs:214:14:214:29 | Some(...) [Some] | provenance |  |
 | main.rs:217:9:217:15 | TupleStructPat [Some] | main.rs:217:14:217:14 | n | provenance |  |
@@ -35,6 +51,26 @@ nodes
 | main.rs:47:10:47:10 | b | semmle.label | b |
 | main.rs:53:9:53:17 | source(...) | semmle.label | source(...) |
 | main.rs:54:10:54:10 | i | semmle.label | i |
+| main.rs:94:13:94:26 | TupleExpr [tuple.0] | semmle.label | TupleExpr [tuple.0] |
+| main.rs:94:14:94:22 | source(...) | semmle.label | source(...) |
+| main.rs:95:10:95:10 | a [tuple.0] | semmle.label | a [tuple.0] |
+| main.rs:95:10:95:12 | a.0 | semmle.label | a.0 |
+| main.rs:108:17:108:31 | TupleExpr [tuple.1] | semmle.label | TupleExpr [tuple.1] |
+| main.rs:108:21:108:30 | source(...) | semmle.label | source(...) |
+| main.rs:110:10:110:10 | a [tuple.1] | semmle.label | a [tuple.1] |
+| main.rs:110:10:110:12 | a.1 | semmle.label | a.1 |
+| main.rs:118:13:118:27 | TupleExpr [tuple.1] | semmle.label | TupleExpr [tuple.1] |
+| main.rs:118:17:118:26 | source(...) | semmle.label | source(...) |
+| main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | semmle.label | TupleExpr [tuple.0, tuple.1] |
+| main.rs:119:14:119:14 | a [tuple.1] | semmle.label | a [tuple.1] |
+| main.rs:121:10:121:10 | b [tuple.0, tuple.1] | semmle.label | b [tuple.0, tuple.1] |
+| main.rs:121:10:121:12 | b.0 [tuple.1] | semmle.label | b.0 [tuple.1] |
+| main.rs:121:10:121:14 | ... .1 | semmle.label | ... .1 |
+| main.rs:153:13:156:5 | Point {...} [Point.x] | semmle.label | Point {...} [Point.x] |
+| main.rs:154:12:154:21 | source(...) | semmle.label | source(...) |
+| main.rs:157:9:157:28 | Point {...} [Point.x] | semmle.label | Point {...} [Point.x] |
+| main.rs:157:20:157:20 | a | semmle.label | a |
+| main.rs:158:10:158:10 | a | semmle.label | a |
 | main.rs:214:14:214:29 | Some(...) [Some] | semmle.label | Some(...) [Some] |
 | main.rs:214:19:214:28 | source(...) | semmle.label | source(...) |
 | main.rs:217:9:217:15 | TupleStructPat [Some] | semmle.label | TupleStructPat [Some] |
@@ -65,6 +101,10 @@ testFailures
 | main.rs:36:10:36:10 | b | main.rs:31:13:31:21 | source(...) | main.rs:36:10:36:10 | b | $@ | main.rs:31:13:31:21 | source(...) | source(...) |
 | main.rs:47:10:47:10 | b | main.rs:45:15:45:23 | source(...) | main.rs:47:10:47:10 | b | $@ | main.rs:45:15:45:23 | source(...) | source(...) |
 | main.rs:54:10:54:10 | i | main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | $@ | main.rs:53:9:53:17 | source(...) | source(...) |
+| main.rs:95:10:95:12 | a.0 | main.rs:94:14:94:22 | source(...) | main.rs:95:10:95:12 | a.0 | $@ | main.rs:94:14:94:22 | source(...) | source(...) |
+| main.rs:110:10:110:12 | a.1 | main.rs:108:21:108:30 | source(...) | main.rs:110:10:110:12 | a.1 | $@ | main.rs:108:21:108:30 | source(...) | source(...) |
+| main.rs:121:10:121:14 | ... .1 | main.rs:118:17:118:26 | source(...) | main.rs:121:10:121:14 | ... .1 | $@ | main.rs:118:17:118:26 | source(...) | source(...) |
+| main.rs:158:10:158:10 | a | main.rs:154:12:154:21 | source(...) | main.rs:158:10:158:10 | a | $@ | main.rs:154:12:154:21 | source(...) | source(...) |
 | main.rs:217:25:217:25 | n | main.rs:214:19:214:28 | source(...) | main.rs:217:25:217:25 | n | $@ | main.rs:214:19:214:28 | source(...) | source(...) |
 | main.rs:235:35:235:35 | n | main.rs:232:29:232:38 | source(...) | main.rs:235:35:235:35 | n | $@ | main.rs:232:29:232:38 | source(...) | source(...) |
 | main.rs:239:55:239:55 | n | main.rs:232:29:232:38 | source(...) | main.rs:239:55:239:55 | n | $@ | main.rs:232:29:232:38 | source(...) | source(...) |