Go: migrate remaining ConditionGuardNode uses to the shared guards library

Agent-Logs-Url: https://github.com/github/codeql/sessions/d505ffcf-a693-4b67-9dc3-54fa92e27896

Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>

Author: Claude

go/ql/lib/semmle/go/controlflow/Guards.qll

@@ -317,3 +317,31 @@ module ValidationWrapper<GuardsLogic::guardChecksSig/3 guardChecks> {
  */
 pragma[inline]
 predicate guardEnsures(Expr e, boolean b, BasicBlock bb) { e.(Guard).controls(bb, b) }
+
+/** Holds if `guard` evaluating to `branch` ensures that `i = j` holds. */
+predicate guardEnsuresEq(Guard guard, boolean branch, DataFlow::Node i, DataFlow::Node j) {
+  guard.isEquality(i.asExpr(), j.asExpr(), branch)
+}
+
+/** Holds if `guard` evaluating to `branch` ensures that `i != j` holds. */
+predicate guardEnsuresNeq(Guard guard, boolean branch, DataFlow::Node i, DataFlow::Node j) {
+  exists(boolean eqval |
+    guard.isEquality(i.asExpr(), j.asExpr(), eqval) and
+    branch = eqval.booleanNot()
+  )
+}
+
+/**
+ * Holds if `guard` evaluating to `branch` ensures that `lesser <= greater + bias`
+ * holds.
+ */
+predicate guardEnsuresLeq(
+  Guard guard, boolean branch, DataFlow::Node lesser, DataFlow::Node greater, int bias
+) {
+  exists(DataFlow::RelationalComparisonNode rel |
+    guard = rel.asExpr() and
+    rel.leq(branch, lesser, greater, bias)
+  )
+  or
+  guardEnsuresEq(guard, branch, lesser, greater) and bias = 0
+}

go/ql/src/InconsistentCode/ConstantLengthComparison.ql

@@ -13,10 +13,11 @@
  */
 
 import go
+private import semmle.go.controlflow.Guards
 
 from
-  ForStmt fs, Variable i, DataFlow::ElementReadNode idx, GVN a,
-  ControlFlow::ConditionGuardNode cond, DataFlow::CallNode lenA
+  ForStmt fs, Variable i, DataFlow::ElementReadNode idx, GVN a, Guard cond, boolean branch,
+  DataFlow::CallNode lenA
 where
   // `i` is incremented in `fs`
   fs.getPost().(IncStmt).getOperand() = i.getAReference() and
@@ -27,11 +28,11 @@ where
   lenA.getArgument(0) = a.getANode() and
   // and is checked against a constant
   exists(DataFlow::Node const | exists(const.getIntValue()) |
-    cond.ensuresNeq(lenA, const) or
-    cond.ensuresLeq(const, lenA, _)
+    guardEnsuresNeq(cond, branch, lenA, const) or
+    guardEnsuresLeq(cond, branch, const, lenA, _)
   ) and
-  cond.dominates(idx.getBasicBlock()) and
+  cond.controls(idx.getBasicBlock(), branch) and
   // and that check happens inside the loop body
-  cond.getCondition().getParent+() = fs
-select cond.getCondition(), "This checks the length against a constant, but it $@.", idx,
+  cond.(Expr).getParent+() = fs
+select cond, "This checks the length against a constant, but it $@.", idx,
   "is indexed using a variable"

go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql

@@ -13,6 +13,7 @@
  */
 
 import go
+private import semmle.go.controlflow.Guards
 
 newtype TIndex =
   VariableIndex(DataFlow::SsaNode v) { v.getAUse() = any(DataFlow::ElementReadNode e).getIndex() } or
@@ -41,21 +42,23 @@ DataFlow::CallNode arrayLen(DataFlow::SsaNode array) {
 }
 
 /**
- * Gets a condition that checks that `index` is less than or equal to `array.length`.
+ * Holds if `guard` evaluating to `branch` checks that `index` is less than or
+ * equal to `array.length`.
  */
-ControlFlow::ConditionGuardNode getLengthLEGuard(Index index, DataFlow::SsaNode array) {
-  result.ensuresLeq(getAUse(index), arrayLen(array), 0)
+predicate lengthLeGuard(Guard guard, boolean branch, Index index, DataFlow::SsaNode array) {
+  guardEnsuresLeq(guard, branch, getAUse(index), arrayLen(array), 0)
   or
   exists(int i, int bias | index = ConstantIndex(i) |
-    result.ensuresLeq(getAUse(ConstantIndex(i + bias)), arrayLen(array), bias)
+    guardEnsuresLeq(guard, branch, getAUse(ConstantIndex(i + bias)), arrayLen(array), bias)
   )
 }
 
 /**
- * Gets a condition that checks that `index` is not equal to `array.length`.
+ * Holds if `guard` evaluating to `branch` checks that `index` is not equal to
+ * `array.length`.
  */
-ControlFlow::ConditionGuardNode getLengthNEGuard(Index index, DataFlow::SsaNode array) {
-  result.ensuresNeq(getAUse(index), arrayLen(array))
+predicate lengthNeGuard(Guard guard, boolean branch, Index index, DataFlow::SsaNode array) {
+  guardEnsuresNeq(guard, branch, getAUse(index), arrayLen(array))
 }
 
 /**
@@ -78,23 +81,24 @@ predicate isRegexpMethodCall(DataFlow::MethodCallNode c) {
 }
 
 from
-  ControlFlow::ConditionGuardNode cond, DataFlow::SsaNode array, Index index,
-  DataFlow::ElementReadNode ea, BasicBlock bb
+  Guard cond, boolean branch, DataFlow::SsaNode array, Index index, DataFlow::ElementReadNode ea,
+  BasicBlock bb
 where
   // there is a comparison `index <= len(array)`
-  cond = getLengthLEGuard(index, array) and
+  lengthLeGuard(cond, branch, index, array) and
   // there is a read from `array[index]`
   elementRead(ea, array, index, bb) and
   // and the read is guarded by the comparison
-  cond.dominates(bb) and
+  cond.controls(bb, branch) and
   // but the read is not guarded by another check that `index != len(array)`
-  not getLengthNEGuard(index, array).dominates(bb) and
+  not exists(Guard ne, boolean neBranch |
+    lengthNeGuard(ne, neBranch, index, array) and ne.controls(bb, neBranch)
+  ) and
   // and it is not additionally guarded by a stronger index check
-  not exists(Index index2, int i, int i2 |
+  not exists(Index index2, int i, int i2, Guard g2, boolean b2 |
     index = ConstantIndex(i) and index2 = ConstantIndex(i2) and i < i2
   |
-    getLengthLEGuard(index2, array).dominates(bb)
+    lengthLeGuard(g2, b2, index2, array) and g2.controls(bb, b2)
   ) and
   not isRegexpMethodCall(array.getInit())
-select cond.getCondition(),
-  "Off-by-one index comparison against length may lead to out-of-bounds $@.", ea, "read"
+select cond, "Off-by-one index comparison against length may lead to out-of-bounds $@.", ea, "read"

go/ql/src/experimental/CWE-807/SensitiveConditionBypass.ql

@@ -14,20 +14,18 @@
 
 import go
 import SensitiveConditionBypass
+private import semmle.go.controlflow.Guards
 
 from
-  ControlFlow::ConditionGuardNode guard, DataFlow::Node sensitiveSink,
-  SensitiveExpr::Classification classification, DataFlow::Node source, DataFlow::Node operand,
-  ComparisonExpr comp
+  DataFlow::Node sensitiveSink, SensitiveExpr::Classification classification, DataFlow::Node source,
+  DataFlow::Node operand, ComparisonExpr comp
 where
   // there should be a flow between source and the operand sink
   Flow::flow(source, operand) and
   // both the operand should belong to the same comparison expression
   operand.asExpr() = comp.getAnOperand() and
-  // get the ConditionGuardNode corresponding to the comparison expr.
-  guard.getCondition() = comp and
   // the sink `sensitiveSink` should be sensitive,
   isSensitive(sensitiveSink, classification) and
-  // the guard should control the sink
-  guard.dominates(sensitiveSink.getBasicBlock())
+  // the comparison should control the sink
+  comp.(Guard).controls(sensitiveSink.getBasicBlock(), _)
 select comp, "This sensitive comparision check can potentially be bypassed."

go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql

@@ -14,6 +14,7 @@
 
 import go
 import semmle.go.security.InsecureFeatureFlag::InsecureFeatureFlag
+private import semmle.go.controlflow.Guards
 
 /**
  * A flag indicating a check for satisfied permissions or test configuration.
@@ -59,10 +60,8 @@ module UntrustedToAllowOriginHeaderConfig implements DataFlow::ConfigSig {
   }
 
   predicate isBarrier(DataFlow::Node node) {
-    exists(ControlFlow::ConditionGuardNode cgn |
-      cgn.ensures(any(AllowedFlag f).getAFlag().getANode(), _)
-    |
-      cgn.dominates(node.getBasicBlock())
+    exists(Guard g | g = any(AllowedFlag f).getAFlag().getANode().asExpr() |
+      g.controls(node.getBasicBlock(), _)
     )
   }
 
@@ -173,7 +172,7 @@ module FromUntrustedConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node sink) { isSinkCgn(sink, _) }
 
-  additional predicate isSinkCgn(DataFlow::Node sink, ControlFlow::ConditionGuardNode cgn) {
+  additional predicate isSinkCgn(DataFlow::Node sink, Guard guard) {
     exists(IfStmt ifs |
       exists(Expr operand |
         operand = ifs.getCond().getAChildExpr*() and
@@ -202,7 +201,7 @@ module FromUntrustedConfig implements DataFlow::ConfigSig {
         )
       )
     |
-      cgn.getCondition() = ifs.getCond()
+      guard = ifs.getCond()
     )
   }
 }
@@ -217,10 +216,10 @@ module FromUntrustedFlow = TaintTracking::Global<FromUntrustedConfig>;
  * Holds if the provided `allowOriginHW` is also destination of a `ActiveThreatModelSource`.
  */
 predicate flowsToGuardedByCheckOnUntrusted(DataFlow::ExprNode allowOriginHW) {
-  exists(DataFlow::Node sink, ControlFlow::ConditionGuardNode cgn |
-    FromUntrustedFlow::flowTo(sink) and FromUntrustedConfig::isSinkCgn(sink, cgn)
+  exists(DataFlow::Node sink, Guard guard |
+    FromUntrustedFlow::flowTo(sink) and FromUntrustedConfig::isSinkCgn(sink, guard)
   |
-    cgn.dominates(allowOriginHW.getBasicBlock())
+    guard.controls(allowOriginHW.getBasicBlock(), _)
   )
 }
 
@@ -233,9 +232,7 @@ where
     allowOriginIsNull(allowOriginHW, message)
   ) and
   not flowsToGuardedByCheckOnUntrusted(allowOriginHW) and
-  not exists(ControlFlow::ConditionGuardNode cgn |
-    cgn.ensures(any(AllowedFlag f).getAFlag().getANode(), _)
-  |
-    cgn.dominates(allowOriginHW.getBasicBlock())
+  not exists(Guard g | g = any(AllowedFlag f).getAFlag().getANode().asExpr() |
+    g.controls(allowOriginHW.getBasicBlock(), _)
   )
 select allowOriginHW, message

go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll

@@ -1,4 +1,5 @@
 import go
+private import semmle.go.controlflow.Guards
 
 Expr getAUse(SsaDefinition def) {
   result = def.getVariable().getAUse().(IR::EvalInstruction).getExpr()
@@ -46,12 +47,12 @@ float getAnUpperBound(Expr expr) {
         if
           //if a condition expression exists before and one of the operand happens to be the identifier, we use this condition expression to narrow down the range.
           exists(
-            ControlFlow::ConditionGuardNode n, DataFlow::Node lesser, DataFlow::Node greater,
+            Guard n, boolean branch, DataFlow::Node lesser, DataFlow::Node greater,
             ReachableBasicBlock bb
           |
-            n.ensuresLeq(lesser, greater, _) and
+            guardEnsuresLeq(n, branch, lesser, greater, _) and
             IR::evalExprInstruction(lesser.asExpr()) = v.getAUse() and
-            n.dominates(bb) and
+            n.controls(bb, branch) and
             bb.getANode() = IR::evalExprInstruction(identifier) and
             not exists(Expr e |
               e = v.getAUse().(IR::EvalInstruction).getExpr() and
@@ -60,12 +61,12 @@ float getAnUpperBound(Expr expr) {
           )
         then
           exists(
-            ControlFlow::ConditionGuardNode n, ReachableBasicBlock bb, DataFlow::Node lesser,
+            Guard n, boolean branch, ReachableBasicBlock bb, DataFlow::Node lesser,
             DataFlow::Node greater, int bias
           |
-            n.dominates(bb) and
+            n.controls(bb, branch) and
             bb.getANode() = IR::evalExprInstruction(identifier) and
-            n.ensuresLeq(lesser, greater, bias) and
+            guardEnsuresLeq(n, branch, lesser, greater, bias) and
             v.getAUse() = IR::evalExprInstruction(lesser.asExpr()) and
             not exists(Expr e |
               e = v.getAUse().(IR::EvalInstruction).getExpr() and
@@ -202,12 +203,12 @@ float getALowerBound(Expr expr) {
         //if exists a condition expression before this identifier
         if
           exists(
-            ControlFlow::ConditionGuardNode n, DataFlow::Node greater, DataFlow::Node lesser,
+            Guard n, boolean branch, DataFlow::Node greater, DataFlow::Node lesser,
             ReachableBasicBlock bb
           |
-            n.ensuresLeq(lesser, greater, _) and
+            guardEnsuresLeq(n, branch, lesser, greater, _) and
             IR::evalExprInstruction(greater.asExpr()) = v.getAUse() and
-            n.dominates(bb) and
+            n.controls(bb, branch) and
             bb.getANode() = IR::evalExprInstruction(identifier) and
             not exists(Expr e |
               e = v.getAUse().(IR::EvalInstruction).getExpr() and
@@ -216,12 +217,12 @@ float getALowerBound(Expr expr) {
           )
         then
           exists(
-            ControlFlow::ConditionGuardNode n, ReachableBasicBlock bb, DataFlow::Node lesser,
+            Guard n, boolean branch, ReachableBasicBlock bb, DataFlow::Node lesser,
             DataFlow::Node greater, int bias, float lbs
           |
-            n.dominates(bb) and
+            n.controls(bb, branch) and
             bb.getANode() = IR::evalExprInstruction(identifier) and
-            n.ensuresLeq(lesser, greater, bias) and
+            guardEnsuresLeq(n, branch, lesser, greater, bias) and
             v.getAUse() = IR::evalExprInstruction(greater.asExpr()) and
             not exists(Expr e |
               e = v.getAUse().(IR::EvalInstruction).getExpr() and