Make two separate queries

ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Untrusted data passed to unknown external API
+ * @description Data provided remotely is used in this unknown external API without sanitization, which could be a security risk.
+ * @id go/untrusted-data-to-unknown-external-api
+ * @kind path-problem
+ * @precision low
+ * @problem.severity error
+ * @tags security external/cwe/cwe-20
+ */
+
+import go
+import semmle.go.security.ExternalAPIs
+import DataFlow::PathGraph
+
+from
+  UntrustedDataToUnknownExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Call to " + sink.getNode().(UnknownExternalAPIDataNode).getFunctionDescription() +
+    " with untrusted data from $@.", source, source.toString()

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

@@ -17,8 +17,7 @@ private import Logrus
 abstract class SafeExternalAPIFunction extends Function { }
 
 private predicate isDefaultSafePackage(Package package) {
-  package.getPath() in ["time", "unicode/utf8", Logrus::packagePath(),
-        GolangOrgXNetWebsocket::packagePath(), GorillaWebsocket::packagePath(),
+  package.getPath() in ["time", "unicode/utf8",
         package("http://gopkg.in/go-playground/validator", "")]
 }
 
@@ -43,32 +42,6 @@ private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
   }
 }
 
-/** Gets the name of a method in package `p` which has a function model. */
-TaintTracking::FunctionModel getAMethodModelInPackage(Package p) {
-  p = result.getPackage() and
-  result instanceof Method and
-  result.getName() != "String" and
-  not exists(TaintTracking::FunctionModel baseMethod |
-    baseMethod != result and result.(Method).implements(baseMethod)
-  )
-}
-
-/** Gets the name of a package which has models for some functions. */
-Package getAPackageWithModels() {
-  exists(TaintTracking::FunctionModel f | not f instanceof Method | result = f.getPackage())
-  or
-  exists(getAMethodModelInPackage(result))
-}
-
-/** Holds if `n` is a sink for XSS, SQL injection or request forgery. */
-predicate isACommonSink(DataFlow::Node n) {
-  n instanceof SharedXss::Sink or
-  n instanceof SqlInjection::Sink or
-  n instanceof RequestForgery::Sink or
-  n instanceof CommandInjection::Sink or
-  n instanceof CleartextLogging::Sink
-}
-
 /** Holds if `callNode` is a local function pointer. */
 private predicate isProbableLocalFunctionPointer(DataFlow::CallNode callNode) {
   // Not a method call
@@ -107,10 +80,6 @@ class ExternalAPIDataNode extends DataFlow::Node {
     not call.getFile() instanceof TestFile and
     // Not already modeled as a taint step
     not exists(DataFlow::Node next | TaintTracking::localTaintStep(this, next)) and
-    // Not a sink for a commonly-used query
-    not isACommonSink(this) and
-    // Not in a package that has some functions modeled
-    not call.getTarget().getPackage() = getAPackageWithModels() and
     // Not a call to a known safe external API
     not call = any(SafeExternalAPIFunction f).getACall()
   }
@@ -140,6 +109,51 @@ class ExternalAPIDataNode extends DataFlow::Node {
   }
 }
 
+/** Gets the name of a method in package `p` which has a function model. */
+TaintTracking::FunctionModel getAMethodModelInPackage(Package p) {
+  p = result.getPackage() and
+  result instanceof Method and
+  result.getName() != "String" and
+  not exists(TaintTracking::FunctionModel baseMethod |
+    baseMethod != result and result.(Method).implements(baseMethod)
+  )
+}
+
+/** Gets the name of a package which has models for some functions. */
+Package getAPackageWithFunctionModels() {
+  exists(TaintTracking::FunctionModel f | not f instanceof Method | result = f.getPackage())
+  or
+  exists(getAMethodModelInPackage(result))
+}
+
+/** Gets the name of a package which has models. */
+Package getAPackageWithModels() {
+  result = getAPackageWithFunctionModels()
+  or
+  // An incomplete list of packages which have been modelled but do not have any function models
+  result.getPath() in [Logrus::packagePath(), GolangOrgXNetWebsocket::packagePath(),
+        GorillaWebsocket::packagePath()]
+}
+
+/** Holds if `n` is a sink for XSS, SQL injection or request forgery. */
+predicate isACommonSink(DataFlow::Node n) {
+  n instanceof SharedXss::Sink or
+  n instanceof SqlInjection::Sink or
+  n instanceof RequestForgery::Sink or
+  n instanceof CommandInjection::Sink or
+  n instanceof CleartextLogging::Sink
+}
+
+/** A node representing data being passed to an external API. */
+class UnknownExternalAPIDataNode extends ExternalAPIDataNode {
+  UnknownExternalAPIDataNode() {
+    // Not a sink for a commonly-used query
+    not isACommonSink(this) and
+    // Not in a package that has some functions modeled
+    not call.getTarget().getPackage() = getAPackageWithModels()
+  }
+}
+
 /** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
 class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
   UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
@@ -149,6 +163,15 @@ class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
 }
 
+/** A configuration for tracking flow from `RemoteFlowSource`s to `UnknownExternalAPIDataNode`s. */
+class UntrustedDataToUnknownExternalAPIConfig extends TaintTracking::Configuration {
+  UntrustedDataToUnknownExternalAPIConfig() { this = "UntrustedDataToUnknownExternalAPIConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof UnknownExternalAPIDataNode }
+}
+
 /** A node representing untrusted data being passed to an external API. */
 class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
   UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }