Merge branch 'main' into redsun82/cargo-upgrade

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

@@ -641,7 +641,7 @@ private module TrackInstanceInput implements CallGraphConstruction::InputSig {
     // type being checked against
     localFlowStep(nodeFrom, nodeTo, summary) and
     not hasAdjacentTypeCheckedRead(nodeTo) and
-    not TypeInference::asModulePattern(nodeTo.(SsaDefinitionExtNode).getDefinitionExt(), _)
+    not TypeInference::asModulePattern(nodeTo.(SsaDefinitionNodeImpl).getDefinition(), _)
   }
 
   predicate stepCall(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, StepSummary summary) {

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

@@ -93,9 +93,9 @@ module SsaFlow {
     result = TSelfToplevelParameterNode(p.asToplevelSelf())
   }
 
-  ParameterNodeImpl toParameterNodeImpl(SsaDefinitionExtNode node) {
+  ParameterNodeImpl toParameterNodeImpl(SsaDefinitionNodeImpl node) {
     exists(SsaImpl::WriteDefinition def, SsaImpl::ParameterExt p |
-      def = node.getDefinitionExt() and
+      def = node.getDefinition() and
       result = toParameterNode(p) and
       p.isInitializedBy(def)
     )
@@ -392,10 +392,10 @@ module VariableCapture {
 
   // From an assignment or implicit initialization of a captured variable to its flow-insensitive node
   private predicate flowInsensitiveWriteStep(
-    SsaDefinitionExtNode node1, CapturedVariableNode node2, CapturedVariable v
+    SsaDefinitionNodeImpl node1, CapturedVariableNode node2, CapturedVariable v
   ) {
     exists(CapturedSsaDefinitionExt def |
-      def = node1.getDefinitionExt() and
+      def = node1.getDefinition() and
       def.getSourceVariable() = v and
       (
         def instanceof Ssa::WriteDefinition
@@ -408,11 +408,11 @@ module VariableCapture {
 
   // From a captured variable node to its flow-sensitive capture nodes
   private predicate flowInsensitiveReadStep(
-    CapturedVariableNode node1, SsaDefinitionExtNode node2, CapturedVariable v
+    CapturedVariableNode node1, SsaDefinitionNodeImpl node2, CapturedVariable v
   ) {
     exists(CapturedSsaDefinitionExt def |
       node1.getVariable() = v and
-      def = node2.getDefinitionExt() and
+      def = node2.getDefinition() and
       def.getSourceVariable() = v and
       (
         def instanceof Ssa::CapturedCallDefinition
@@ -571,8 +571,8 @@ private module Cached {
   }
 
   /** Holds if `n` wraps an SSA definition without ingoing flow. */
-  private predicate entrySsaDefinition(SsaDefinitionExtNode n) {
-    n.getDefinitionExt() =
+  private predicate entrySsaDefinition(SsaDefinitionNodeImpl n) {
+    n.getDefinition() =
       any(SsaImpl::WriteDefinition def | not def.(Ssa::WriteDefinition).assigns(_))
   }
 
@@ -614,7 +614,7 @@ private module Cached {
     // to parameters (which are themselves local sources)
     entrySsaDefinition(n) and
     not exists(SsaImpl::ParameterExt p |
-      p.isInitializedBy(n.(SsaDefinitionExtNode).getDefinitionExt())
+      p.isInitializedBy(n.(SsaDefinitionNodeImpl).getDefinition())
     )
     or
     isStoreTargetNode(n)
@@ -749,101 +749,50 @@ predicate nodeIsHidden(Node n) {
 }
 
 /** An SSA node. */
-abstract class SsaNode extends NodeImpl, TSsaNode {
+class SsaNode extends NodeImpl, TSsaNode {
   SsaImpl::DataFlowIntegration::SsaNode node;
-  SsaImpl::DefinitionExt def;
 
-  SsaNode() {
-    this = TSsaNode(node) and
-    def = node.getDefinitionExt()
-  }
+  SsaNode() { this = TSsaNode(node) }
 
-  SsaImpl::DefinitionExt getDefinitionExt() { result = def }
+  /** Gets the underlying variable. */
+  Variable getVariable() { result = node.getSourceVariable() }
 
   /** Holds if this node should be hidden from path explanations. */
-  abstract predicate isHidden();
+  predicate isHidden() { any() }
+
+  override CfgScope getCfgScope() { result = node.getBasicBlock().getScope() }
 
   override Location getLocationImpl() { result = node.getLocation() }
 
   override string toStringImpl() { result = node.toString() }
 }
 
-/** An (extended) SSA definition, viewed as a node in a data flow graph. */
-class SsaDefinitionExtNode extends SsaNode {
-  override SsaImpl::DataFlowIntegration::SsaDefinitionExtNode node;
+class SsaDefinitionNodeImpl extends SsaNode {
+  override SsaImpl::DataFlowIntegration::SsaDefinitionNode node;
 
-  /** Gets the underlying variable. */
-  Variable getVariable() { result = def.getSourceVariable() }
+  SsaImpl::Definition getDefinition() { result = node.getDefinition() }
 
   override predicate isHidden() {
-    not def instanceof Ssa::WriteDefinition
-    or
-    isDesugarNode(def.(Ssa::WriteDefinition).getWriteAccess().getExpr())
-    or
-    def = getParameterDef(_)
+    exists(SsaImpl::Definition def | def = this.getDefinition() |
+      not def instanceof Ssa::WriteDefinition
+      or
+      isDesugarNode(def.(Ssa::WriteDefinition).getWriteAccess().getExpr())
+      or
+      def = getParameterDef(_)
+    )
   }
-
-  override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() }
 }
 
-class SsaDefinitionNodeImpl extends SsaDefinitionExtNode {
-  Ssa::Definition ssaDef;
-
-  SsaDefinitionNodeImpl() { ssaDef = def }
-
-  override Location getLocationImpl() { result = ssaDef.getLocation() }
-
-  override string toStringImpl() { result = ssaDef.toString() }
-}
-
-/**
- * A node that represents an input to an SSA phi (read) definition.
- *
- * This allows for barrier guards to filter input to phi nodes. For example, in
- *
- * ```rb
- * x = taint
- * if x != "safe" then
- *     x = "safe"
- * end
- * sink x
- * ```
- *
- * the `false` edge out of `x != "safe"` guards the input from `x = taint` into the
- * `phi` node after the condition.
- *
- * It is also relevant to filter input into phi read nodes:
- *
- * ```rb
- * x = taint
- * if b then
- *     if x != "safe1" then
- *         return
- *     end
- * else
- *     if x != "safe2" then
- *         return
- *     end
- * end
- *
- * sink x
- * ```
- *
- * both inputs into the phi read node after the outer condition are guarded.
- */
-class SsaInputNode extends SsaNode {
-  override SsaImpl::DataFlowIntegration::SsaInputNode node;
-
-  override predicate isHidden() { any() }
-
-  override CfgScope getCfgScope() { result = node.getDefinitionExt().getBasicBlock().getScope() }
+/** A synthesized SSA read. */
+class SsaSynthReadNode extends SsaNode {
+  override SsaImpl::DataFlowIntegration::SsaSynthReadNode node;
 }
 
 /** An SSA definition for a `self` variable. */
-class SsaSelfDefinitionNode extends SsaDefinitionExtNode {
+class SsaSelfDefinitionNode extends SsaDefinitionNodeImpl {
   private SelfVariable self;
 
-  SsaSelfDefinitionNode() { self = def.getSourceVariable() }
+  SsaSelfDefinitionNode() { self = super.getVariable() }
 
   /** Gets the scope in which the `self` variable is declared. */
   Scope getSelfScope() { result = self.getDeclaringScope() }
@@ -1976,9 +1925,9 @@ predicate localMustFlowStep(Node node1, Node node2) {
   or
   exists(SsaImpl::Definition def |
     def.(Ssa::WriteDefinition).assigns(node1.asExpr()) and
-    node2.(SsaDefinitionExtNode).getDefinitionExt() = def
+    node2.(SsaDefinitionNodeImpl).getDefinition() = def
     or
-    def = node1.(SsaDefinitionExtNode).getDefinitionExt() and
+    def = node1.(SsaDefinitionNodeImpl).getDefinition() and
     node2.asExpr() = SsaImpl::getARead(def)
   )
   or
@@ -2122,8 +2071,8 @@ class CastNode extends Node {
 predicate neverSkipInPathGraph(Node n) {
   // ensure that all variable assignments are included in the path graph
   n =
-    any(SsaDefinitionExtNode def |
-      def.getDefinitionExt() instanceof Ssa::WriteDefinition and
+    any(SsaDefinitionNodeImpl def |
+      def.getDefinition() instanceof Ssa::WriteDefinition and
       not def.isHidden()
     )
 }
@@ -2446,7 +2395,7 @@ module TypeInference {
   }
 
   pragma[nomagic]
-  private predicate ssaDefHasType(SsaDefinitionExtNode def, Module tp, boolean exact) {
+  private predicate ssaDefHasType(SsaDefinitionNodeImpl def, Module tp, boolean exact) {
     exists(ParameterNodeImpl p |
       parameterNodeHasType(p, tp, exact) and
       p = SsaFlow::toParameterNodeImpl(def)
@@ -2454,7 +2403,7 @@ module TypeInference {
     or
     selfInMethodOrToplevelHasType(def.getVariable(), tp, exact)
     or
-    asModulePattern(def.getDefinitionExt(), tp) and
+    asModulePattern(def.getDefinition(), tp) and
     exact = false
   }
 
@@ -2523,11 +2472,11 @@ module TypeInference {
     or
     parameterNodeHasType(n, tp, exact)
     or
-    exists(SsaDefinitionExtNode def | ssaDefHasType(def, tp, exact) |
+    exists(SsaDefinitionNodeImpl def | ssaDefHasType(def, tp, exact) |
       n = def or
       n.asExpr() =
         any(CfgNodes::ExprCfgNode read |
-          read = def.getDefinitionExt().getARead() and
+          read = def.getDefinition().(SsaImpl::DefinitionExt).getARead() and
           not isTypeCheckedRead(read, _) // could in principle be checked against a new type
         )
     )

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

@@ -363,7 +363,7 @@ class PostUpdateNode extends Node {
 /** An SSA definition, viewed as a node in a data flow graph. */
 class SsaDefinitionNode extends Node instanceof SsaDefinitionNodeImpl {
   /** Gets the underlying SSA definition. */
-  Ssa::Definition getDefinition() { result = super.getDefinitionExt() }
+  Ssa::Definition getDefinition() { result = super.getDefinition() }
 
   /** Gets the underlying variable. */
   Variable getVariable() { result = this.getDefinition().getSourceVariable() }
@@ -434,7 +434,7 @@ private module Cached {
   LocalSourceNode getConstantAccessNode(ConstantAccess access) {
     // Namespaces don't evaluate to the constant being accessed, they return the value of their last statement.
     // Use the definition of 'self' in the namespace as the representative in this case.
-    result.(SsaDefinitionExtNode).getDefinitionExt().(Ssa::SelfDefinition).getSourceVariable() =
+    result.(SsaDefinitionNode).getDefinition().(Ssa::SelfDefinition).getSourceVariable() =
       access.(Namespace).getModuleSelfVariable()
     or
     not access instanceof Namespace and
@@ -1002,7 +1002,7 @@ class ModuleNode instanceof Module {
    * This only gets `self` at the module level, not inside any (singleton) method.
    */
   LocalSourceNode getModuleLevelSelf() {
-    result.(SsaDefinitionExtNode).getVariable() = super.getADeclaration().getModuleSelfVariable()
+    result.(SsaDefinitionNode).getVariable() = super.getADeclaration().getModuleSelfVariable()
   }
 
   /**

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

@@ -88,7 +88,7 @@ private module Cached {
         nodeFrom.asExpr() = value and
         value = case.getValue() and
         clause = case.getBranch(_) and
-        def = nodeTo.(SsaDefinitionExtNode).getDefinitionExt() and
+        def = nodeTo.(SsaDefinitionNodeImpl).getDefinition() and
         def.getControlFlowNode() = variablesInPattern(clause.getPattern()) and
         not def.(Ssa::WriteDefinition).assigns(value)
       )

ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected

@@ -2,40 +2,87 @@ testFailures
 newStyleBarrierGuards
 | barrier-guards.rb:3:16:4:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:4:5:4:7 | foo |
+| barrier-guards.rb:9:25:10:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:10:5:10:7 | foo |
+| barrier-guards.rb:17:1:18:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:18:5:18:7 | foo |
+| barrier-guards.rb:23:1:24:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:24:5:24:7 | foo |
+| barrier-guards.rb:27:20:28:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:28:5:28:7 | foo |
+| barrier-guards.rb:37:21:38:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:38:5:38:7 | foo |
+| barrier-guards.rb:43:16:46:5 | [input] SSA phi read(foo) |
 | barrier-guards.rb:45:9:45:11 | foo |
 | barrier-guards.rb:70:22:71:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:71:5:71:7 | foo |
+| barrier-guards.rb:82:26:83:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:83:5:83:7 | foo |
+| barrier-guards.rb:90:1:91:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:91:5:91:7 | foo |
+| barrier-guards.rb:125:11:126:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:126:5:126:7 | foo |
+| barrier-guards.rb:132:11:133:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:133:5:133:7 | foo |
+| barrier-guards.rb:134:11:135:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:135:5:135:7 | foo |
+| barrier-guards.rb:139:18:140:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:140:5:140:7 | foo |
+| barrier-guards.rb:141:19:142:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:142:5:142:7 | foo |
+| barrier-guards.rb:148:21:149:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:149:5:149:7 | foo |
+| barrier-guards.rb:153:18:154:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:154:5:154:7 | foo |
+| barrier-guards.rb:158:10:159:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:159:5:159:7 | foo |
+| barrier-guards.rb:163:11:164:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:164:5:164:7 | foo |
+| barrier-guards.rb:191:4:191:15 | [input] SSA phi read(foo) |
+| barrier-guards.rb:191:20:191:31 | [input] SSA phi read(foo) |
+| barrier-guards.rb:191:32:192:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:192:5:192:7 | foo |
+| barrier-guards.rb:195:4:195:15 | [input] SSA phi read(foo) |
+| barrier-guards.rb:195:4:195:31 | [input] SSA phi read(foo) |
+| barrier-guards.rb:195:20:195:31 | [input] SSA phi read(foo) |
+| barrier-guards.rb:195:36:195:47 | [input] SSA phi read(foo) |
+| barrier-guards.rb:195:48:196:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:196:5:196:7 | foo |
+| barrier-guards.rb:199:4:199:15 | [input] SSA phi read(foo) |
+| barrier-guards.rb:199:4:199:31 | [input] SSA phi read(foo) |
+| barrier-guards.rb:199:20:199:31 | [input] SSA phi read(foo) |
+| barrier-guards.rb:203:4:203:15 | [input] SSA phi read(foo) |
+| barrier-guards.rb:203:36:203:47 | [input] SSA phi read(foo) |
+| barrier-guards.rb:207:21:207:21 | [input] SSA phi read(foo) |
+| barrier-guards.rb:207:22:208:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:208:5:208:7 | foo |
+| barrier-guards.rb:211:22:212:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:212:5:212:7 | foo |
+| barrier-guards.rb:215:28:216:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:216:5:216:7 | foo |
 | barrier-guards.rb:219:21:219:23 | foo |
+| barrier-guards.rb:219:21:219:32 | [input] SSA phi read(foo) |
+| barrier-guards.rb:219:95:220:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:220:5:220:7 | foo |
+| barrier-guards.rb:227:21:227:21 | [input] SSA phi read(foo) |
+| barrier-guards.rb:227:22:228:7 | [input] SSA phi read(foo) |
+| barrier-guards.rb:232:18:233:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:233:5:233:7 | foo |
+| barrier-guards.rb:237:19:237:38 | [input] SSA phi read(foo) |
 | barrier-guards.rb:237:24:237:26 | foo |
+| barrier-guards.rb:243:9:244:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:244:5:244:7 | foo |
+| barrier-guards.rb:259:17:260:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:260:5:260:7 | foo |
+| barrier-guards.rb:264:17:265:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:265:5:265:7 | foo |
+| barrier-guards.rb:272:17:272:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:272:17:272:19 | foo |
+| barrier-guards.rb:275:20:276:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:276:5:276:7 | foo |
+| barrier-guards.rb:281:21:282:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:282:5:282:7 | foo |
+| barrier-guards.rb:291:7:292:19 | [input] SSA phi read(foo) |
 | barrier-guards.rb:292:5:292:7 | foo |
 | barrier_flow.rb:19:14:19:14 | x |
 | barrier_flow.rb:32:10:32:10 | x |

ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql

@@ -26,7 +26,7 @@ module BarrierGuardTest implements TestSig {
     tag = "guarded" and
     exists(DataFlow::Node n |
       newStyleBarrierGuards(n) and
-      not n instanceof SsaInputNode and
+      not n instanceof SsaSynthReadNode and
       location = n.getLocation() and
       element = n.toString() and
       value = ""