InsecureFeatureFlag: make getAFlag a member of FlagKind

Author: smowton

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

@@ -52,7 +52,7 @@ class InsecureCertificateFlag extends FlagKind {
  * Gets a control-flow node that represents a (likely) flag controlling an insecure certificate setup.
  */
 ControlFlow::ConditionGuardNode getAnInsecureCertificateCheck() {
-  result.ensures(getAFlag(any(InsecureCertificateFlag f)).getANode(), _)
+  result.ensures(any(InsecureCertificateFlag f).getAFlag().getANode(), _)
 }
 
 /**

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

@@ -261,7 +261,7 @@ class LegacyTlsVersionFlag extends FlagKind {
  * Gets a control-flow node that represents a (likely) flag controlling TLS version selection.
  */
 ControlFlow::ConditionGuardNode getALegacyTlsVersionCheck() {
-  result.ensures(getAFlag(any(LegacyTlsVersionFlag f)).getANode(), _)
+  result.ensures(any(LegacyTlsVersionFlag f).getAFlag().getANode(), _)
 }
 
 /**

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

@@ -16,6 +16,54 @@ module InsecureFeatureFlag {
      * Returns a flag name of this type.
      */
     abstract string getAFlagName();
+
+    /** Gets a global value number representing a (likely) security flag. */
+    GVN getAFlag() {
+      // a call like `cfg.disableVerification()`
+      exists(DataFlow::CallNode c | c.getTarget().getName() = getAFlagName() |
+        result = globalValueNumber(c)
+      )
+      or
+      // a variable or field like `insecure`
+      exists(ValueEntity flag | flag.getName() = getAFlagName() |
+        result = globalValueNumber(flag.getARead())
+      )
+      or
+      // a string constant such as `"insecure"` or `"skipVerification"`
+      exists(DataFlow::Node const | const.getStringValue() = getAFlagName() |
+        result = globalValueNumber(const)
+      )
+      or
+      // track feature flags through various operations
+      exists(DataFlow::Node flag | flag = getAFlag().getANode() |
+        // tuple destructurings
+        result = globalValueNumber(DataFlow::extractTupleElement(flag, _))
+        or
+        // type casts
+        exists(DataFlow::TypeCastNode tc |
+          tc.getOperand() = flag and
+          result = globalValueNumber(tc)
+        )
+        or
+        // pointer dereferences
+        exists(DataFlow::PointerDereferenceNode deref |
+          deref.getOperand() = flag and
+          result = globalValueNumber(deref)
+        )
+        or
+        // calls like `os.Getenv("DISABLE_TLS_VERIFICATION")`
+        exists(DataFlow::CallNode call |
+          call.getAnArgument() = flag and
+          result = globalValueNumber(call)
+        )
+        or
+        // comparisons like `insecure == true`
+        exists(DataFlow::EqualityTestNode eq |
+          eq.getAnOperand() = flag and
+          result = globalValueNumber(eq)
+        )
+      )
+    }
   }
 
   /**
@@ -28,54 +76,6 @@ module InsecureFeatureFlag {
     override string getAFlagName() { result.regexpMatch("(?i).*(secure|(en|dis)able).*") }
   }
 
-  /** Gets a global value number representing a (likely) security flag. */
-  GVN getAFlag(FlagKind flagKind) {
-    // a call like `cfg.disableVerification()`
-    exists(DataFlow::CallNode c | c.getTarget().getName() = flagKind.getAFlagName() |
-      result = globalValueNumber(c)
-    )
-    or
-    // a variable or field like `insecure`
-    exists(ValueEntity flag | flag.getName() = flagKind.getAFlagName() |
-      result = globalValueNumber(flag.getARead())
-    )
-    or
-    // a string constant such as `"insecure"` or `"skipVerification"`
-    exists(DataFlow::Node const | const.getStringValue() = flagKind.getAFlagName() |
-      result = globalValueNumber(const)
-    )
-    or
-    // track feature flags through various operations
-    exists(DataFlow::Node flag | flag = getAFlag(flagKind).getANode() |
-      // tuple destructurings
-      result = globalValueNumber(DataFlow::extractTupleElement(flag, _))
-      or
-      // type casts
-      exists(DataFlow::TypeCastNode tc |
-        tc.getOperand() = flag and
-        result = globalValueNumber(tc)
-      )
-      or
-      // pointer dereferences
-      exists(DataFlow::PointerDereferenceNode deref |
-        deref.getOperand() = flag and
-        result = globalValueNumber(deref)
-      )
-      or
-      // calls like `os.Getenv("DISABLE_TLS_VERIFICATION")`
-      exists(DataFlow::CallNode call |
-        call.getAnArgument() = flag and
-        result = globalValueNumber(call)
-      )
-      or
-      // comparisons like `insecure == true`
-      exists(DataFlow::EqualityTestNode eq |
-        eq.getAnOperand() = flag and
-        result = globalValueNumber(eq)
-      )
-    )
-  }
-
   /**
    * Holds for string literals or named values matching `flagKind` and their fields.
    */
@@ -113,6 +113,6 @@ module InsecureFeatureFlag {
    * Gets a control-flow node that represents a (likely) security feature-flag check
    */
   ControlFlow::ConditionGuardNode getASecurityFeatureFlagCheck() {
-    result.ensures(getAFlag(any(SecurityFeatureFlag f)).getANode(), _)
+    result.ensures(any(SecurityFeatureFlag f).getAFlag().getANode(), _)
   }
 }