Add query to detect CORS misconfiguration

ql/src/experimental/CWE-942/CorsMisconfiguration.qhelp

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+	When configuring the allowed origins for CORS requests that allow passing cookies
+	(i.e. the <code>Access-Control-Allow-Credentials</code> is set to true),
+	if the <code>Access-Control-Allow-Origin</code> header is set to a user-provided value (and not correctly validated),
+	or is set to special values such as <code>*</code> or <code>null</code>, then the users of that application might be
+	vulnerable to attacks where the attacker impersonates the user and sends request on their behalf.
+</p>
+</overview>
+<recommendation>
+<p>
+	When configuring CORS that allow credentials passing,
+	it's best not to use user-provided values for the allowed origins response header,
+	especially if the cookies grant session permissions on the user's account.
+</p>
+<p>
+	It also can be very dangerous to set the allowed origins to <code>*</code> or <code>null</code> (which can be bypassed).
+</p>
+</recommendation>
+<example>
+<p>
+	The first example shows a few possible CORS misconfiguration cases:
+</p>
+<sample src="CorsMisconfigurationBad.go"/>
+<p>
+	The second example show better configurations:
+</p>
+<sample src="CorsMisconfigurationGood.go"/>
+</example>
+<references>
+<li>
+	Reference 1: <a href="https://portswigger.net/web-security/cors">PortSwigger Web Security Academy on CORS</a>.
+</li>
+<li>
+	Reference 2: <a href="https://www.youtube.com/watch?v=wgkj4ZgxI4c">AppSec EU 2017 Exploiting CORS Misconfigurations For Bitcoins And Bounties by James Kettle</a>.
+</li>
+</references>
+</qhelp>

ql/src/experimental/CWE-942/CorsMisconfiguration.ql

@@ -0,0 +1,84 @@
+/**
+ * @name CORS misconfiguration
+ * @description If a CORS policy is configured to accept an origin value obtained from the request data,
+ * 				or is set to `*` or `null`, and it allows credential sharing, then the users of the
+ * 				application are vulnerable to the same range of attacks as in XSS (credential stealing, etc.).
+ * @kind problem
+ * @problem.severity warning
+ * @id go/cors-misconfiguration
+ * @tags security
+ *       external/cwe/cwe-942
+ *       external/cwe/cwe-346
+ */
+
+import go
+
+/**
+ * Provides the name of the `Access-Control-Allow-Origin` header key.
+ */
+string headerAllowOrigin() { result = "Access-Control-Allow-Origin".toLowerCase() }
+
+/**
+ * Provides the name of the `Access-Control-Allow-Credentials` header key.
+ */
+string headerAllowCredentials() { result = "Access-Control-Allow-Credentials".toLowerCase() }
+
+/**
+ * A taint-tracking configuration for reasoning about when an UntrustedFlowSource
+ * flows to a HeaderWrite that writes an `Access-Control-Allow-Origin` header's value.
+ */
+class FlowsUntrustedToAllowOriginHeader extends TaintTracking::Configuration {
+  FlowsUntrustedToAllowOriginHeader() { this = "from-untrusted-to-allow-origin-header-value" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
+
+  predicate isSink(DataFlow::Node sink, HTTP::HeaderWrite hw) {
+    hw.getHeaderName() = headerAllowOrigin() and sink = hw.getValue()
+  }
+
+  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
+}
+
+/**
+ * Holds if the provided `allowOriginHW` HeaderWrite's parent ResponseWriter
+ * also has another HeaderWrite that sets a `Access-Control-Allow-Credentials`
+ * header to `true`.
+ */
+predicate allowCredentialsIsSetToTrue(HTTP::HeaderWrite allowOriginHW) {
+  exists(HTTP::HeaderWrite allowCredentialsHW |
+    allowCredentialsHW.getHeaderName() = headerAllowCredentials() and
+    allowCredentialsHW.getHeaderValue() = "true"
+  |
+    allowOriginHW.getResponseWriter() = allowCredentialsHW.getResponseWriter()
+  )
+}
+
+/**
+ * Holds if the provided `allowOriginHW` HeaderWrite's value is set using an
+ * UntrustedFlowSource.
+ * The `message` parameter is populated with the warning message to be returned by the query.
+ */
+predicate flowsFromUntrustedToAllowOrigin(HTTP::HeaderWrite allowOriginHW, string message) {
+  exists(FlowsUntrustedToAllowOriginHeader cfg, DataFlow::PathNode source, DataFlow::PathNode sink |
+    cfg.hasFlowPath(source, sink) and
+    cfg.isSink(sink.getNode(), allowOriginHW)
+  |
+    message =
+      headerAllowOrigin() + " header is set to a user-defined value, and " +
+        headerAllowCredentials() + " is set to `true`"
+  )
+}
+
+from HTTP::HeaderWrite allowOriginHW, string message
+where
+  (
+    flowsFromUntrustedToAllowOrigin(allowOriginHW, message)
+    or
+    allowOriginHW.getHeaderName() = headerAllowOrigin() and
+    allowOriginHW.getHeaderValue() = ["*", "null"] and
+    message =
+      headerAllowOrigin() + " header is set to `" + allowOriginHW.getHeaderValue() + "`, and " +
+        headerAllowCredentials() + " is set to `true`"
+  ) and
+  allowCredentialsIsSetToTrue(allowOriginHW)
+select allowOriginHW, message

ql/src/experimental/CWE-942/CorsMisconfigurationBad.go

@@ -0,0 +1,34 @@
+package main
+
+import "net/http"
+
+func main() {}
+
+// bad is an example of a bad implementation
+func bad1() {
+	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		// BAD: all origins are allowed,
+		// and Access-Control-Allow-Credentials is set to 'true'.
+		w.Header().Set("Access-Control-Allow-Origin", "*")
+		w.Header().Set("Access-Control-Allow-Credentials", "true")
+	})
+}
+
+func bad2() {
+	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		// BAD: 'null' origin is allowed,
+		// and Access-Control-Allow-Credentials is set to 'true'.
+		w.Header().Set("Access-Control-Allow-Origin", "null")
+		w.Header().Set("Access-Control-Allow-Credentials", "true")
+	})
+}
+
+func bad3() {
+	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
+		// and `Access-Control-Allow-Credentials` is set to 'true':
+		origin := req.Header.Get("origin")
+		w.Header().Set("Access-Control-Allow-Origin", origin)
+		w.Header().Set("Access-Control-Allow-Credentials", "true")
+	})
+}

