Ruby: Do not cache getAMaybeGuardedCapturedDef

Author: hvitved

ruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll

@@ -0,0 +1,11 @@
+private import codeql.ruby.CFG
+
+/** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
+pragma[nomagic]
+predicate guardControlsBlock(CfgNodes::AstCfgNode guard, BasicBlock bb, boolean branch) {
+  exists(ConditionBlock conditionBlock, SuccessorTypes::ConditionalSuccessor s |
+    guard = conditionBlock.getLastNode() and
+    s.getValue() = branch and
+    conditionBlock.controls(bb, s)
+  )
+}

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

@@ -854,13 +854,21 @@ class ContentSet extends TContentSet {
  */
 signature predicate guardChecksSig(CfgNodes::AstCfgNode g, CfgNode e, boolean branch);
 
+bindingset[def1, def2]
+pragma[inline_late]
+private predicate sameSourceVariable(Ssa::Definition def1, Ssa::Definition def2) {
+  def1.getSourceVariable() = def2.getSourceVariable()
+}
+
 /**
  * Provides a set of barrier nodes for a guard that validates an expression.
  *
  * This is expected to be used in `isBarrier`/`isSanitizer` definitions
  * in data flow and taint tracking.
  */
 module BarrierGuard<guardChecksSig/3 guardChecks> {
+  private import codeql.ruby.controlflow.internal.Guards
+
   /**
    * Gets an implicit entry definition for a captured variable that
    * may be guarded, because a call to the capturing callable is guarded.
@@ -870,9 +878,15 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
    */
   pragma[nomagic]
   private Ssa::CapturedEntryDefinition getAMaybeGuardedCapturedDef() {
-    exists(CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def |
-      result = SsaImpl::getAMaybeGuardedCapturedDef(g, branch, def) and
-      guardChecks(g, def.getARead(), branch)
+    exists(
+      CfgNodes::ExprCfgNode g, boolean branch, CfgNodes::ExprCfgNode testedNode,
+      Ssa::Definition def, CfgNodes::ExprNodes::CallCfgNode call
+    |
+      def.getARead() = testedNode and
+      guardChecks(g, testedNode, branch) and
+      guardControlsBlock(g, call.getBasicBlock(), branch) and
+      result.getBasicBlock().getScope() = call.getExpr().(MethodCall).getBlock() and
+      sameSourceVariable(def, result)
     )
   }
 

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

@@ -439,30 +439,6 @@ private module Cached {
       result = DataFlowIntegrationImpl::getABarrierNode(guard, def, branch)
     }
   }
-
-  bindingset[def1, def2]
-  pragma[inline_late]
-  private predicate sameSourceVariable(Ssa::Definition def1, Ssa::Definition def2) {
-    def1.getSourceVariable() = def2.getSourceVariable()
-  }
-
-  /**
-   * Gets an implicit entry definition for a captured variable that
-   * may be guarded, because a call to the capturing callable is guarded.
-   *
-   * This is restricted to calls where the variable is captured inside a
-   * block.
-   */
-  cached
-  Ssa::CapturedEntryDefinition getAMaybeGuardedCapturedDef(
-    Cfg::CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def
-  ) {
-    exists(Cfg::CfgNodes::ExprNodes::CallCfgNode call |
-      DataFlowIntegrationInput::guardControlsBlock(g, call.getBasicBlock(), branch) and
-      result.getBasicBlock().getScope() = call.getExpr().(MethodCall).getBlock() and
-      sameSourceVariable(def, result)
-    )
-  }
 }
 
 import Cached
@@ -552,6 +528,8 @@ class ParameterExt extends TParameterExt {
 }
 
 private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig {
+  private import codeql.ruby.controlflow.internal.Guards as Guards
+
   class Parameter = ParameterExt;
 
   class Expr extends Cfg::CfgNodes::ExprCfgNode {
@@ -570,11 +548,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
 
   /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
   predicate guardControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {
-    exists(Cfg::ConditionBlock conditionBlock, Cfg::SuccessorTypes::ConditionalSuccessor s |
-      guard = conditionBlock.getLastNode() and
-      s.getValue() = branch and
-      conditionBlock.controls(bb, s)
-    )
+    Guards::guardControlsBlock(guard, bb, branch)
   }
 
   /** Gets an immediate conditional successor of basic block `bb`, if any. */