First draft

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

@@ -0,0 +1,8 @@
+public class XSS extends HttpServlet {
+	protected void doGet(HttpServletRequest request, HttpServletResponse response)
+	throws ServletException, IOException {
+		// BAD: a request parameter is written directly to an error response page
+		response.sendError(HttpServletResponse.SC_NOT_FOUND,
+				"The page \"" + request.getParameter("page") + "\" was not found.");
+	}
+}

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

@@ -0,0 +1,12 @@
+public class SQLInjection extends HttpServlet {
+	protected void doGet(HttpServletRequest request, HttpServletResponse response)
+	throws ServletException, IOException {
+
+		StringBuilder sqlQueryBuilder = new StringBuilder();
+		sqlQueryBuilder.append("SELECT * FROM user WHERE user_id='");
+		sqlQueryBuilder.append(request.getParameter("user_id"));
+		sqlQueryBuilder.append("'");
+
+		// ...
+	}
+}

ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp

@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
+unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
+may be relevant for security analysis of this application.</p>
+
+<p>An external API is defined as a method call to a method that is not defined in the source code, not overridden 
+in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the 
+Go standard library, third party dependencies or from internal dependencies. The query will report the method 
+signature with a fully qualified name, along with either <code>[param x]</code>, where <code>x</code> indicates the 
+position of the parameter receiving the untrusted data or <code>[receiver]</code> indicating the untrusted data is 
+used as the receiver of the method call.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, no action is required.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
+  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIMethod</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>If the query were to return the API <code>javax.servlet.http.HttpServletResponse.sendError(int, java.lang.String) [param 1]</code> 
+then we should first consider whether this a security relevant sink. In this case, this is writing to a HTTP response, so we should 
+consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
+
+<p>If the query were to return the API <code>java.lang.StringBuilder.append(java.lang.String) [param 0]</code>, then this should be
+reviewed as a possible taint step, because tainted data would flow from the 0th argument to the qualifier of the call.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,17 @@
+/**
+ * @name Frequency counts for external APIs that are used with untrusted data
+ * @description This reports the external APIs that are used with untrusted data, along with how
+ *              frequently the API is called, and how many unique sources of untrusted data flow
+ *              to it.
+ * @id go/count-untrusted-data-external-api
+ * @kind table
+ * @tags security external/cwe/cwe-20
+ */
+
+import go
+import semmle.go.security.ExternalAPIs
+
+from ExternalAPIUsedWithUntrustedData externalAPI
+select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
+  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
+    numberOfUntrustedSources desc

ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp

@@ -0,0 +1,59 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query
+provides data for security reviews of the application and you can also use it to identify external APIs that should
+be modeled as either taint steps, or sinks for specific problems.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, not overridden 
+in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the
+Go standard library, third-party dependencies or from internal dependencies. The query reports uses of 
+untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
+  that the result is a false positive because this data is sanitized.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
+  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
+  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIMethod</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>In this first example, a request parameter is read from <code>HttpServletRequest</code> and then ultimately used in a call to the 
+<code>HttpServletResponse.sendError</code> external API:</p>
+
+<sample src="ExternalAPISinkExample.java" />
+
+<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
+and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
+some existing sanitization.</p>
+
+<p>In this second example, again a request parameter is read from <code>HttpServletRequest</code>.</p>
+
+<sample src="ExternalAPITaintStepExample.java" />
+
+<p>If the query reported the call to <code>StringBuilder.append</code> on line 7, this would suggest that this external API is 
+not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
+re-run the query to determine what additional results might be found. In this example, it seems likely that the result of the
+<code>StringBuilder</code> will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,19 @@
+/**
+ * @name Untrusted data passed to external API
+ * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
+ * @id go/untrusted-data-to-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 UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Call to " + sink.getNode().(ExternalAPIDataNode).getFunctionDescription() +
+    " with untrusted data from $@.", source, source.toString()

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

@@ -0,0 +1,109 @@
+/**
+ * Definitions for reasoning about untrusted data used in APIs defined outside the
+ * database.
+ */
+
+import go
+
+/**
+ * A `Function` that is considered a "safe" external API from a security perspective.
+ */
+abstract class SafeExternalAPIFunction extends Function { }
+
+/** The default set of "safe" external APIs. */
+private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
+  DefaultSafeExternalAPIFunction() {
+    this instanceof BuiltinFunction
+    // TODO: Add more external API functions which we know are safe here
+  }
+}
+
+/** A node representing data being passed to an external API. */
+class ExternalAPIDataNode extends DataFlow::Node {
+  DataFlow::CallNode call;
+  int i;
+
+  ExternalAPIDataNode() {
+    (
+      // Argument to call to a function
+      this = call.getArgument(i)
+      or
+      // Receiver to a call to a method which returns non trivial value
+      this = call.getReceiver() and
+      i = -1 and
+      (
+        call.getTarget().getNumResult() >= 2
+        or
+        call.getTarget().getNumResult() = 1 and
+        not call.getTarget().getResultType(0) instanceof BoolType
+      )
+    ) and
+    // Not defined in the code that is being analysed
+    not exists(call.getACallee().getBody()) and
+    // Not already modeled as a taint step
+    not exists(DataFlow::Node next | TaintTracking::localTaintStep(this, next)) and
+    // Not a call to a known safe external API
+    not call = any(SafeExternalAPIFunction f).getACall()
+  }
+
+  /** Gets the called API `Function`. */
+  Function getFunction() { result.getACall() = call }
+
+  /** Gets the index which is passed untrusted data (where -1 indicates the receiver). */
+  int getIndex() { result = i }
+
+  /** Gets the description of the function being called. */
+  string getFunctionDescription() { result = call.getCalleeName() }
+}
+
+/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
+class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
+  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
+}
+
+/** A node representing untrusted data being passed to an external API. */
+class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
+  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
+
+  /** Gets a source of untrusted data which is passed to this external API data node. */
+  DataFlow::Node getAnUntrustedSource() {
+    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
+  }
+}
+
+private newtype TExternalAPI =
+  TExternalAPIParameter(Function m, int index) {
+    exists(UntrustedExternalAPIDataNode n |
+      m = n.getFunction() and
+      index = n.getIndex()
+    )
+  }
+
+/** An external API which is used with untrusted data. */
+class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
+  /** Gets a possibly untrusted use of this external API. */
+  UntrustedExternalAPIDataNode getUntrustedDataNode() {
+    this = TExternalAPIParameter(result.getFunction(), result.getIndex())
+  }
+
+  /** Gets the number of untrusted sources used with this external API. */
+  int getNumberOfUntrustedSources() {
+    result = count(getUntrustedDataNode().getAnUntrustedSource())
+  }
+
+  /** Gets a textual representation of this element. */
+  string toString() {
+    exists(Function f, int index, string indexString |
+      if index = -1 then indexString = "receiver" else indexString = "param " + index
+    |
+      this = TExternalAPIParameter(f, index) and
+      if exists(f.getQualifiedName())
+      then result = f.getQualifiedName() + " [" + indexString + "]"
+      else result = f.getName() + " [" + indexString + "]"
+    )
+  }
+}