Go: migrate simple 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/dataflow/internal/DataFlowPrivate.qll

@@ -2,6 +2,7 @@ overlay[local?]
 module;
 
 private import go
+private import semmle.go.controlflow.Guards
 private import DataFlowUtil
 private import DataFlowImplCommon
 private import ContainerFlow
@@ -390,19 +391,21 @@ private class ConstantBooleanArgumentNode extends ArgumentNode, ExprNode {
 }
 
 /**
- * Returns a guard that will certainly not hold in calling context `call`.
+ * Holds if `guard` evaluating to `branch` will certainly not happen in calling
+ * context `call`.
  *
  * In particular it does not hold because it checks that `param` has value `b`, but
  * in context `call` it is known to have value `!b`. Note this is `noinline`d in order
  * to avoid a bad join order in `isUnreachableInCall`.
  */
 pragma[noinline]
-private ControlFlow::ConditionGuardNode getAFalsifiedGuard(DataFlowCall call) {
+private predicate falsifiedGuard(DataFlowCall call, Guard guard, boolean branch) {
   exists(SsaParameterNode param, ConstantBooleanArgumentNode arg |
     // get constant bool argument and parameter for this call
     viableParamArg(call, pragma[only_bind_into](param), pragma[only_bind_into](arg)) and
     // which is used in a guard controlling `n` with the opposite value of `arg`
-    result.ensures(param.getAUse(), arg.getBooleanValue().booleanNot())
+    guard = param.getAUse().asExpr() and
+    branch = arg.getBooleanValue().booleanNot()
   )
 }
 
@@ -416,7 +419,10 @@ class NodeRegion instanceof BasicBlock {
  * Holds if the nodes in `nr` are unreachable when the call context is `call`.
  */
 predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) {
-  getAFalsifiedGuard(call).dominates(nr)
+  exists(Guard guard, boolean branch |
+    falsifiedGuard(call, guard, branch) and
+    guard.controls(nr, branch)
+  )
 }
 
 /**

go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll

@@ -5,6 +5,7 @@ overlay[local?]
 module;
 
 private import go
+private import semmle.go.controlflow.Guards
 private import semmle.go.dataflow.FunctionInputsAndOutputs
 private import semmle.go.dataflow.ExternalFlow
 private import DataFlowPrivate
@@ -388,51 +389,47 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
 module ParameterizedBarrierGuard<ParamSig P, WithParam<P>::guardChecksSig/4 guardChecks> {
   /** Gets a node that is safely guarded by the given guard check. */
   Node getABarrierNode(P param) {
-    exists(ControlFlow::ConditionGuardNode guard, SsaWithFields var |
+    exists(Guard guard, boolean branch, SsaWithFields var |
       result = pragma[only_bind_out](var).getAUse()
     |
-      guards(_, guard, _, var, param) and
-      pragma[only_bind_out](guard).dominates(result.getBasicBlock())
+      guards(_, guard, branch, _, var, param) and
+      pragma[only_bind_out](guard).controls(result.getBasicBlock(), branch)
     )
   }
 
   /**
    * Gets a node that is safely guarded by the given guard check.
    */
   Node getABarrierNodeForGuard(Node guardCheck, P param) {
-    exists(ControlFlow::ConditionGuardNode guard, SsaWithFields var | result = var.getAUse() |
-      guards(guardCheck, guard, _, var, param) and
-      guard.dominates(result.getBasicBlock())
+    exists(Guard guard, boolean branch, SsaWithFields var | result = var.getAUse() |
+      guards(guardCheck, guard, branch, _, var, param) and
+      guard.controls(result.getBasicBlock(), branch)
     )
   }
 
   /**
-   * Holds if `guard` marks a point in the control-flow graph where `g`
-   * is known to validate `nd`, which is represented by `ap`.
+   * Holds if `guard` evaluating to `branch` marks a point in the control-flow
+   * graph where `g` is known to validate `nd`, which is represented by `ap`.
    *
    * This predicate exists to enforce a good join order in `getAGuardedNode`.
    */
   pragma[noinline]
-  private predicate guards(
-    Node g, ControlFlow::ConditionGuardNode guard, Node nd, SsaWithFields ap, P param
-  ) {
-    guards(g, guard, nd, param) and nd = ap.getAUse()
+  private predicate guards(Node g, Guard guard, boolean branch, Node nd, SsaWithFields ap, P param) {
+    guards(g, guard, branch, nd, param) and nd = ap.getAUse()
   }
 
   /**
-   * Holds if `guard` marks a point in the control-flow graph where `g`
-   * is known to validate `nd`.
+   * Holds if `guard` evaluating to `branch` marks a point in the control-flow
+   * graph where `g` is known to validate `nd`.
    */
-  private predicate guards(Node g, ControlFlow::ConditionGuardNode guard, Node nd, P param) {
-    exists(boolean branch |
-      guardChecks(g, nd.asExpr(), branch, param) and
-      guard.ensures(g, branch)
-    )
+  private predicate guards(Node g, Guard guard, boolean branch, Node nd, P param) {
+    guardChecks(g, nd.asExpr(), branch, param) and
+    guard = g.asExpr()
     or
-    exists(DataFlow::Property p, Node resNode, Node check, boolean outcome |
+    exists(DataFlow::Property p, Node resNode, Node check |
       guardingCall(g, _, _, _, p, _, nd, resNode, param) and
-      p.checkOn(check, outcome, resNode) and
-      guard.ensures(pragma[only_bind_into](check), outcome)
+      p.checkOn(check, branch, resNode) and
+      guard = pragma[only_bind_into](check).asExpr()
     )
   }
 
@@ -487,9 +484,9 @@ module ParameterizedBarrierGuard<ParamSig P, WithParam<P>::guardChecksSig/4 guar
       localFlow(inp.getExitNode(fd), pragma[only_bind_out](arg)) and
       (
         // Case: a function like "if someBarrierGuard(arg) { return true } else { return false }"
-        exists(ControlFlow::ConditionGuardNode guard |
-          guards(g, pragma[only_bind_out](guard), arg, param) and
-          guard.dominates(pragma[only_bind_out](ret).getBasicBlock())
+        exists(Guard guard, boolean branch |
+          guards(g, pragma[only_bind_out](guard), branch, arg, param) and
+          guard.controls(pragma[only_bind_out](ret).getBasicBlock(), branch)
         |
           onlyPossibleReturnSatisfyingProperty(fd, outp, ret, p)
         )

go/ql/lib/semmle/go/security/InsecureFeatureFlag.qll

@@ -3,6 +3,7 @@
  */
 
 import go
+private import semmle.go.controlflow.Guards
 
 /**
  * Provides classes and predicates relating to flags that may indicate security expectations.
@@ -114,9 +115,9 @@ module InsecureFeatureFlag {
   }
 
   /**
-   * Gets a control-flow node that represents a (likely) security feature-flag check
+   * Gets a guard that represents a (likely) security feature-flag check.
    */
-  ControlFlow::ConditionGuardNode getASecurityFeatureFlagCheck() {
-    result.ensures(any(SecurityFeatureFlag f).getAFlag().getANode(), _)
+  Guard getASecurityFeatureFlagCheck() {
+    result = any(SecurityFeatureFlag f).getAFlag().getANode().asExpr()
   }
 }

go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql

@@ -13,6 +13,7 @@
  */
 
 import go
+private import semmle.go.controlflow.Guards
 
 /**
  * Holds if `pattern` is a regular expression pattern for URLs with a host matched by `hostPart`,
@@ -70,12 +71,12 @@ predicate regexpGuardsHandler(RegexpPattern regexp, Http::RequestHandler handler
 
 /** Holds if `regexp` guards an HTTP error write. */
 predicate regexpGuardsError(RegexpPattern regexp) {
-  exists(ControlFlow::ConditionGuardNode cond, RegexpMatchFunction match, DataFlow::CallNode call |
+  exists(Guard cond, RegexpMatchFunction match, DataFlow::CallNode call |
     call.getTarget() = match and
     match.getRegexp(call) = regexp
   |
-    cond.ensures(match.getResult().getNode(call).getASuccessor*(), true) and
-    cond.dominates(any(ReachableBasicBlock b | writesHttpError(b)))
+    cond = match.getResult().getNode(call).getASuccessor*().asExpr() and
+    cond.controls(any(ReachableBasicBlock b | writesHttpError(b)), true)
   )
 }
 

go/ql/src/Security/CWE-209/StackTraceExposure.ql

@@ -15,6 +15,7 @@
 
 import go
 import semmle.go.security.InsecureFeatureFlag::InsecureFeatureFlag
+private import semmle.go.controlflow.Guards
 
 /**
  * A flag indicating the program is in debug or development mode, or that stack
@@ -56,10 +57,8 @@ module StackTraceExposureConfig implements DataFlow::ConfigSig {
     // Sanitize everything controlled by an is-debug-mode check.
     // Imprecision: I don't try to guess which arm of a branch is intended
     // to mean debug mode, and which is production mode.
-    exists(ControlFlow::ConditionGuardNode cgn |
-      cgn.ensures(any(DebugModeFlag f).getAFlag().getANode(), _)
-    |
-      cgn.dominates(node.getBasicBlock())
+    exists(Guard g | g = any(DebugModeFlag f).getAFlag().getANode().asExpr() |
+      g.controls(node.getBasicBlock(), _)
     )
   }
 

go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql

@@ -24,6 +24,7 @@
 
 import go
 import semmle.go.security.InsecureFeatureFlag::InsecureFeatureFlag
+private import semmle.go.controlflow.Guards
 
 /**
  * Holds if `part` becomes a part of `whole`, either by (local) data flow or by being incorporated
@@ -50,10 +51,10 @@ class InsecureCertificateFlag extends FlagKind {
 }
 
 /**
- * Gets a control-flow node that represents a (likely) flag controlling an insecure certificate setup.
+ * Gets a guard that represents a (likely) flag controlling an insecure certificate setup.
  */
-ControlFlow::ConditionGuardNode getAnInsecureCertificateCheck() {
-  result.ensures(any(InsecureCertificateFlag f).getAFlag().getANode(), _)
+Guard getAnInsecureCertificateCheck() {
+  result = any(InsecureCertificateFlag f).getAFlag().getANode().asExpr()
 }
 
 /**
@@ -80,7 +81,8 @@ where
   f.hasQualifiedName("crypto/tls", "Config", "InsecureSkipVerify") and
   rhs.getBoolValue() = true and
   // exclude writes guarded by a feature flag
-  not [getASecurityFeatureFlagCheck(), getAnInsecureCertificateCheck()].dominatesNode(w) and
+  not [getASecurityFeatureFlagCheck(), getAnInsecureCertificateCheck()]
+      .controls(w.getBasicBlock(), _) and
   // exclude results in functions whose name documents the insecurity
   not exists(FuncDef fn | fn = w.getRoot() |
     isSecurityOrCertificateConfigFlag(fn.getEnclosingFunction*().getName())

go/ql/src/Security/CWE-327/InsecureTLS.ql

@@ -13,6 +13,7 @@
 
 import go
 import semmle.go.security.InsecureFeatureFlag::InsecureFeatureFlag
+private import semmle.go.controlflow.Guards
 
 /**
  * Holds if it is insecure to assign TLS version `val` named `name` to `tls.Config` field `fieldName`.
@@ -247,10 +248,10 @@ class LegacyTlsVersionFlag extends FlagKind {
 }
 
 /**
- * Gets a control-flow node that represents a (likely) flag controlling TLS version selection.
+ * Gets a guard that represents a (likely) flag controlling TLS version selection.
  */
-ControlFlow::ConditionGuardNode getALegacyTlsVersionCheck() {
-  result.ensures(any(LegacyTlsVersionFlag f).getAFlag().getANode(), _)
+Guard getALegacyTlsVersionCheck() {
+  result = any(LegacyTlsVersionFlag f).getAFlag().getANode().asExpr()
 }
 
 /**
@@ -276,7 +277,7 @@ where
   ) and
   // Exclude sources or sinks guarded by a feature or legacy flag
   not [getASecurityFeatureFlagCheck(), getALegacyTlsVersionCheck()]
-      .dominatesNode([source, sink].getNode().asInstruction()) and
+      .controls([source, sink].getNode().getBasicBlock(), _) and
   // Exclude sources or sinks that occur lexically within a block related to a feature or legacy flag
   not astNodeIsFlag([source, sink].getNode().asExpr().getParent*(), securityOrTlsVersionFlag()) and
   // Exclude results in functions whose name documents insecurity