Java: Queries to detect remote source flow to CORS header

java/ql/src/Security/CWE/CWE-346/UnvalidatedCors.java

@@ -0,0 +1,40 @@
+import java.io.IOException;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.lang3.StringUtils;
+
+public class CorsFilter implements Filter {
+    public void init(FilterConfig filterConfig) throws ServletException {
+        // init
+    }
+
+    public void doFilter(ServletRequest req, ServletResponse res,
+            FilterChain chain) throws IOException, ServletException {
+        HttpServletRequest request = (HttpServletRequest) req;
+        HttpServletResponse response = (HttpServletResponse) res;
+        String url = request.getHeader("Origin");
+
+        if (!StringUtils.isEmpty(url)) {
+            String val = response.getHeader("Access-Control-Allow-Origin");
+
+            if (StringUtils.isEmpty(val)) {
+                response.addHeader("Access-Control-Allow-Origin", url);
+                response.addHeader("Access-Control-Allow-Credentials", "true");
+            }
+        }
+
+        chain.doFilter(req, res);
+    }
+
+    public void destroy() {
+        // destroy
+    }
+}

java/ql/src/Security/CWE/CWE-346/UnvalidatedCors.qhelp

@@ -0,0 +1,75 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+	<p>
+
+		A server can send the
+		<code>"Access-Control-Allow-Credentials"</code> CORS header to control
+		when a browser may send user credentials in Cross-Origin HTTP
+		requests.
+
+	</p>
+	<p>
+
+		When the <code>Access-Control-Allow-Credentials</code> header
+		is <code>"true"</code>, the <code>Access-Control-Allow-Origin</code>
+		header must have a value different from <code>"*"</code> in order to
+		make browsers accept the header. Therefore, to allow multiple origins
+		for Cross-Origin requests with credentials, the server must
+		dynamically compute the value of the
+		<code>"Access-Control-Allow-Origin"</code> header. Computing this
+		header value from information in the request to the server can
+		therefore potentially allow an attacker to control the origins that
+		the browser sends credentials to.
+
+	</p>
+
+
+
+</overview>
+
+<recommendation>
+	<p>
+
+		When the <code>Access-Control-Allow-Credentials</code> header
+		value is <code>"true"</code>, a dynamic computation of the
+		<code>Access-Control-Allow-Origin</code> header must involve
+		sanitization if it relies on user-controlled input.
+
+
+	</p>
+	<p>
+
+		Since the <code>"null"</code> origin is easy to obtain for an
+		attacker, it is never safe to use <code>"null"</code> as the value of
+		the <code>Access-Control-Allow-Origin</code> header when the
+		<code>Access-Control-Allow-Credentials</code> header value is
+		<code>"true"</code>.
+
+	</p>
+</recommendation>
+
+<example>
+	<p>
+
+		In the example below, the server allows the browser to send
+		user credentials in a Cross-Origin request. The request header
+		<code>origins</code> controls the allowed origins for such a
+		Cross-Origin request.
+
+	</p>
+
+	<sample src="UnvalidatedCors.java"/>
+
+</example>
+
+<references>
+	<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin">CORS, Access-Control-Allow-Origin</a>.</li>
+	<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials">CORS, Access-Control-Allow-Credentials</a>.</li>
+	<li>PortSwigger: <a href="http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html">Exploiting CORS Misconfigurations for Bitcoins and Bounties</a></li>
+	<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a></li>
+</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-346/UnvalidatedCors.ql

