QL: Add codeql-go sources (894102d)

repo-tests/codeql-go.txt

@@ -1 +1 @@
-4cae4b23fc1b2b1760e259b660996e9bb5573279
+894102defd0777931a0e261ad66e631e63ec0ad8

repo-tests/codeql-go/ql/lib/semmle/go/Decls.qll

@@ -137,7 +137,8 @@ class FuncDef extends @funcdef, StmtParent, ExprParent {
    */
   DataFlow::CallNode getACall() { result.getACallee() = this }
 
-  predicate isVariadic() { getType().isVariadic() }
+  /** Holds if this function is variadic. */
+  predicate isVariadic() { this.getType().isVariadic() }
 
   override string getAPrimaryQlClass() { result = "FuncDef" }
 }

repo-tests/codeql-go/ql/lib/semmle/go/Scopes.qll

@@ -377,10 +377,11 @@ class Function extends ValueEntity, @functionobject {
   /** Gets the declaration of this function, if any. */
   FuncDecl getFuncDecl() { none() }
 
+  /** Holds if this function is variadic. */
   predicate isVariadic() {
     this.(BuiltinFunction).getName() = ["append", "make", "print", "println"]
     or
-    this.(DeclaredFunction).getFuncDecl().isVariadic()
+    this.(DeclaredFunction).getType().(SignatureType).isVariadic()
   }
 
   /** Holds if this function has no observable side effects. */

repo-tests/codeql-go/ql/lib/semmle/go/dataflow/barrierguardutil/RegexpCheck.qll

@@ -8,18 +8,26 @@ import go
  * A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
  *
  * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
+ *
+ * Use this if you want to define a derived `DataFlow::BarrierGuard` without
+ * make the type recursive. Otherwise use `RegexpCheck`.
  */
-class RegexpCheck extends DataFlow::BarrierGuard {
-  RegexpMatchFunction matchfn;
-  DataFlow::CallNode call;
-
-  RegexpCheck() {
+predicate regexpFunctionChecksExpr(DataFlow::Node resultNode, Expr checked, boolean branch) {
+  exists(RegexpMatchFunction matchfn, DataFlow::CallNode call |
     matchfn.getACall() = call and
-    this = matchfn.getResult().getNode(call).getASuccessor*()
-  }
-
-  override predicate checks(Expr e, boolean branch) {
-    e = matchfn.getValue().getNode(call).asExpr() and
+    resultNode = matchfn.getResult().getNode(call).getASuccessor*() and
+    checked = matchfn.getValue().getNode(call).asExpr() and
     (branch = false or branch = true)
-  }
+  )
+}
+
+/**
+ * A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
+ *
+ * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
+ */
+class RegexpCheck extends DataFlow::BarrierGuard {
+  RegexpCheck() { regexpFunctionChecksExpr(this, _, _) }
+
+  override predicate checks(Expr e, boolean branch) { regexpFunctionChecksExpr(this, e, branch) }
 }

repo-tests/codeql-go/ql/lib/semmle/go/frameworks/Glog.qll

@@ -11,10 +11,17 @@ import go
  */
 module Glog {
   private class GlogCall extends LoggerCall::Range, DataFlow::CallNode {
+    int firstPrintedArg;
+
     GlogCall() {
-      exists(string pkg, Function f, string fn |
+      exists(string pkg, Function f, string fn, string level |
         pkg = package(["github.com/golang/glog", "gopkg.in/glog", "k8s.io/klog"], "") and
-        fn.regexpMatch("(Error|Exit|Fatal|Info|Warning)(|f|ln)") and
+        level = ["Error", "Exit", "Fatal", "Info", "Warning"] and
+        (
+          fn = level + ["", "f", "ln"] and firstPrintedArg = 0
+          or
+          fn = level + "Depth" and firstPrintedArg = 1
+        ) and
         this = f.getACall()
       |
         f.hasQualifiedName(pkg, fn)
@@ -23,6 +30,8 @@ module Glog {
       )
     }
 
-    override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
+    override DataFlow::Node getAMessageComponent() {
+      result = this.getArgument(any(int i | i >= firstPrintedArg))
+    }
   }
 }

repo-tests/codeql-go/ql/lib/semmle/go/frameworks/Logrus.qll

@@ -11,7 +11,10 @@ module Logrus {
 
   bindingset[result]
   private string getALogResultName() {
-    result.matches(["Debug%", "Error%", "Fatal%", "Info%", "Panic%", "Print%", "Trace%", "Warn%"])
+    result
+        .matches([
+            "Debug%", "Error%", "Fatal%", "Info%", "Log%", "Panic%", "Print%", "Trace%", "Warn%"
+          ])
   }
 
   bindingset[result]
@@ -23,7 +26,7 @@ module Logrus {
     LogCall() {
       exists(string name | name = getALogResultName() or name = getAnEntryUpdatingMethodName() |
         this.getTarget().hasQualifiedName(packagePath(), name) or
-        this.getTarget().(Method).hasQualifiedName(packagePath(), "Entry", name)
+        this.getTarget().(Method).hasQualifiedName(packagePath(), ["Entry", "Logger"], name)
       )
     }
 

repo-tests/codeql-go/ql/lib/semmle/go/frameworks/SQL.qll

@@ -88,10 +88,11 @@ module SQL {
             // first argument to `squirrel.Expr`
             fn.hasQualifiedName(sq, "Expr")
             or
-            // first argument to the `Prefix` or `Suffix` method of one of the `*Builder` classes
+            // first argument to the `Prefix`, `Suffix` or `Where` method of one of the `*Builder` classes
             exists(string builder | builder.matches("%Builder") |
               fn.(Method).hasQualifiedName(sq, builder, "Prefix") or
-              fn.(Method).hasQualifiedName(sq, builder, "Suffix")
+              fn.(Method).hasQualifiedName(sq, builder, "Suffix") or
+              fn.(Method).hasQualifiedName(sq, builder, "Where")
             )
           ) and
           this = fn.getACall().getArgument(0) and

repo-tests/codeql-go/ql/lib/semmle/go/frameworks/stdlib/Log.qll

@@ -8,13 +8,7 @@ import go
 module Log {
   private class LogCall extends LoggerCall::Range, DataFlow::CallNode {
     LogCall() {
-      exists(string fn |
-        fn.matches("Fatal%")
-        or
-        fn.matches("Panic%")
-        or
-        fn.matches("Print%")
-      |
+      exists(string fn | fn.matches(["Fatal%", "Panic%", "Print%"]) |
         this.getTarget().hasQualifiedName("log", fn)
         or
         this.getTarget().(Method).hasQualifiedName("log", "Logger", fn)

repo-tests/codeql-go/ql/lib/semmle/go/security/IncorrectIntegerConversionLib.qll

@@ -124,7 +124,7 @@ class ConversionWithoutBoundsCheckConfig extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) { isSink(sink, sinkBitSize) }
+  override predicate isSink(DataFlow::Node sink) { this.isSink(sink, sinkBitSize) }
 
   override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
     // To catch flows that only happen on 32-bit architectures we
@@ -136,7 +136,7 @@ class ConversionWithoutBoundsCheckConfig extends TaintTracking::Configuration {
 
   override predicate isSanitizerOut(DataFlow::Node node) {
     exists(int bitSize | isIncorrectIntegerConversion(sourceBitSize, bitSize) |
-      isSink(node, bitSize)
+      this.isSink(node, bitSize)
     )
   }
 }

repo-tests/codeql-go/ql/lib/semmle/go/security/LogInjection.qll

@@ -0,0 +1,33 @@
+/**
+ * Provides a taint tracking configuration for reasoning about log injection vulnerabilities.
+ *
+ * Note: for performance reasons, only import this file if `LogInjection::Configuration` is needed,
+ * otherwise `LogInjectionCustomizations` should be imported instead.
+ */
+
+import go
+
+/**
+ * Provides a taint-tracking configuration for reasoning about
+ * log injection vulnerabilities.
+ */
+module LogInjection {
+  import LogInjectionCustomizations::LogInjection
+
+  /**
+   * A taint-tracking configuration for reasoning about log injection vulnerabilities.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "LogInjection" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer }
+
+    override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+      guard instanceof SanitizerGuard
+    }
+  }
+}

repo-tests/codeql-go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll

@@ -0,0 +1,42 @@
+/**
+ * Provides default sources, sinks, and sanitizers for reasoning about
+ * log injection vulnerabilities, as well as extension points for adding your own.
+ */
+
+import go
+
+/**
+ * Provides extension points for customizing the data-flow tracking configuration for reasoning
+ * about log injection.
+ */
+module LogInjection {
+  /**
+   * A data flow source for log injection vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for log injection vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for log injection vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A sanitizer guard for log injection vulnerabilities.
+   */
+  abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+
+  /** A source of untrusted data, considered as a taint source for log injection. */
+  class UntrustedFlowAsSource extends Source {
+    UntrustedFlowAsSource() { this instanceof UntrustedFlowSource }
+  }
+
+  /** An argument to a logging mechanism. */
+  class LoggerSink extends Sink {
+    LoggerSink() { this = any(LoggerCall log).getAMessageComponent() }
+  }
+}

repo-tests/codeql-go/ql/lib/semmle/go/security/TaintedPath.qll

@@ -27,7 +27,7 @@ module TaintedPath {
     }
 
     override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-      guard instanceof SanitizerGuard
+      guard instanceof SanitizerGuardAsBarrierGuard
     }
   }
 }

repo-tests/codeql-go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll

@@ -28,8 +28,30 @@ module TaintedPath {
 
   /**
    * A sanitizer guard for path-traversal vulnerabilities.
+   *
+   * Note this class should be extended to define more taint-path sanitizer guards, but isn't itself a
+   * `DataFlow::BarrierGuard` so that other queries can use this to define `BarrierGuard`s without
+   * introducing recursion. The class `SanitizerGuardAsBarrierGuard` plugs all instances of this class
+   * into the `DataFlow::BarrierGuard` type hierarchy.
+   */
+  abstract class SanitizerGuard extends DataFlow::Node {
+    abstract predicate checks(Expr e, boolean branch);
+  }
+
+  /**
+   * A sanitizer guard for path-traversal vulnerabilities, as a `DataFlow::BarrierGuard`.
+   *
+   * Use this class if you want all `TaintedPath::SanitizerGuard`s as a `DataFlow::BarrierGuard`,
+   * e.g. to use directly in a `DataFlow::Configuration::isSanitizerGuard` method. If you want to
+   * provide a new instance of a tainted path sanitizer, extend `TaintedPath::SanitizerGuard` instead.
    */
-  abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+  class SanitizerGuardAsBarrierGuard extends DataFlow::BarrierGuard {
+    SanitizerGuard guardImpl;
+
+    SanitizerGuardAsBarrierGuard() { this = guardImpl }
+
+    override predicate checks(Expr e, boolean branch) { guardImpl.checks(e, branch) }
+  }
 
   /** A source of untrusted data, considered as a taint source for path traversal. */
   class UntrustedFlowAsSource extends Source {
@@ -134,5 +156,9 @@ module TaintedPath {
    *
    * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
    */
-  class RegexpCheckAsSanitizerGuard extends RegexpCheck, SanitizerGuard { }
+  class RegexpCheckAsSanitizerGuard extends SanitizerGuard {
+    RegexpCheckAsSanitizerGuard() { regexpFunctionChecksExpr(this, _, _) }
+
+    override predicate checks(Expr e, boolean branch) { regexpFunctionChecksExpr(this, e, branch) }
+  }
 }

repo-tests/codeql-go/ql/lib/semmle/go/security/ZipSlipCustomizations.qll

@@ -86,6 +86,27 @@ module ZipSlip {
     TaintedPathSanitizerAsSanitizer() { this instanceof TaintedPath::Sanitizer }
   }
 
+  pragma[noinline]
+  private predicate taintedPathGuardChecks(
+    TaintedPath::SanitizerGuard guard, DataFlow::Node checked, boolean branch
+  ) {
+    guard.checks(checked.asExpr(), branch)
+  }
+
+  pragma[noinline]
+  private predicate taintFlowsToCheckedNode(DataFlow::Node source, DataFlow::Node checked) {
+    taintedPathGuardChecks(_, checked, _) and
+    (
+      // Manual magic that is equivalent to `localTaint(source, checked)`
+      source = checked
+      or
+      exists(DataFlow::Node succ |
+        taintFlowsToCheckedNode(succ, checked) and
+        TaintTracking::localTaintStep(source, succ)
+      )
+    )
+  }
+
   /**
    * A sanitizer guard for zip-slip vulnerabilities which backtracks to sanitize expressions
    * that locally flow into a guarded expression. For example, an ordinary sanitizer guard
@@ -106,8 +127,8 @@ module ZipSlip {
 
     override predicate checks(Expr e, boolean branch) {
       exists(DataFlow::Node source, DataFlow::Node checked |
-        taintedPathGuard.checks(checked.asExpr(), branch) and
-        TaintTracking::localTaint(source, checked)
+        taintedPathGuardChecks(taintedPathGuard, checked, branch) and
+        taintFlowsToCheckedNode(source, checked)
       |
         e = source.asExpr()
       )

repo-tests/codeql-go/ql/src/Security/CWE-117/LogInjection.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Log entries created from user input
+ * @description Building log entries from user-controlled sources is vulnerable to
+ *              insertion of forged log entries by a malicious user.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.8
+ * @precision high
+ * @id go/log-injection
+ * @tags security
+ *       external/cwe/cwe-117
+ */
+
+import go
+import semmle.go.security.LogInjection
+import DataFlow::PathGraph
+
+from LogInjection::Configuration c, DataFlow::PathNode source, DataFlow::PathNode sink
+where c.hasFlowPath(source, sink)
+select sink, source, sink, "This log write receives unsanitized user input from $@.",
+  source.getNode(), "here"

repo-tests/codeql-go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql

@@ -22,7 +22,7 @@ class InsecureIgnoreHostKey extends Function {
 class HostKeyCallbackFunc extends DataFlow::Node {
   HostKeyCallbackFunc() {
     exists(NamedType nt | nt.hasQualifiedName(CryptoSsh::packagePath(), "HostKeyCallback") |
-      getType().getUnderlyingType() = nt.getUnderlyingType()
+      this.getType().getUnderlyingType() = nt.getUnderlyingType()
     ) and
     // Restrict possible sources to either function definitions or
     // the result of some external function call (e.g. InsecureIgnoreHostKey())
@@ -71,7 +71,7 @@ class HostKeyCallbackAssignmentConfig extends DataFlow::Configuration {
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+  override predicate isSink(DataFlow::Node sink) { this.isSink(sink, _) }
 }
 
 /**

repo-tests/codeql-go/ql/src/Security/CWE-352/ConstantOauth2State.ql

@@ -46,7 +46,7 @@ class ConstantStateFlowConf extends DataFlow::Configuration {
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+  override predicate isSink(DataFlow::Node sink) { this.isSink(sink, _) }
 }
 
 /**
@@ -113,7 +113,7 @@ class PrivateUrlFlowsToAuthCodeUrlCall extends DataFlow::Configuration {
     exists(AuthCodeURL m | call = m.getACall() | sink = call.getReceiver())
   }
 
-  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+  override predicate isSink(DataFlow::Node sink) { this.isSink(sink, _) }
 }
 
 /**
@@ -142,7 +142,7 @@ class FlowToPrint extends DataFlow::Configuration {
     source = any(AuthCodeURL m).getACall().getResult()
   }
 
-  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+  override predicate isSink(DataFlow::Node sink) { this.isSink(sink, _) }
 }
 
 /** Holds if the provided `CallNode`'s result flows to an argument of a printer call. */