Skip to content

Commit 27408fe

Browse files
aschackmullaschackmull
authored
Merge pull request github#5008 from torque59/cwe-346
Java: Queries to detect remote source flow origins to CORS header.
2 parents 9a56601 + abdebc2 commit 27408fe

File tree

12 files changed

+288
-1
lines changed

12 files changed

+288
-1
lines changed

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import java.io.IOException;
2+
3+
import javax.servlet.Filter;
4+
import javax.servlet.FilterChain;
5+
import javax.servlet.FilterConfig;
6+
import javax.servlet.ServletException;
7+
import javax.servlet.ServletRequest;
8+
import javax.servlet.ServletResponse;
9+
import javax.servlet.http.HttpServletRequest;
10+
import javax.servlet.http.HttpServletResponse;
11+
12+
import org.apache.commons.lang3.StringUtils;
13+
14+
public class CorsFilter implements Filter {
15+
public void init(FilterConfig filterConfig) throws ServletException {}
16+
17+
public void doFilter(ServletRequest req, ServletResponse res,
18+
FilterChain chain) throws IOException, ServletException {
19+
HttpServletRequest request = (HttpServletRequest) req;
20+
HttpServletResponse response = (HttpServletResponse) res;
21+
String url = request.getHeader("Origin");
22+
23+
if (!StringUtils.isEmpty(url)) {
24+
String val = response.getHeader("Access-Control-Allow-Origin");
25+
26+
if (StringUtils.isEmpty(val)) {
27+
response.addHeader("Access-Control-Allow-Origin", url); // BAD -> User controlled CORS header being set here.
28+
response.addHeader("Access-Control-Allow-Credentials", "true");
29+
}
30+
}
31+
32+
if (!StringUtils.isEmpty(url)) {
33+
List<String> checkorigins = Arrays.asList("www.example.com", "www.sub.example.com");
34+
35+
if (checkorigins.contains(url)) { // GOOD -> Origin is validated here.
36+
response.addHeader("Access-Control-Allow-Origin", url);
37+
response.addHeader("Access-Control-Allow-Credentials", "true");
38+
}
39+
}
40+
41+
chain.doFilter(req, res);
42+
}
43+
44+
public void destroy() {}
45+
}

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
9+
A server can send the
10+
<code>Access-Control-Allow-Credentials</code> CORS header to control
11+
when a browser may send user credentials in Cross-Origin HTTP
12+
requests.
13+
14+
</p>
15+
<p>
16+
17+
When the <code>Access-Control-Allow-Credentials</code> header
18+
is <code>true</code>, the <code>Access-Control-Allow-Origin</code>
19+
header must have a value different from <code>*</code> in order
20+
for browsers to accept the header. Therefore, to allow multiple origins
21+
for cross-origin requests with credentials, the server must
22+
dynamically compute the value of the
23+
<code>Access-Control-Allow-Origin</code> header. Computing this
24+
header value from information in the request to the server can
25+
therefore potentially allow an attacker to control the origins that
26+
the browser sends credentials to.
27+
28+
</p>
29+
30+
31+
32+
</overview>
33+
34+
<recommendation>
35+
<p>
36+
37+
When the <code>Access-Control-Allow-Credentials</code> header
38+
value is <code>true</code>, a dynamic computation of the
39+
<code>Access-Control-Allow-Origin</code> header must involve
40+
sanitization if it relies on user-controlled input.
41+
42+
43+
</p>
44+
<p>
45+
46+
Since the <code>null</code> origin is easy to obtain for an
47+
attacker, it is never safe to use <code>null</code> as the value of
48+
the <code>Access-Control-Allow-Origin</code> header when the
49+
<code>Access-Control-Allow-Credentials</code> header value is
50+
<code>true</code>.A null origin can be set by an attacker using a sandboxed iframe.
51+
A more detailed explanation is available in the portswigger blogpost referenced below.
52+
53+
</p>
54+
</recommendation>
55+
56+
<example>
57+
<p>
58+
59+
In the example below, the server allows the browser to send
60+
user credentials in a cross-origin request. The request header
61+
<code>origins</code> controls the allowed origins for such a
62+
Cross-Origin request.
63+
64+
</p>
65+
66+
<sample src="UnvalidatedCors.java"/>
67+
68+
</example>
69+
70+
<references>
71+
<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>
72+
<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>
73+
<li>PortSwigger: <a href="http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html">Exploiting CORS Misconfigurations for Bitcoins and Bounties</a></li>
74+
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a></li>
75+
</references>
76+
</qhelp>

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* @name CORS is derived from untrusted input
3+
* @description CORS header is derived from untrusted input, allowing a remote user to control which origins are trusted.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id java/unvalidated-cors-origin-set
8+
* @tags security
9+
* external/cwe/cwe-346
10+
*/
11+
12+
import java
13+
import semmle.code.java.dataflow.FlowSources
14+
import semmle.code.java.frameworks.Servlets
15+
import semmle.code.java.dataflow.TaintTracking
16+
import semmle.code.java.dataflow.TaintTracking2
17+
import DataFlow::PathGraph
18+
19+
/**
20+
* Holds if `header` sets `Access-Control-Allow-Credentials` to `true`. This ensures fair chances of exploitability.
21+
*/
22+
private predicate setsAllowCredentials(MethodAccess header) {
23+
(
24+
header.getMethod() instanceof ResponseSetHeaderMethod or
25+
header.getMethod() instanceof ResponseAddHeaderMethod
26+
) and
27+
header.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() =
28+
"access-control-allow-credentials" and
29+
header.getArgument(1).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "true"
30+
}
31+
32+
private class CorsProbableCheckAccess extends MethodAccess {
33+
CorsProbableCheckAccess() {
34+
getMethod().hasName("contains") and
35+
getMethod().getDeclaringType().getASourceSupertype*() instanceof CollectionType
36+
or
37+
getMethod().hasName("containsKey") and
38+
getMethod().getDeclaringType().getASourceSupertype*() instanceof MapType
39+
or
40+
getMethod().hasName("equals") and
41+
getQualifier().getType() instanceof TypeString
42+
}
43+
}
44+
45+
private Expr getAccessControlAllowOriginHeaderName() {
46+
result.(CompileTimeConstantExpr).getStringValue().toLowerCase() = "access-control-allow-origin"
47+
}
48+
49+
/**
50+
* This taintflow2 configuration checks if there is a flow from source node towards CorsProbableCheckAccess methods.
51+
*/
52+
class CorsSourceReachesCheckConfig extends TaintTracking2::Configuration {
53+
CorsSourceReachesCheckConfig() { this = "CorsOriginConfig" }
54+
55+
override predicate isSource(DataFlow::Node source) { any(CorsOriginConfig c).hasFlow(source, _) }
56+
57+
override predicate isSink(DataFlow::Node sink) {
58+
sink.asExpr() = any(CorsProbableCheckAccess check).getAnArgument()
59+
}
60+
}
61+
62+
private class CorsOriginConfig extends TaintTracking::Configuration {
63+
CorsOriginConfig() { this = "CorsOriginConfig" }
64+
65+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
66+
67+
override predicate isSink(DataFlow::Node sink) {
68+
exists(MethodAccess corsHeader, MethodAccess allowCredentialsHeader |
69+
(
70+
corsHeader.getMethod() instanceof ResponseSetHeaderMethod or
71+
corsHeader.getMethod() instanceof ResponseAddHeaderMethod
72+
) and
73+
getAccessControlAllowOriginHeaderName() = corsHeader.getArgument(0) and
74+
setsAllowCredentials(allowCredentialsHeader) and
75+
corsHeader.getEnclosingCallable() = allowCredentialsHeader.getEnclosingCallable() and
76+
sink.asExpr() = corsHeader.getArgument(1)
77+
)
78+
}
79+
}
80+
81+
from
82+
DataFlow::PathNode source, DataFlow::PathNode sink, CorsOriginConfig conf,
83+
CorsSourceReachesCheckConfig sanconf
84+
where conf.hasFlowPath(source, sink) and not sanconf.hasFlow(source.getNode(), _)
85+
select sink.getNode(), source, sink, "CORS header is being set using user controlled value $@.",
86+
source.getNode(), "user-provided value"

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
edges
2+
| UnvalidatedCors.java:21:22:21:48 | getHeader(...) : String | UnvalidatedCors.java:27:67:27:69 | url |
3+
nodes
4+
| UnvalidatedCors.java:21:22:21:48 | getHeader(...) : String | semmle.label | getHeader(...) : String |
5+
| UnvalidatedCors.java:27:67:27:69 | url | semmle.label | url |
6+
#select
7+
| UnvalidatedCors.java:27:67:27:69 | url | UnvalidatedCors.java:21:22:21:48 | getHeader(...) : String | UnvalidatedCors.java:27:67:27:69 | url | CORS header is being set using user controlled value $@. | UnvalidatedCors.java:21:22:21:48 | getHeader(...) | user-provided value |

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import java.io.IOException;
2+
3+
import javax.servlet.Filter;
4+
import javax.servlet.FilterChain;
5+
import javax.servlet.FilterConfig;
6+
import javax.servlet.ServletException;
7+
import javax.servlet.ServletRequest;
8+
import javax.servlet.ServletResponse;
9+
import javax.servlet.http.HttpServletRequest;
10+
import javax.servlet.http.HttpServletResponse;
11+
12+
import org.apache.commons.lang3.StringUtils;
13+
14+
public class UnvalidatedCors implements Filter {
15+
public void init(FilterConfig filterConfig) throws ServletException {}
16+
17+
public void doFilter(ServletRequest req, ServletResponse res,
18+
FilterChain chain) throws IOException, ServletException {
19+
HttpServletRequest request = (HttpServletRequest) req;
20+
HttpServletResponse response = (HttpServletResponse) res;
21+
String url = request.getHeader("Origin");
22+
23+
if (!StringUtils.isEmpty(url)) {
24+
String val = response.getHeader("Access-Control-Allow-Origin");
25+
26+
if (StringUtils.isEmpty(val)) {
27+
response.addHeader("Access-Control-Allow-Origin", url);
28+
response.addHeader("Access-Control-Allow-Credentials", "true");
29+
}
30+
}
31+
32+
chain.doFilter(req, res);
33+
}
34+
35+
public void destroy() {}
36+
}
37+

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-346/UnvalidatedCors.ql

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,4 +960,4 @@ public static String wrapIfMissing(final String str, final String wrapWith) {
960960
public StringUtils() {
961961
}
962962

963-
}
963+
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package javax.servlet;
2+
3+
import java.io.IOException;
4+
5+
public interface Filter {
6+
default public void init(FilterConfig filterConfig) throws ServletException {}
7+
public void doFilter(ServletRequest request, ServletResponse response,
8+
FilterChain chain)
9+
throws IOException, ServletException;
10+
default public void destroy() {}
11+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package javax.servlet;
2+
3+
import java.io.IOException;
4+
5+
public interface FilterChain {
6+
public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException;
7+
}

0 commit comments

Comments
 (0)