@@ -0,0 +1,55 @@
+/**
+ * @name Cors header being set from remote source
+ * @description Cors header is being set from remote source, allowing to control the origin.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/unvalidated-cors-origin-set
+ * @tags security
+ *       external/cwe/cwe-346
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.frameworks.Servlets
+import semmle.code.java.dataflow.TaintTracking
+import DataFlow::PathGraph
+
+// Check for Access-Control-Allow-Credentials as well, this ensures fair chances of exploitability.
+predicate satisfyAllowCredentials(MethodAccess header, MethodAccess check) {
+  header.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() =
+    "access-control-allow-credentials" and
+  header.getArgument(1).(CompileTimeConstantExpr).getStringValue() = "true" and
+  header.getEnclosingCallable() = check.getEnclosingCallable()
+}
+
+predicate checkAccessControlAllowOriginHeader(Expr expr) {
+  expr.(CompileTimeConstantExpr).getStringValue().toLowerCase() = "access-control-allow-origin"
+}
+
+class CorsOriginConfig extends TaintTracking::Configuration {
+  CorsOriginConfig() { this = "CORSOriginConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ResponseSetHeaderMethod h, MethodAccess m |
+      m = h.getAReference() and
+      checkAccessControlAllowOriginHeader(m.getArgument(0)) and
+      satisfyAllowCredentials(h.getAReference(), m) and
+      sink.asExpr() = m.getArgument(1)
+    )
+    or
+    exists(ResponseAddHeaderMethod a, MethodAccess m |
+      m = a.getAReference() and
+      checkAccessControlAllowOriginHeader(m.getArgument(0)) and
+      satisfyAllowCredentials(a.getAReference(), m) and
+      sink.asExpr() = m.getArgument(1)
+    )
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, CorsOriginConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Cors header is being set using user controlled value $@.",
+  source.getNode(), "user-provided value"

java/ql/test/query-tests/security/CWE-346/UnvalidatedCors.expected

@@ -0,0 +1,7 @@
+edges
+| UnvalidatedCors.java:34:22:34:48 | getHeader(...) : String | UnvalidatedCors.java:40:67:40:69 | url |
+nodes
+| UnvalidatedCors.java:34:22:34:48 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UnvalidatedCors.java:40:67:40:69 | url | semmle.label | url |
+#select
+| UnvalidatedCors.java:40:67:40:69 | url | UnvalidatedCors.java:34:22:34:48 | getHeader(...) : String | UnvalidatedCors.java:40:67:40:69 | url | Cors header is being set using user controlled value $@. | UnvalidatedCors.java:34:22:34:48 | getHeader(...) | user-provided value |

java/ql/test/query-tests/security/CWE-346/UnvalidatedCors.java

@@ -0,0 +1,52 @@
+package com.mossle.core.servlet;
+
+import java.io.IOException;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * <pre>
+ * <script>
+ *   $(function () {
+ *     $.ajaxSetup({crossDomain: true, xhrFields: {withCredentials: true}});
+ *   });
+ * </script>
+ * </pre>
+ */
+public class UnvalidatedCors implements Filter {
+    public void init(FilterConfig filterConfig) throws ServletException {
+        // init
+    }
+
+    public void doFilter(ServletRequest req, ServletResponse res,
+            FilterChain chain) throws IOException, ServletException {
+        HttpServletRequest request = (HttpServletRequest) req;
+        HttpServletResponse response = (HttpServletResponse) res;
+        String url = request.getHeader("Origin");
+
+        if (!StringUtils.isEmpty(url)) {
+            String val = response.getHeader("Access-Control-Allow-Origin");
+
+            if (StringUtils.isEmpty(val)) {
+                response.addHeader("Access-Control-Allow-Origin", url);
+                response.addHeader("Access-Control-Allow-Credentials", "true");
+            }
+        }
+
+        chain.doFilter(req, res);
+    }
+
+    public void destroy() {
+        // destroy
+    }
+}
+

java/ql/test/query-tests/security/CWE-346/UnvalidatedCors.qlref

@@ -0,0 +1 @@
+Security/CWE/CWE-346/UnvalidatedCors.ql

java/ql/test/query-tests/security/CWE-346/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/apache-commons-lang3-3.7

java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/StringUtils.java

@@ -0,0 +1,7 @@
+package org.apache.commons.lang3;
+
+public class StringUtils {
+    public static boolean isEmpty(final CharSequence cs) {
+        return cs == null || cs.length() == 0;
+    }
+}

java/ql/test/stubs/servlet-api-2.4/javax/servlet/Filter.java

@@ -0,0 +1,11 @@
+package javax.servlet;
+
+import java.io.IOException;
+
+public interface Filter {
+    default public void init(FilterConfig filterConfig) throws ServletException {}
+    public void doFilter(ServletRequest request, ServletResponse response,
+                         FilterChain chain)
+            throws IOException, ServletException;
+    default public void destroy() {}
+}

java/ql/test/stubs/servlet-api-2.4/javax/servlet/FilterChain.java

@@ -0,0 +1,7 @@
+package javax.servlet;
+
+import java.io.IOException;
+
+public interface FilterChain {
+   public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException;
+}

java/ql/test/stubs/servlet-api-2.4/javax/servlet/FilterConfig.java

@@ -0,0 +1,10 @@
+package javax.servlet;
+
+import java.util.Enumeration;
+
+public interface FilterConfig {
+    public String getFilterName();
+    public ServletContext getServletContext();
+    public String getInitParameter(String name);
+    public Enumeration<String> getInitParameterNames();
+}

java/ql/test/stubs/servlet-api-2.4/javax/servlet/http/HttpServletResponse.java

@@ -24,6 +24,7 @@
 package javax.servlet.http;
 
 import java.io.IOException;
+import java.util.Collection;
 import javax.servlet.ServletResponse;
 
 public interface HttpServletResponse extends ServletResponse {
@@ -44,6 +45,11 @@ public interface HttpServletResponse extends ServletResponse {
     public void addIntHeader(String name, int value);
     public void setStatus(int sc);
     public void setStatus(int sc, String sm);
+    public int getStatus();
+    public String getHeader(String name);
+    public Collection<String> getHeaders(String name);
+    public Collection<String> getHeaderNames();
+
 
     public static final int SC_CONTINUE = 100;
     public static final int SC_SWITCHING_PROTOCOLS = 101;