Ruby: Content flow through captured variables

ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

@@ -727,16 +727,30 @@ module ExprNodes {
 
     override VariableAccess e;
 
-    final override VariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+    override VariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+
+    /** Gets the variable that is being accessed. */
+    Variable getVariable() { result = this.getExpr().getVariable() }
   }
 
   /** A control-flow node that wraps a `VariableReadAccess` AST expression. */
-  class VariableReadAccessCfgNode extends ExprCfgNode {
+  class VariableReadAccessCfgNode extends VariableAccessCfgNode {
     override string getAPrimaryQlClass() { result = "VariableReadAccessCfgNode" }
 
     override VariableReadAccess e;
 
-    final override VariableReadAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+    override VariableReadAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
+  }
+
+  /** A control-flow node that wraps a `LocalVariableReadAccess` AST expression. */
+  class LocalVariableReadAccessCfgNode extends VariableReadAccessCfgNode {
+    override string getAPrimaryQlClass() { result = "LocalVariableReadAccessCfgNode" }
+
+    override LocalVariableReadAccess e;
+
+    final override LocalVariableReadAccess getExpr() { result = super.getExpr() }
+
+    final override LocalVariable getVariable() { result = super.getVariable() }
   }
 
   private class InstanceVariableAccessMapping extends ExprChildMapping, InstanceVariableAccess {
@@ -762,21 +776,32 @@ module ExprNodes {
   }
 
   /** A control-flow node that wraps a `SelfVariableAccess` AST expression. */
-  class SelfVariableAccessCfgNode extends ExprCfgNode {
+  class SelfVariableAccessCfgNode extends VariableAccessCfgNode {
     final override string getAPrimaryQlClass() { result = "SelfVariableAccessCfgNode" }
 
     override SelfVariableAccessMapping e;
 
-    override SelfVariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+    override SelfVariableAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
   }
 
   /** A control-flow node that wraps a `VariableWriteAccess` AST expression. */
-  class VariableWriteAccessCfgNode extends ExprCfgNode {
+  class VariableWriteAccessCfgNode extends VariableAccessCfgNode {
     override string getAPrimaryQlClass() { result = "VariableWriteAccessCfgNode" }
 
     override VariableWriteAccess e;
 
-    final override VariableWriteAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+    override VariableWriteAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
+  }
+
+  /** A control-flow node that wraps a `LocalVariableWriteAccess` AST expression. */
+  class LocalVariableWriteAccessCfgNode extends VariableWriteAccessCfgNode {
+    override string getAPrimaryQlClass() { result = "LocalVariableWriteAccessCfgNode" }
+
+    override LocalVariableWriteAccess e;
+
+    final override LocalVariableWriteAccess getExpr() { result = super.getExpr() }
+
+    final override LocalVariable getVariable() { result = super.getVariable() }
   }
 
   /** A control-flow node that wraps a `ConstantReadAccess` AST expression. */

ruby/ql/lib/codeql/ruby/dataflow/SSA.qll

@@ -178,6 +178,9 @@ module Ssa {
 
     /** Gets the location of this SSA definition. */
     Location getLocation() { result = this.getControlFlowNode().getLocation() }
+
+    /** Gets the scope of this SSA definition. */
+    CfgScope getScope() { result = this.getBasicBlock().getScope() }
   }
 
   /**
@@ -289,7 +292,7 @@ module Ssa {
       )
     }
 
-    final override string toString() { result = "<captured> " + this.getSourceVariable() }
+    final override string toString() { result = "<captured entry> " + this.getSourceVariable() }
 
     override Location getLocation() { result = this.getBasicBlock().getLocation() }
   }
@@ -324,7 +327,7 @@ module Ssa {
      */
     final Definition getPriorDefinition() { result = SsaImpl::uncertainWriteDefinitionInput(this) }
 
-    override string toString() { result = this.getControlFlowNode().toString() }
+    override string toString() { result = "<captured exit> " + this.getSourceVariable() }
   }
 
   /**

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -8,10 +8,17 @@ private import FlowSummaryImpl as FlowSummaryImpl
 private import FlowSummaryImplSpecific as FlowSummaryImplSpecific
 private import codeql.ruby.dataflow.FlowSummary
 private import codeql.ruby.dataflow.SSA
+private import codeql.ruby.dataflow.internal.SsaImpl as SsaImpl
 
 newtype TReturnKind =
   TNormalReturnKind() or
-  TBreakReturnKind()
+  TBreakReturnKind() or
+  TCapturedReturnKind(LocalVariable v) {
+    exists(Ssa::Definition def |
+      SsaImpl::captureFlowOut(_, def, _) and
+      v = def.getSourceVariable()
+    )
+  }
 
 /**
  * Gets a node that can read the value returned from `call` with return kind
@@ -43,6 +50,19 @@ class BreakReturnKind extends ReturnKind, TBreakReturnKind {
   override string toString() { result = "break" }
 }
 
+/**
+ * A value implicitly returned by updating a captured variable.
+ */
+class CapturedReturnKind extends ReturnKind, TCapturedReturnKind {
+  LocalVariable v;
+
+  CapturedReturnKind() { this = TCapturedReturnKind(v) }
+
+  LocalVariable getVariable() { result = v }
+
+  override string toString() { result = "captured write to " + v }
+}
+
 /** A callable defined in library code, identified by a unique string. */
 abstract class LibraryCallable extends string {
   bindingset[this]
@@ -151,6 +171,33 @@ private class NormalCall extends DataFlowCall, TNormalCall {
   override Location getLocation() { result = c.getLocation() }
 }
 
+/**
+ * A call that may ultimately reach a callable, which reads or updates
+ * (contents of) a captured variable.
+ */
+class CapturedCall extends DataFlowCall, TCapturedCall {
+  private CfgNodes::ExprNodes::CallCfgNode c;
+
+  CapturedCall() { this = TCapturedCall(c) }
+
+  /** Gets a target, in which a captured variable is referenced. */
+  Callable getATarget() {
+    exists(Ssa::Definition def | result = def.getScope() |
+      SsaImpl::captureFlowIn(c, _, _, _, def)
+      or
+      SsaImpl::captureFlowOut(c, def, _)
+    )
+  }
+
+  override CfgNodes::ExprNodes::CallCfgNode asCall() { none() }
+
+  override DataFlowCallable getEnclosingCallable() { result = TCfgScope(c.getScope()) }
+
+  override string toString() { result = "<captured> " + c.toString() }
+
+  override Location getLocation() { result = c.getLocation() }
+}
+
 /** A call for which we want to compute call targets. */
 private class RelevantCall extends CfgNodes::ExprNodes::CallCfgNode {
   pragma[nomagic]
@@ -341,6 +388,11 @@ private module Cached {
     TNormalCall(CfgNodes::ExprNodes::CallCfgNode c) or
     TSummaryCall(FlowSummaryImpl::Public::SummarizedCallable c, DataFlow::Node receiver) {
       FlowSummaryImpl::Private::summaryCallbackRange(c, receiver)
+    } or
+    TCapturedCall(CfgNodes::ExprNodes::CallCfgNode c) {
+      SsaImpl::captureFlowIn(c, _, _, _, _)
+      or
+      SsaImpl::captureFlowOut(c, _, _)
     }
 
   pragma[nomagic]
@@ -478,6 +530,8 @@ private module Cached {
     result = viableSourceCallable(call)
     or
     result = viableLibraryCallable(call)
+    or
+    result.asCallable() = call.(CapturedCall).getATarget()
   }
 
   cached
@@ -499,7 +553,13 @@ private module Cached {
     THashSplatArgumentPosition() or
     TSplatAllArgumentPosition() or
     TAnyArgumentPosition() or
-    TAnyKeywordArgumentPosition()
+    TAnyKeywordArgumentPosition() or
+    TCapturedArgumentPosition(LocalVariable v) {
+      exists(Ssa::Definition def |
+        SsaImpl::captureFlowIn(_, _, _, _, def) and
+        v = def.getSourceVariable()
+      )
+    }
 
   cached
   newtype TParameterPosition =
@@ -521,7 +581,13 @@ private module Cached {
     THashSplatParameterPosition() or
     TSplatAllParameterPosition() or
     TAnyParameterPosition() or
-    TAnyKeywordParameterPosition()
+    TAnyKeywordParameterPosition() or
+    TCapturedParameterPosition(LocalVariable v) {
+      exists(Ssa::Definition def |
+        SsaImpl::captureFlowIn(_, _, _, _, def) and
+        v = def.getSourceVariable()
+      )
+    }
 }
 
 import Cached
@@ -921,7 +987,7 @@ private predicate paramReturnFlow(
   DataFlow::Node nodeFrom, DataFlow::PostUpdateNode nodeTo, StepSummary summary
 ) {
   exists(RelevantCall call, DataFlow::Node arg, DataFlow::ParameterNode p, Expr nodeFromPreExpr |
-    TypeTrackerSpecific::callStep(call, arg, p) and
+    TypeTrackerSpecific::callStep(TNormalCall(call), arg, p) and
     nodeTo.getPreUpdateNode() = arg and
     summary.toString() = "return" and
     (
@@ -1151,6 +1217,9 @@ class ParameterPosition extends TParameterPosition {
   /** Holds if this position represents any positional parameter. */
   predicate isAnyNamed() { this = TAnyKeywordParameterPosition() }
 
+  /** Holds if this position represents a captured variable `v`. */
+  predicate isCapturedVariable(LocalVariable v) { this = TCapturedParameterPosition(v) }
+
   /** Gets a textual representation of this position. */
   string toString() {
     this.isSelf() and result = "self"
@@ -1170,6 +1239,8 @@ class ParameterPosition extends TParameterPosition {
     this.isAny() and result = "any"
     or
     this.isAnyNamed() and result = "any-named"
+    or
+    exists(LocalVariable v | this.isCapturedVariable(v) and result = "captured " + v)
   }
 }
 
@@ -1196,6 +1267,9 @@ class ArgumentPosition extends TArgumentPosition {
   /** Holds if this position represents any positional parameter. */
   predicate isAnyNamed() { this = TAnyKeywordArgumentPosition() }
 
+  /** Holds if this position represents a captured variable `v`. */
+  predicate isCapturedVariable(LocalVariable v) { this = TCapturedArgumentPosition(v) }
+
   /**
    * Holds if this position represents a synthesized argument containing all keyword
    * arguments wrapped in a hash.
@@ -1221,6 +1295,8 @@ class ArgumentPosition extends TArgumentPosition {
     this.isHashSplat() and result = "**"
     or
     this.isSplatAll() and result = "*"
+    or
+    exists(LocalVariable v | this.isCapturedVariable(v) and result = "captured " + v)
   }
 }
 
@@ -1256,4 +1332,6 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   ppos.isAnyNamed() and apos.isKeyword(_)
   or
   apos.isAnyNamed() and ppos.isKeyword(_)
+  or
+  exists(LocalVariable v | apos.isCapturedVariable(v) and ppos.isCapturedVariable(v))
 }