Rust: Fixes to flow through captured variables

Author: hvitved

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -1070,7 +1070,7 @@ module VariableCapture {
     final class CapturedParameter extends CapturedVariable {
       ParamBase p;
 
-      CapturedParameter() { p = this.getParameter() }
+      CapturedParameter() { this.isParameter(p) }
 
       SourceParameterNode getParameterNode() { result.getParameter() = p }
     }
@@ -1080,33 +1080,15 @@ module VariableCapture {
     }
 
     class VariableWrite extends Expr {
-      Expr source;
       CapturedVariable v;
 
       VariableWrite() {
-        exists(AssignmentExpr assign, Variable::VariableWriteAccess write |
-          this = assign and
-          v = write.getVariable() and
-          assign.getLhs() = write and
-          assign.getRhs() = source
-        )
-        or
-        this =
-          any(LetStmt ls |
-            v.getPat() = ls.getPat() and
-            ls.getInitializer() = source
-          )
+        this = v.getPat()
         or
-        this =
-          any(LetExpr le |
-            v.getPat() = le.getPat() and
-            le.getScrutinee() = source
-          )
+        v = this.(Variable::VariableWriteAccess).getVariable()
       }
 
       CapturedVariable getVariable() { result = v }
-
-      Expr getSource() { result = source }
     }
 
     class VariableRead extends Expr {
@@ -1145,7 +1127,7 @@ module VariableCapture {
     or
     result.(Flow::ExprNode).getExpr() = n.asExpr()
     or
-    result.(Flow::VariableWriteSourceNode).getVariableWrite().getSource() = n.asExpr()
+    result.(Flow::VariableWriteSourceNode).getVariableWrite() = n.getAstNode()
     or
     result.(Flow::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
     or

rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

@@ -310,7 +310,12 @@ module Impl {
     /** Holds if this variable is captured. */
     predicate isCaptured() { this.getAnAccess().isCapture() }
 
-    /** Gets the parameter that introduces this variable, if any. */
+    /**
+     * Gets the parameter that introduces this variable, if any.
+     *
+     * Note that this variable need not be a direct parameter; it could
+     * also be introduced through a destructuring pattern.
+     */
     cached
     ParamBase getParameter() {
       CachedStage::ref() and
@@ -319,6 +324,14 @@ module Impl {
       result.(Param).getPat() = getAPatAncestor*(this.getPat())
     }
 
+    /** Holds if this variable corresponds directly to the parameter `p`. */
+    predicate isParameter(ParamBase p) {
+      CachedStage::ref() and
+      p = this.getSelfParam()
+      or
+      p.(Param).getPat() = this.getPat()
+    }
+
     /** Holds if this variable is mutable. */
     predicate isMutable() { this.getPat().isMut() or this.getSelfParam().isMut() }
 

rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll

@@ -2348,7 +2348,7 @@ private module Input3 implements InputSig3 {
 
     AstNode getDefiningNode() {
       result = this.asLocalVariable().getPat().getName() or
-      result = this.asLocalVariable().getParameter().(SelfParam) or
+      this.asLocalVariable().isParameter(result.(SelfParam)) or
       result = this.asConst().getName() or
       result = this.asStatic().getName()
     }

rust/ql/test/library-tests/dataflow/lambdas/inline-flow.expected

@@ -13,7 +13,8 @@ edges
 | main.rs:29:13:29:22 | f(...) | main.rs:29:9:29:9 | b | provenance |  |
 | main.rs:29:21:29:21 | a | main.rs:27:20:27:23 | ... | provenance |  |
 | main.rs:29:21:29:21 | a | main.rs:29:13:29:22 | f(...) | provenance |  |
-| main.rs:37:16:37:25 | source(...) | main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | provenance |  |
+| main.rs:37:9:37:12 | capt | main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | provenance |  |
+| main.rs:37:16:37:25 | source(...) | main.rs:37:9:37:12 | capt | provenance |  |
 | main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | main.rs:40:10:40:13 | capt | provenance |  |
 | main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | main.rs:44:5:44:5 | g [captured capt] | provenance |  |
 | main.rs:44:5:44:5 | g [captured capt] | main.rs:42:14:42:17 | capt | provenance |  |
@@ -55,22 +56,26 @@ edges
 | main.rs:100:13:100:22 | source(...) | main.rs:100:9:100:9 | b | provenance |  |
 | main.rs:101:17:101:17 | ... | main.rs:101:25:101:25 | x | provenance |  |
 | main.rs:101:29:101:29 | b | main.rs:93:33:93:38 | ...: i64 | provenance |  |
-| main.rs:109:13:109:22 | source(...) | main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
-| main.rs:109:13:109:22 | source(...) | main.rs:111:26:111:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
-| main.rs:109:13:109:22 | source(...) | main.rs:112:26:112:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
-| main.rs:109:13:109:22 | source(...) | main.rs:113:26:113:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
+| main.rs:109:9:109:9 | a | main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
+| main.rs:109:9:109:9 | a | main.rs:111:26:111:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
+| main.rs:109:9:109:9 | a | main.rs:112:26:112:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
+| main.rs:109:9:109:9 | a | main.rs:113:26:113:36 | \|...\| ... : ... => .. [captured a] | provenance |  |
+| main.rs:109:13:109:22 | source(...) | main.rs:109:9:109:9 | a | provenance |  |
 | main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | main.rs:110:35:110:35 | a | provenance | heuristic-callback |
 | main.rs:111:26:111:36 | \|...\| ... : ... => .. [captured a] | main.rs:111:35:111:35 | a | provenance | heuristic-callback |
 | main.rs:112:26:112:36 | \|...\| ... : ... => .. [captured a] | main.rs:112:35:112:35 | a | provenance | heuristic-callback |
 | main.rs:113:26:113:36 | \|...\| ... : ... => .. [captured a] | main.rs:113:35:113:35 | a | provenance | heuristic-callback |
 | main.rs:120:30:120:40 | source(...) | main.rs:121:14:121:25 | await future | provenance |  |
-| main.rs:125:20:125:30 | source(...) | main.rs:126:9:128:9 | { ... } : ... => .. [captured data] | provenance |  |
+| main.rs:125:13:125:16 | data | main.rs:126:9:128:9 | { ... } : ... => .. [captured data] | provenance |  |
+| main.rs:125:20:125:30 | source(...) | main.rs:125:13:125:16 | data | provenance |  |
 | main.rs:126:9:128:9 | { ... } : ... => .. [captured data] | main.rs:127:18:127:21 | data | provenance |  |
-| main.rs:133:20:133:30 | source(...) | main.rs:135:14:135:19 | future [captured data] | provenance |  |
+| main.rs:133:13:133:16 | data | main.rs:135:14:135:19 | future [captured data] | provenance |  |
+| main.rs:133:20:133:30 | source(...) | main.rs:133:13:133:16 | data | provenance |  |
 | main.rs:135:14:135:19 | future [captured data] | main.rs:134:30:134:33 | data | provenance |  |
 | main.rs:135:14:135:19 | future [captured data] | main.rs:135:14:135:25 | await future | provenance |  |
 | main.rs:141:9:143:9 | [post] { ... } : ... => .. [captured captured] | main.rs:145:14:145:21 | captured | provenance |  |
-| main.rs:142:24:142:34 | source(...) | main.rs:141:9:143:9 | [post] { ... } : ... => .. [captured captured] | provenance |  |
+| main.rs:142:13:142:20 | captured | main.rs:141:9:143:9 | [post] { ... } : ... => .. [captured captured] | provenance |  |
+| main.rs:142:24:142:34 | source(...) | main.rs:142:13:142:20 | captured | provenance |  |
 nodes
 | main.rs:10:20:10:52 | if cond {...} else {...} | semmle.label | if cond {...} else {...} |
 | main.rs:10:30:10:39 | source(...) | semmle.label | source(...) |
@@ -88,6 +93,7 @@ nodes
 | main.rs:29:13:29:22 | f(...) | semmle.label | f(...) |
 | main.rs:29:21:29:21 | a | semmle.label | a |
 | main.rs:30:10:30:10 | b | semmle.label | b |
+| main.rs:37:9:37:12 | capt | semmle.label | capt |
 | main.rs:37:16:37:25 | source(...) | semmle.label | source(...) |
 | main.rs:39:5:39:5 | [post] f : ... => .. [captured capt] | semmle.label | [post] f : ... => .. [captured capt] |
 | main.rs:40:10:40:13 | capt | semmle.label | capt |
@@ -137,6 +143,7 @@ nodes
 | main.rs:101:17:101:17 | ... | semmle.label | ... |
 | main.rs:101:25:101:25 | x | semmle.label | x |
 | main.rs:101:29:101:29 | b | semmle.label | b |
+| main.rs:109:9:109:9 | a | semmle.label | a |
 | main.rs:109:13:109:22 | source(...) | semmle.label | source(...) |
 | main.rs:110:26:110:36 | \|...\| ... : ... => .. [captured a] | semmle.label | \|...\| ... : ... => .. [captured a] |
 | main.rs:110:35:110:35 | a | semmle.label | a |
@@ -148,14 +155,17 @@ nodes
 | main.rs:113:35:113:35 | a | semmle.label | a |
 | main.rs:120:30:120:40 | source(...) | semmle.label | source(...) |
 | main.rs:121:14:121:25 | await future | semmle.label | await future |
+| main.rs:125:13:125:16 | data | semmle.label | data |
 | main.rs:125:20:125:30 | source(...) | semmle.label | source(...) |
 | main.rs:126:9:128:9 | { ... } : ... => .. [captured data] | semmle.label | { ... } : ... => .. [captured data] |
 | main.rs:127:18:127:21 | data | semmle.label | data |
+| main.rs:133:13:133:16 | data | semmle.label | data |
 | main.rs:133:20:133:30 | source(...) | semmle.label | source(...) |
 | main.rs:134:30:134:33 | data | semmle.label | data |
 | main.rs:135:14:135:19 | future [captured data] | semmle.label | future [captured data] |
 | main.rs:135:14:135:25 | await future | semmle.label | await future |
 | main.rs:141:9:143:9 | [post] { ... } : ... => .. [captured captured] | semmle.label | [post] { ... } : ... => .. [captured captured] |
+| main.rs:142:13:142:20 | captured | semmle.label | captured |
 | main.rs:142:24:142:34 | source(...) | semmle.label | source(...) |
 | main.rs:145:14:145:21 | captured | semmle.label | captured |
 subpaths