Ruby: Disjunctive barrier guards for free

Author: hvitved

Diff for: ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll

@@ -31,30 +31,6 @@ private predicate stringConstCompare(CfgNodes::AstCfgNode guard, CfgNode testedN
   )
   or
   stringConstCaseCompare(guard, testedNode, branch)
-  or
-  exists(CfgNodes::ExprNodes::BinaryOperationCfgNode g |
-    g = guard and
-    stringConstCompareOr(guard, branch) and
-    stringConstCompare(g.getLeftOperand(), testedNode, _)
-  )
-}
-
-/**
- * Holds if `guard` is an `or` expression whose operands are string comparison guards.
- * For example:
- *
- * ```rb
- * x == "foo" or x == "bar"
- * ```
- */
-private predicate stringConstCompareOr(
-  CfgNodes::ExprNodes::BinaryOperationCfgNode guard, boolean branch
-) {
-  guard.getExpr() instanceof LogicalOrExpr and
-  branch = true and
-  forall(CfgNode innerGuard | innerGuard = guard.getAnOperand() |
-    stringConstCompare(innerGuard, any(Ssa::Definition def).getARead(), branch)
-  )
 }
 
 /**

Diff for: ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll

@@ -560,6 +560,30 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
     predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) }
   }
 
+  class AndGuard extends Guard instanceof Cfg::CfgNodes::ExprNodes::BinaryOperationCfgNode {
+    AndGuard() { this.getExpr() instanceof LogicalAndExpr }
+
+    Guard getLeftOperand() {
+      result = Cfg::CfgNodes::ExprNodes::BinaryOperationCfgNode.super.getLeftOperand()
+    }
+
+    Guard getRightOperand() {
+      result = Cfg::CfgNodes::ExprNodes::BinaryOperationCfgNode.super.getRightOperand()
+    }
+  }
+
+  class OrGuard extends Guard, Cfg::CfgNodes::ExprNodes::BinaryOperationCfgNode {
+    OrGuard() { this.getExpr() instanceof LogicalOrExpr }
+
+    Guard getLeftOperand() {
+      result = Cfg::CfgNodes::ExprNodes::BinaryOperationCfgNode.super.getLeftOperand()
+    }
+
+    Guard getRightOperand() {
+      result = Cfg::CfgNodes::ExprNodes::BinaryOperationCfgNode.super.getRightOperand()
+    }
+  }
+
   /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
   predicate guardControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {
     Guards::guardControlsBlock(guard, bb, branch)

Diff for: ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-flow.expected

@@ -10,7 +10,6 @@ edges
 | barrier_flow.rb:82:5:82:5 | x | barrier_flow.rb:93:14:93:14 | x | provenance |  |
 | barrier_flow.rb:82:5:82:5 | x | barrier_flow.rb:99:14:99:14 | x | provenance |  |
 | barrier_flow.rb:82:5:82:5 | x | barrier_flow.rb:103:14:103:14 | x | provenance |  |
-| barrier_flow.rb:82:5:82:5 | x | barrier_flow.rb:105:14:105:14 | x | provenance |  |
 | barrier_flow.rb:82:9:82:18 | call to source | barrier_flow.rb:82:5:82:5 | x | provenance |  |
 | barrier_flow.rb:110:5:110:5 | x | barrier_flow.rb:115:14:115:14 | x | provenance |  |
 | barrier_flow.rb:110:5:110:5 | x | barrier_flow.rb:119:14:119:14 | x | provenance |  |
@@ -33,7 +32,6 @@ nodes
 | barrier_flow.rb:93:14:93:14 | x | semmle.label | x |
 | barrier_flow.rb:99:14:99:14 | x | semmle.label | x |
 | barrier_flow.rb:103:14:103:14 | x | semmle.label | x |
-| barrier_flow.rb:105:14:105:14 | x | semmle.label | x |
 | barrier_flow.rb:110:5:110:5 | x | semmle.label | x |
 | barrier_flow.rb:110:9:110:18 | call to source | semmle.label | call to source |
 | barrier_flow.rb:115:14:115:14 | x | semmle.label | x |
@@ -42,7 +40,6 @@ nodes
 | barrier_flow.rb:131:14:131:14 | x | semmle.label | x |
 subpaths
 testFailures
-| barrier_flow.rb:105:14:105:14 | x | Unexpected result: hasValueFlow=10 |
 #select
 | barrier_flow.rb:4:10:4:10 | x | barrier_flow.rb:2:9:2:17 | call to source | barrier_flow.rb:4:10:4:10 | x | $@ | barrier_flow.rb:2:9:2:17 | call to source | call to source |
 | barrier_flow.rb:11:14:11:14 | x | barrier_flow.rb:8:9:8:17 | call to source | barrier_flow.rb:11:14:11:14 | x | $@ | barrier_flow.rb:8:9:8:17 | call to source | call to source |
@@ -51,7 +48,6 @@ testFailures
 | barrier_flow.rb:93:14:93:14 | x | barrier_flow.rb:82:9:82:18 | call to source | barrier_flow.rb:93:14:93:14 | x | $@ | barrier_flow.rb:82:9:82:18 | call to source | call to source |
 | barrier_flow.rb:99:14:99:14 | x | barrier_flow.rb:82:9:82:18 | call to source | barrier_flow.rb:99:14:99:14 | x | $@ | barrier_flow.rb:82:9:82:18 | call to source | call to source |
 | barrier_flow.rb:103:14:103:14 | x | barrier_flow.rb:82:9:82:18 | call to source | barrier_flow.rb:103:14:103:14 | x | $@ | barrier_flow.rb:82:9:82:18 | call to source | call to source |
-| barrier_flow.rb:105:14:105:14 | x | barrier_flow.rb:82:9:82:18 | call to source | barrier_flow.rb:105:14:105:14 | x | $@ | barrier_flow.rb:82:9:82:18 | call to source | call to source |
 | barrier_flow.rb:115:14:115:14 | x | barrier_flow.rb:110:9:110:18 | call to source | barrier_flow.rb:115:14:115:14 | x | $@ | barrier_flow.rb:110:9:110:18 | call to source | call to source |
 | barrier_flow.rb:119:14:119:14 | x | barrier_flow.rb:110:9:110:18 | call to source | barrier_flow.rb:119:14:119:14 | x | $@ | barrier_flow.rb:110:9:110:18 | call to source | call to source |
 | barrier_flow.rb:125:14:125:14 | x | barrier_flow.rb:110:9:110:18 | call to source | barrier_flow.rb:125:14:125:14 | x | $@ | barrier_flow.rb:110:9:110:18 | call to source | call to source |

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

@@ -1,5 +1,4 @@
 testFailures
-| barrier_flow.rb:105:16:105:26 | # $ guarded | Missing result:guarded= |
 failures
 newStyleBarrierGuards
 | barrier-guards.rb:3:16:4:19 | [input] SSA phi read(foo) |
@@ -54,6 +53,7 @@ newStyleBarrierGuards
 | barrier_flow.rb:91:14:91:14 | x |
 | barrier_flow.rb:96:24:96:24 | x |
 | barrier_flow.rb:97:14:97:14 | x |
+| barrier_flow.rb:105:14:105:14 | x |
 | barrier_flow.rb:112:8:112:19 | [input] SSA phi read(x) |
 | barrier_flow.rb:112:24:112:35 | [input] SSA phi read(x) |
 | barrier_flow.rb:113:14:113:14 | x |