ql/src/experimental/CWE-942/CorsMisconfigurationGood.go

@@ -0,0 +1,32 @@
+package main
+
+import "net/http"
+
+// good1 is an example of a good implementation
+func good1() {
+	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		// OK-ish: all origins are allowed,
+		// but Access-Control-Allow-Credentials is not set.
+		w.Header().Set("Access-Control-Allow-Origin", "*")
+	})
+}
+
+func good2() {
+	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		// OK-ish: all origins are allowed,
+		// and some write methods are allowed,
+		// BUT `Access-Control-Allow-Credentials` is not set:
+		w.Header().Set("Access-Control-Allow-Origin", "*")
+		w.Header().Set("Access-Control-Allow-Methods", "GET,POST,PUT")
+	})
+}
+
+func good3() {
+	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
+		// BUT `Access-Control-Allow-Credentials` is set to 'false':
+		origin := req.Header.Get("origin")
+		w.Header().Set("Access-Control-Allow-Origin", origin)
+		w.Header().Set("Access-Control-Allow-Credentials", "false")
+	})
+}

ql/test/experimental/CWE-942/CorsMisconfiguration.expected

@@ -0,0 +1,7 @@
+| CorsMisconfiguration.go:15:4:15:53 | call to Set | access-control-allow-origin header is set to `*`, and access-control-allow-credentials is set to `true` |
+| CorsMisconfiguration.go:21:4:21:41 | call to Set | access-control-allow-origin header is set to `*`, and access-control-allow-credentials is set to `true` |
+| CorsMisconfiguration.go:41:4:41:56 | call to Set | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
+| CorsMisconfiguration.go:47:4:47:42 | call to Set | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
+| CorsMisconfiguration.go:68:4:68:53 | call to Set | access-control-allow-origin header is set to `*`, and access-control-allow-credentials is set to `true` |
+| CorsMisconfiguration.go:86:4:86:44 | call to Set | access-control-allow-origin header is set to a user-defined value, and access-control-allow-credentials is set to `true` |
+| CorsMisconfiguration.go:93:4:93:56 | call to Set | access-control-allow-origin header is set to a user-defined value, and access-control-allow-credentials is set to `true` |

ql/test/experimental/CWE-942/CorsMisconfiguration.go

@@ -0,0 +1,110 @@
+package main
+
+import "net/http"
+
+const (
+	HeaderAllowOrigin      = "Access-Control-Allow-Origin"
+	HeaderAllowCredentials = "Access-Control-Allow-Credentials"
+)
+
+func main() {
+	{
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: all origins are allowed,
+			// and Access-Control-Allow-Credentials is set to 'true'.
+			w.Header().Set("Access-Control-Allow-Origin", "*")
+			w.Header().Set("Access-Control-Allow-Credentials", "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: all origins are allowed,
+			// and `Access-Control-Allow-Credentials` is set to 'true':
+			w.Header().Set(HeaderAllowOrigin, "*")
+			w.Header().Set("Access-Control-Allow-Credentials", "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK: all origins are allowed,
+			// but Access-Control-Allow-Credentials is set to 'false'.
+			w.Header().Set("Access-Control-Allow-Origin", "*")
+			w.Header().Set("Access-Control-Allow-Credentials", "false")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK: all origins are allowed,
+			// but Access-Control-Allow-Credentials is not set.
+			w.Header().Set("Access-Control-Allow-Origin", "*")
+		})
+	}
+	{
+		const Null = "null"
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: 'null' origin is allowed,
+			// and Access-Control-Allow-Credentials is set to 'true'.
+			w.Header().Set("Access-Control-Allow-Origin", "null")
+			w.Header().Set("Access-Control-Allow-Credentials", "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: 'null' origin is allowed,
+			// and `Access-Control-Allow-Credentials` is set to 'true':
+			w.Header().Set(HeaderAllowOrigin, Null)
+			w.Header().Set("Access-Control-Allow-Credentials", "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK: 'null' origin is allowed,
+			// but Access-Control-Allow-Credentials is set to 'false'.
+			w.Header().Set("Access-Control-Allow-Origin", Null)
+			w.Header().Set("Access-Control-Allow-Credentials", "false")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK: 'null' origin is allowed,
+			// but Access-Control-Allow-Credentials is not set.
+			w.Header().Set("Access-Control-Allow-Origin", Null)
+		})
+	}
+
+	{
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: all origins are allowed,
+			// and some write methods are allowed,
+			// and `Access-Control-Allow-Credentials` is set to 'true':
+			w.Header().Set("Access-Control-Allow-Origin", "*")
+			w.Header().Set("Access-Control-Allow-Methods", "GET,POST,PUT")
+			w.Header().Set("Access-Control-Allow-Credentials", "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK: all origins are allowed,
+			// and some write methods are allowed,
+			// BUT `Access-Control-Allow-Credentials` is not set:
+			w.Header().Set("Access-Control-Allow-Origin", "*")
+			w.Header().Set("Access-Control-Allow-Methods", "GET,POST,PUT")
+		})
+	}
+
+	{
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
+			// and `Access-Control-Allow-Credentials` is set to 'true':
+			origin := req.Header.Get("origin")
+			w.Header().Set(HeaderAllowOrigin, origin)
+			w.Header().Set("Access-Control-Allow-Credentials", "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
+			// and `Access-Control-Allow-Credentials` is set to 'true':
+			origin := req.Header.Get("origin")
+			w.Header().Set("Access-Control-Allow-Origin", origin)
+			w.Header().Set(HeaderAllowCredentials, "true")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
+			// BUT `Access-Control-Allow-Credentials` is set to 'false':
+			origin := req.Header.Get("origin")
+			w.Header().Set("Access-Control-Allow-Origin", origin)
+			w.Header().Set("Access-Control-Allow-Credentials", "false")
+		})
+		http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+			// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
+			// BUT `Access-Control-Allow-Credentials` is not set:
+			origin := req.Header.Get("origin")
+			w.Header().Set("Access-Control-Allow-Origin", origin)
+		})
+	}
+}

ql/test/experimental/CWE-942/CorsMisconfiguration.qlref

@@ -0,0 +1 @@
+experimental/CWE-942/CorsMisconfiguration.ql