Fix logic for feature flag guarding default case clauses

Author: owen-mc

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

@@ -115,9 +115,23 @@ module InsecureFeatureFlag {
   }
 
   /**
-   * Gets a guard that represents a (likely) security feature-flag check.
+   * Holds if `block` is controlled by a flag of kind `flagKind`.
+   *
+   * For a switch case expression, only the matching branch is controlled by that flag. Other
+   * branches, including the default case, are reached when the flag does not match.
    */
-  Guard getASecurityFeatureFlagCheck() {
-    result = any(SecurityFeatureFlag f).getAFlag().getANode().asExpr()
+  predicate flagControls(FlagKind flagKind, BasicBlock block) {
+    exists(GVN flag, Guard guard, boolean branch |
+      flag = flagKind.getAFlag() and
+      guard = flag.getANode().asExpr() and
+      guard.controls(block, branch) and
+      (
+        branch = true
+        or
+        not exists(Expr caseExpr |
+          caseExpr = flag.getANode().asExpr() and caseExpr.getParent() instanceof CaseClause
+        )
+      )
+    )
   }
 }

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

@@ -50,13 +50,6 @@ class InsecureCertificateFlag extends FlagKind {
   }
 }
 
-/**
- * Gets a guard that represents a (likely) flag controlling an insecure certificate setup.
- */
-Guard getAnInsecureCertificateCheck() {
-  result = any(InsecureCertificateFlag f).getAFlag().getANode().asExpr()
-}
-
 /**
  * Returns flag kinds relevant to this query: a generic security feature flag, or one
  * specifically controlling insecure certificate configuration.
@@ -81,8 +74,7 @@ where
   f.hasQualifiedName("crypto/tls", "Config", "InsecureSkipVerify") and
   rhs.getBoolValue() = true and
   // exclude writes guarded by a feature flag
-  not [getASecurityFeatureFlagCheck(), getAnInsecureCertificateCheck()]
-      .controls(w.getBasicBlock(), _) and
+  not flagControls(securityOrTlsVersionFlag(), 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

@@ -247,13 +247,6 @@ class LegacyTlsVersionFlag extends FlagKind {
   override string getAFlagName() { result.regexpMatch("(?i).*(old|intermediate|legacy).*") }
 }
 
-/**
- * Gets a guard that represents a (likely) flag controlling TLS version selection.
- */
-Guard getALegacyTlsVersionCheck() {
-  result = any(LegacyTlsVersionFlag f).getAFlag().getANode().asExpr()
-}
-
 /**
  * Returns flag kinds relevant to this query: a generic security feature flag, or one
  * specifically controlling TLS version selection.
@@ -276,8 +269,7 @@ where
     isInsecureTlsCipherFlow(source.asPathNode2(), sink.asPathNode2(), message)
   ) and
   // Exclude sources or sinks guarded by a feature or legacy flag
-  not [getASecurityFeatureFlagCheck(), getALegacyTlsVersionCheck()]
-      .controls([source, sink].getNode().getBasicBlock(), _) and
+  not flagControls(securityOrTlsVersionFlag(), [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