wip

Author: hvitved

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

@@ -176,9 +176,6 @@ module Ssa {
 
     override string toString() { result = this.getControlFlowNode().toString() }
 
-    /** 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() }
   }

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

@@ -87,39 +87,37 @@ class SsaInputDefinitionExt extends SsaImpl::DefinitionExt {
 
 /** Provides predicates related to local data flow. */
 module LocalFlow {
-  /**
-   * Holds if `nodeFrom` is a node for SSA definition `def`, which can reach `next`.
-   */
-  pragma[nomagic]
-  private predicate localFlowSsaInputFromDef(
-    SsaDefinitionExtNode nodeFrom, SsaImpl::DefinitionExt def, SsaInputNode nodeTo
-  ) {
-    exists(BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next |
-      next.hasInputFromBlock(def, bb, i, input) and
-      def = nodeFrom.getDefinitionExt() and
-      def.definesAt(_, bb, i, _) and
-      nodeTo = TSsaInputNode(next, input)
-    )
-  }
-
-  /**
-   * Holds if `nodeFrom` is a last read of SSA definition `def`, which
-   * can reach `nodeTo`.
-   */
-  pragma[nomagic]
-  predicate localFlowSsaInputFromRead(SsaImpl::DefinitionExt def, Node nodeFrom, SsaInputNode nodeTo) {
-    exists(
-      BasicBlock bb, int i, CfgNodes::ExprCfgNode exprFrom, BasicBlock input,
-      SsaInputDefinitionExt next
-    |
-      next.hasInputFromBlock(def, bb, i, input) and
-      exprFrom = bb.getNode(i) and
-      exprFrom.getExpr() instanceof VariableReadAccess and
-      exprFrom = [nodeFrom.asExpr(), nodeFrom.(PostUpdateNodeImpl).getPreUpdateNode().asExpr()] and
-      nodeTo = TSsaInputNode(next, input)
-    )
-  }
-
+  // /**
+  //  * Holds if `nodeFrom` is a node for SSA definition `def`, which can reach `next`.
+  //  */
+  // pragma[nomagic]
+  // private predicate localFlowSsaInputFromDef(
+  //   SsaDefinitionExtNode nodeFrom, SsaImpl::DefinitionExt def, SsaInputNode nodeTo
+  // ) {
+  //   exists(BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next |
+  //     next.hasInputFromBlock(def, bb, i, input) and
+  //     def = nodeFrom.getDefinitionExt() and
+  //     def.definesAt(_, bb, i, _) and
+  //     nodeTo = TSsaInputNode(next, input)
+  //   )
+  // }
+  // /**
+  //  * Holds if `nodeFrom` is a last read of SSA definition `def`, which
+  //  * can reach `nodeTo`.
+  //  */
+  // pragma[nomagic]
+  // predicate localFlowSsaInputFromRead(SsaImpl::DefinitionExt def, Node nodeFrom, SsaInputNode nodeTo) {
+  //   exists(
+  //     BasicBlock bb, int i, CfgNodes::ExprCfgNode exprFrom, BasicBlock input,
+  //     SsaInputDefinitionExt next
+  //   |
+  //     next.hasInputFromBlock(def, bb, i, input) and
+  //     exprFrom = bb.getNode(i) and
+  //     exprFrom.getExpr() instanceof VariableReadAccess and
+  //     exprFrom = [nodeFrom.asExpr(), nodeFrom.(PostUpdateNodeImpl).getPreUpdateNode().asExpr()] and
+  //     nodeTo = TSsaInputNode(next, input)
+  //   )
+  // }
   /** Gets the SSA definition node corresponding to parameter `p`. */
   pragma[nomagic]
   SsaDefinitionExtNode getParameterDefNode(NamedParameter p) {
@@ -139,14 +137,13 @@ module LocalFlow {
     nodeTo.getDefinitionExt() = nodeFrom.(SelfParameterNodeImpl).getSelfDefinition()
   }
 
-  /**
-   * Holds if there is a local use-use flow step from `nodeFrom` to `nodeTo`
-   * involving SSA definition `def`.
-   */
-  predicate localSsaFlowStepUseUse(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    SsaImpl::adjacentReadPairExt(def, nodeFrom.asExpr(), nodeTo.asExpr())
-  }
-
+  // /**
+  //  * Holds if there is a local use-use flow step from `nodeFrom` to `nodeTo`
+  //  * involving SSA definition `def`.
+  //  */
+  // predicate localSsaFlowStepUseUse(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
+  //   SsaImpl::adjacentReadPairExt(def, nodeFrom.asExpr(), nodeTo.asExpr())
+  // }
   /**
    * Holds if SSA definition `def` assigns `value` to the underlying variable.
    *
@@ -173,33 +170,32 @@ module LocalFlow {
     )
   }
 
-  /**
-   * Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
-   * SSA definition `def`.
-   */
-  pragma[nomagic]
-  predicate localSsaFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    // Flow from assignment into SSA definition
-    ssaDefAssigns(def, nodeFrom.asExpr()) and
-    nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def
-    or
-    // Flow from SSA definition to first read
-    def = nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() and
-    SsaImpl::firstReadExt(def, nodeTo.asExpr())
-    or
-    // Flow from post-update read to next read
-    localSsaFlowStepUseUse(def, nodeFrom.(PostUpdateNodeImpl).getPreUpdateNode(), nodeTo)
-    or
-    // Flow into phi (read) SSA definition node from def
-    localFlowSsaInputFromDef(nodeFrom, def, nodeTo)
-    or
-    nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and
-    def = nodeFrom.(SsaInputNode).getDefinitionExt()
-    or
-    localFlowSsaParamInput(nodeFrom, nodeTo) and
-    def = nodeTo.(SsaDefinitionExtNode).getDefinitionExt()
-  }
-
+  // /**
+  //  * Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
+  //  * SSA definition `def`.
+  //  */
+  // pragma[nomagic]
+  // predicate localSsaFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
+  //   // // Flow from assignment into SSA definition
+  //   // ssaDefAssigns(def, nodeFrom.asExpr()) and
+  //   // nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def
+  //   // or
+  //   // // Flow from SSA definition to first read
+  //   // def = nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() and
+  //   // SsaImpl::firstReadExt(def, nodeTo.asExpr())
+  //   // or
+  //   // // Flow from post-update read to next read
+  //   // localSsaFlowStepUseUse(def, nodeFrom.(PostUpdateNodeImpl).getPreUpdateNode(), nodeTo)
+  //   // or
+  //   // // Flow into phi (read) SSA definition node from def
+  //   // localFlowSsaInputFromDef(nodeFrom, def, nodeTo)
+  //   // or
+  //   // nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and
+  //   // def = nodeFrom.(SsaInputNode).getDefinitionExt()
+  //   // or
+  //   localFlowSsaParamInput(nodeFrom, nodeTo) and
+  //   def = nodeTo.(SsaDefinitionExtNode).getDefinitionExt()
+  // }
   pragma[nomagic]
   predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
     nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::BlockArgumentCfgNode).getValue()
@@ -338,6 +334,70 @@ predicate isNonConstantExpr(CfgNodes::ExprCfgNode n) {
   not n.getExpr() instanceof ConstantAccess
 }
 
+private module SsaIntegration {
+  private module Input implements SsaImpl::Impl::DataFlowIntegrationInputSig {
+    private import ruby::Ast as R
+
+    class Expr extends CfgNodes::ExprCfgNode {
+      predicate hasCfgNode(SsaImpl::SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) }
+
+      predicate hasPostUpdateNode() { this = any(ExprPostUpdateNode n).getPreUpdateNode().asExpr() }
+    }
+
+    /**
+     * Holds if SSA definition `def` assigns `value` to the underlying variable.
+     *
+     * This is either a direct assignment, `x = value`, or an assignment via
+     * simple pattern matching
+     *
+     * ```rb
+     * case value
+     *  in Foo => x then ...
+     *  in y => then ...
+     * end
+     * ```
+     */
+    predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) {
+      def.(Ssa::WriteDefinition).assigns(value)
+      or
+      exists(CfgNodes::ExprNodes::CaseExprCfgNode case, CfgNodes::AstCfgNode pattern |
+        case.getValue() = value and
+        pattern = case.getBranch(_).(CfgNodes::ExprNodes::InClauseCfgNode).getPattern()
+      |
+        def.(Ssa::WriteDefinition).getWriteAccess() =
+          [pattern, pattern.(CfgNodes::ExprNodes::AsPatternCfgNode).getVariableAccess()]
+      )
+    }
+
+    class Parameter = ParameterNodeImpl;
+
+    predicate ssaDefInitializesParam(SsaImpl::WriteDefinition def, ParameterNodeImpl p) {
+      exists(BasicBlock bb, int i |
+        bb.getNode(i).getAstNode() = p.getParameter().(NamedParameter).getDefiningAccess() and
+        def.definesAt(_, bb, i)
+      )
+      or
+      def = p.(SelfParameterNodeImpl).getSelfDefinition()
+    }
+  }
+
+  module Impl = SsaImpl::Impl::DataFlowIntegration<Input>;
+
+  private Impl::Node asNode(Node n) {
+    result = n.(SsaNode).getSsaNode()
+    or
+    result.(Impl::ExprNode).getExpr() = n.asExpr()
+    or
+    result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
+    or
+    result.(Impl::ParameterNode).getParameter() = n
+  }
+
+  predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
+    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
+  }
+}
+
 /** Provides logic related to captured variables. */
 module VariableCapture {
   private import codeql.dataflow.VariableCapture as Shared
@@ -359,6 +419,8 @@ module VariableCapture {
 
     class BasicBlock extends BasicBlocks::BasicBlock {
       Callable getEnclosingCallable() { result = this.getScope() }
+
+      Location getLocation(int i) { result = this.getNode(i).getLocation() }
     }
 
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
@@ -537,10 +599,11 @@ private module Cached {
   newtype TNode =
     TExprNode(CfgNodes::ExprCfgNode n) or
     TReturningNode(CfgNodes::ReturningCfgNode n) { exists(n.getReturnedValueNode()) } or
-    TSsaDefinitionExtNode(SsaImpl::DefinitionExt def) or
-    TSsaInputNode(SsaInputDefinitionExt def, BasicBlock input) {
-      def.hasInputFromBlock(_, _, _, input)
-    } or
+    TSsaNode(SsaIntegration::Impl::SsaNode node) or
+    // TSsaDefinitionExtNode(SsaImpl::DefinitionExt def) or
+    // TSsaInputNode(SsaInputDefinitionExt def, BasicBlock input) {
+    //   def.hasInputFromBlock(_, _, _, input)
+    // } or
     TCapturedVariableNode(VariableCapture::CapturedVariable v) or
     TNormalParameterNode(Parameter p) {
       p instanceof SimpleParameter or
@@ -621,13 +684,16 @@ private module Cached {
         // captured variables are handled by the shared `VariableCapture` library
         not def instanceof VariableCapture::CapturedSsaDefinitionExt
       |
-        LocalFlow::localSsaFlowStep(def, nodeFrom, nodeTo)
-        or
-        LocalFlow::localSsaFlowStepUseUse(def, nodeFrom, nodeTo) and
-        not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
-        or
-        LocalFlow::localFlowSsaInputFromRead(def, nodeFrom, nodeTo) and
+        SsaIntegration::localFlowStep(def, nodeFrom, nodeTo) and
         not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
+        // // SsaIntegration::Impl::localFlowStep(def, nodeFrom, nodeTo)
+        // LocalFlow::localSsaFlowStep(def, nodeFrom, nodeTo)
+        // or
+        // LocalFlow::localSsaFlowStepUseUse(def, nodeFrom, nodeTo) and
+        // not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
+        // or
+        // LocalFlow::localFlowSsaInputFromRead(def, nodeFrom, nodeTo) and
+        // not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
       )
       or
       VariableCapture::valueStep(nodeFrom, nodeTo)
@@ -642,10 +708,11 @@ private module Cached {
   predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) {
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
     or
-    LocalFlow::localSsaFlowStep(_, nodeFrom, nodeTo)
-    or
-    LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo)
+    // LocalFlow::localSsaFlowStep(_, nodeFrom, nodeTo)
+    SsaIntegration::localFlowStep(_, nodeFrom, nodeTo)
     or
+    // LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo)
+    // or
     // Simple flow through library code is included in the exposed local
     // step relation, even though flow is technically inter-procedural
     FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo, _)
@@ -658,12 +725,13 @@ private module Cached {
   predicate localFlowStepTypeTracker(Node nodeFrom, Node nodeTo) {
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
     or
-    LocalFlow::localSsaFlowStep(_, nodeFrom, nodeTo)
-    or
-    LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo)
-    or
-    LocalFlow::localFlowSsaInputFromRead(_, nodeFrom, nodeTo)
+    SsaIntegration::localFlowStep(_, nodeFrom, nodeTo)
     or
+    // LocalFlow::localSsaFlowStep(_, nodeFrom, nodeTo)
+    // or
+    // LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo)
+    // or
+    // LocalFlow::localFlowSsaInputFromRead(_, nodeFrom, nodeTo)
     VariableCapture::flowInsensitiveStep(nodeFrom, nodeTo)
     or
     LocalFlow::flowSummaryLocalStep(nodeFrom, nodeTo, any(LibraryCallableToIncludeInTypeTracking c),
@@ -862,14 +930,28 @@ predicate nodeIsHidden(Node n) {
 }
 
 /** An SSA definition, viewed as a node in a data flow graph. */
-class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode {
+abstract class SsaNode extends NodeImpl, TSsaNode {
+  SsaIntegration::Impl::SsaNode node;
   SsaImpl::DefinitionExt def;
 
-  SsaDefinitionExtNode() { this = TSsaDefinitionExtNode(def) }
+  SsaNode() {
+    this = TSsaNode(node) and
+    def = node.getDefinitionExt()
+  }
+
+  SsaIntegration::Impl::SsaNode getSsaNode() { result = node }
 
-  /** Gets the underlying SSA definition. */
   SsaImpl::DefinitionExt getDefinitionExt() { result = def }
 
+  override Location getLocationImpl() { result = node.getLocation() }
+
+  override string toStringImpl() { result = node.toString() }
+}
+
+/** An SSA definition, viewed as a node in a data flow graph. */
+class SsaDefinitionExtNode extends SsaNode {
+  override SsaIntegration::Impl::SsaDefinitionExtNode node;
+
   /** Gets the underlying variable. */
   Variable getVariable() { result = def.getSourceVariable() }
 
@@ -881,10 +963,16 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode {
   }
 
   override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() }
+}
+
+class SsaDefinitionNodeImpl extends SsaDefinitionExtNode {
+  Ssa::Definition ssaDef;
 
-  override Location getLocationImpl() { result = def.getLocation() }
+  SsaDefinitionNodeImpl() { ssaDef = def }
 
-  override string toStringImpl() { result = def.toString() }
+  override Location getLocationImpl() { result = ssaDef.getLocation() }
+
+  override string toStringImpl() { result = ssaDef.toString() }
 }
 
 /**
@@ -922,20 +1010,20 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode {
  *
  * both inputs into the phi read node after the outer condition are guarded.
  */
-class SsaInputNode extends NodeImpl, TSsaInputNode {
-  SsaImpl::DefinitionExt def;
+class SsaInputNode extends SsaNode {
+  override SsaIntegration::Impl::SsaInputNode node;
   BasicBlock input;
 
-  SsaInputNode() { this = TSsaInputNode(def, input) }
+  SsaInputNode() { node.hasInputFromBlock(def, input) }
 
-  /** Gets the underlying SSA definition. */
-  SsaImpl::DefinitionExt getDefinitionExt() { result = def }
+  predicate isInputInto(BasicBlock bb, SsaImpl::DefinitionExt target) {
+    bb = input and
+    target = def
+  }
 
   override CfgScope getCfgScope() { result = input.getScope() }
-
-  override Location getLocationImpl() { result = input.getLastNode().getLocation() }
-
-  override string toStringImpl() { result = "[input] " + def }
+  // override Location getLocationImpl() { result = input.getLastNode().getLocation() }
+  // override string toStringImpl() { result = "[input] " + def }
 }
 
 /** An SSA definition for a `self` variable. */