Skip to content

Commit f70072a

Browse files
aschackmullaschackmull
authored
Merge pull request github#3454 from porcupineyhairs/javaSSRf
Java : add request forgery query
2 parents d29a6ec + 9c30b82 commit f70072a

39 files changed

+1559
-26
lines changed

‎java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import java
1313
import semmle.code.java.dataflow.TaintTracking
14+
import semmle.code.java.frameworks.Networking
1415
import DataFlow::PathGraph
1516

1617
class HTTPString extends StringLiteral {
@@ -29,18 +30,6 @@ class HTTPString extends StringLiteral {
2930
}
3031
}
3132

32-
class URLConstructor extends ClassInstanceExpr {
33-
URLConstructor() { this.getConstructor().getDeclaringType().getQualifiedName() = "java.net.___URL" }
34-
35-
Expr protocolArg() {
36-
// In all cases except where the first parameter is a URL, the argument
37-
// containing the protocol is the first one, otherwise it is the second.
38-
if this.getConstructor().getParameter(0).getType().getName() = "URL"
39-
then result = this.getArgument(1)
40-
else result = this.getArgument(0)
41-
}
42-
}
43-
4433
class URLOpenMethod extends Method {
4534
URLOpenMethod() {
4635
this.getDeclaringType().getQualifiedName() = "java.net.___URL" and
@@ -63,7 +52,7 @@ class HTTPStringToURLOpenMethodFlowConfig extends TaintTracking::Configuration {
6352
}
6453

6554
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
66-
exists(URLConstructor u |
55+
exists(UrlConstructorCall u |
6756
node1.asExpr() = u.protocolArg() and
6857
node2.asExpr() = u
6958
)

‎java/ql/src/experimental/CWE-918/RequestForgery.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import java.net.http.HttpClient;
2+
3+
public class SSRF extends HttpServlet {
4+
private static final String VALID_URI = "http://lgtm.com";
5+
private HttpClient client = HttpClient.newHttpClient();
6+
7+
protected void doGet(HttpServletRequest request, HttpServletResponse response)
8+
throws ServletException, IOException {
9+
URI uri = new URI(request.getParameter("uri"));
10+
// BAD: a request parameter is incorporated without validation into a Http request
11+
HttpRequest r = HttpRequest.newBuilder(uri).build();
12+
client.send(r, null);
13+
14+
// GOOD: the request parameter is validated against a known fixed string
15+
if (VALID_URI.equals(request.getParameter("uri"))) {
16+
HttpRequest r2 = HttpRequest.newBuilder(uri).build();
17+
client.send(r2, null);
18+
}
19+
}
20+
}

‎java/ql/src/experimental/CWE-918/RequestForgery.qhelp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
7+
<overview>
8+
<p>Directly incorporating user input into a HTTP request without validating the input
9+
can facilitate Server Side Request Forgery (SSRF) attacks. In these attacks, the server
10+
may be tricked into making a request and interacting with an attacker-controlled server.
11+
</p>
12+
13+
</overview>
14+
<recommendation>
15+
16+
<p>To guard against SSRF attacks, it is advisable to avoid putting user input
17+
directly into the request URL. Instead, maintain a list of authorized
18+
URLs on the server; then choose from that list based on the user input provided.</p>
19+
20+
</recommendation>
21+
<example>
22+
23+
<p>The following example shows an HTTP request parameter being used directly in a forming a
24+
new request without validating the input, which facilitates SSRF attacks.
25+
It also shows how to remedy the problem by validating the user input against a known fixed string.
26+
</p>
27+
28+
<sample src="RequestForgery.java" />
29+
30+
</example>
31+
<references>
32+
<li>
33+
<a href="https://owasp.org/www-community/attacks/Server_Side_Request_Forgery">OWASP SSRF</a>
34+
</li>
35+
36+
</references>
37+
</qhelp>

‎java/ql/src/experimental/CWE-918/RequestForgery.ql

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @name Server Side Request Forgery (SSRF)
3+
* @description Making web requests based on unvalidated user-input
4+
* may cause server to communicate with malicious servers.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/ssrf
9+
* @tags security
10+
* external/cwe/cwe-918
11+
*/
12+
13+
import java
14+
import semmle.code.java.dataflow.FlowSources
15+
import RequestForgery
16+
import DataFlow::PathGraph
17+
18+
class RequestForgeryConfiguration extends TaintTracking::Configuration {
19+
RequestForgeryConfiguration() { this = "Server Side Request Forgery" }
20+
21+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
22+
23+
override predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgerySink }
24+
25+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
26+
requestForgeryStep(pred, succ)
27+
}
28+
}
29+
30+
from DataFlow::PathNode source, DataFlow::PathNode sink, RequestForgeryConfiguration conf
31+
where conf.hasFlowPath(source, sink)
32+
select sink.getNode(), source, sink, "Potential server side request forgery due to $@.",
33+
source.getNode(), "a user-provided value"

‎java/ql/src/experimental/CWE-918/RequestForgery.qll

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
import java
2+
import semmle.code.java.frameworks.Networking
3+
import semmle.code.java.frameworks.ApacheHttp
4+
import semmle.code.java.frameworks.spring.Spring
5+
import semmle.code.java.frameworks.JaxWS
6+
import semmle.code.java.frameworks.javase.Http
7+
import semmle.code.java.dataflow.DataFlow
8+
9+
predicate requestForgeryStep(DataFlow::Node pred, DataFlow::Node succ) {
10+
// propagate to a URI when its host is assigned to
11+
exists(UriCreation c | c.getHostArg() = pred.asExpr() | succ.asExpr() = c)
12+
or
13+
// propagate to a URL when its host is assigned to
14+
exists(UrlConstructorCall c | c.getHostArg() = pred.asExpr() | succ.asExpr() = c)
15+
or
16+
// propagate to a RequestEntity when its url is assigned to
17+
exists(MethodAccess m |
18+
m.getMethod().getDeclaringType() instanceof SpringRequestEntity and
19+
(
20+
m.getMethod().hasName(["get", "post", "head", "delete", "options", "patch", "put"]) and
21+
m.getArgument(0) = pred.asExpr() and
22+
m = succ.asExpr()
23+
or
24+
m.getMethod().hasName("method") and
25+
m.getArgument(1) = pred.asExpr() and
26+
m = succ.asExpr()
27+
)
28+
)
29+
or
30+
// propagate from a `RequestEntity<>$BodyBuilder` to a `RequestEntity`
31+
// when the builder is tainted
32+
exists(MethodAccess m, RefType t |
33+
m.getMethod().getDeclaringType() = t and
34+
t.hasQualifiedName("org.springframework.http", "RequestEntity<>$BodyBuilder") and
35+
m.getMethod().hasName("body") and
36+
m.getQualifier() = pred.asExpr() and
37+
m = succ.asExpr()
38+
)
39+
}
40+
41+
/** A data flow sink for request forgery vulnerabilities. */
42+
abstract class RequestForgerySink extends DataFlow::Node { }
43+
44+
/**
45+
* An argument to an url `openConnection` or `openStream` call
46+
* taken as a sink for request forgery vulnerabilities.
47+
*/
48+
private class UrlOpen extends RequestForgerySink {
49+
UrlOpen() {
50+
exists(MethodAccess ma |
51+
ma.getMethod() instanceof UrlOpenConnectionMethod or
52+
ma.getMethod() instanceof UrlOpenStreamMethod
53+
|
54+
this.asExpr() = ma.getQualifier()
55+
)
56+
}
57+
}
58+
59+
/**
60+
* An argument to an Apache `setURI` call taken as a
61+
* sink for request forgery vulnerabilities.
62+
*/
63+
private class ApacheSetUri extends RequestForgerySink {
64+
ApacheSetUri() {
65+
exists(MethodAccess ma |
66+
ma.getReceiverType() instanceof ApacheHttpRequest and
67+
ma.getMethod().hasName("setURI")
68+
|
69+
this.asExpr() = ma.getArgument(0)
70+
)
71+
}
72+
}
73+
74+
/**
75+
* An argument to any Apache Request Instantiation call taken as a
76+
* sink for request forgery vulnerabilities.
77+
*/
78+
private class ApacheHttpRequestInstantiation extends RequestForgerySink {
79+
ApacheHttpRequestInstantiation() {
80+
exists(ClassInstanceExpr c | c.getConstructedType() instanceof ApacheHttpRequest |
81+
this.asExpr() = c.getArgument(0)
82+
)
83+
}
84+
}
85+
86+
/**
87+
* An argument to a Apache RequestBuilder method call taken as a
88+
* sink for request forgery vulnerabilities.
89+
*/
90+
private class ApacheHttpRequestBuilderArgument extends RequestForgerySink {
91+
ApacheHttpRequestBuilderArgument() {
92+
exists(MethodAccess ma |
93+
ma.getReceiverType() instanceof TypeApacheHttpRequestBuilder and
94+
ma.getMethod().hasName(["setURI", "get", "post", "put", "optons", "head", "delete"])
95+
|
96+
this.asExpr() = ma.getArgument(0)
97+
)
98+
}
99+
}
100+
101+
/**
102+
* An argument to any Java.net.http.request Instantiation call taken as a
103+
* sink for request forgery vulnerabilities.
104+
*/
105+
private class HttpRequestNewBuilder extends RequestForgerySink {
106+
HttpRequestNewBuilder() {
107+
exists(MethodAccess call |
108+
call.getCallee().hasName("newBuilder") and
109+
call.getMethod().getDeclaringType().getName() = "HttpRequest"
110+
|
111+
this.asExpr() = call.getArgument(0)
112+
)
113+
}
114+
}
115+
116+
/**
117+
* An argument to an Http Builder `uri` call taken as a
118+
* sink for request forgery vulnerabilities.
119+
*/
120+
private class HttpBuilderUriArgument extends RequestForgerySink {
121+
HttpBuilderUriArgument() {
122+
exists(MethodAccess ma | ma.getMethod() instanceof HttpBuilderUri |
123+
this.asExpr() = ma.getArgument(0)
124+
)
125+
}
126+
}
127+
128+
/**
129+
* An argument to a Spring Rest Template method call taken as a
130+
* sink for request forgery vulnerabilities.
131+
*/
132+
private class SpringRestTemplateArgument extends RequestForgerySink {
133+
SpringRestTemplateArgument() {
134+
exists(MethodAccess ma |
135+
this.asExpr() = ma.getMethod().(SpringRestTemplateUrlMethods).getUrlArgument(ma)
136+
)
137+
}
138+
}
139+
140+
/**
141+
* An argument to `javax.ws.rs.Client`s `target` method call taken as a
142+
* sink for request forgery vulnerabilities.
143+
*/
144+
private class JaxRsClientTarget extends RequestForgerySink {
145+
JaxRsClientTarget() {
146+
exists(MethodAccess ma |
147+
ma.getMethod().getDeclaringType() instanceof JaxRsClient and
148+
ma.getMethod().hasName("target")
149+
|
150+
this.asExpr() = ma.getArgument(0)
151+
)
152+
}
153+
}
154+
155+
/**
156+
* An argument to `org.springframework.http.RequestEntity`s constructor call
157+
* which is an URI taken as a sink for request forgery vulnerabilities.
158+
*/
159+
private class RequestEntityUriArg extends RequestForgerySink {
160+
RequestEntityUriArg() {
161+
exists(ClassInstanceExpr e, Argument a |
162+
e.getConstructedType() instanceof SpringRequestEntity and
163+
e.getAnArgument() = a and
164+
a.getType() instanceof TypeUri and
165+
this.asExpr() = a
166+
)
167+
}
168+
}
169+
170+
/**
171+
* A class representing all Spring Rest Template methods
172+
* which take an URL as an argument.
173+
*/
174+
private class SpringRestTemplateUrlMethods extends Method {
175+
SpringRestTemplateUrlMethods() {
176+
this.getDeclaringType() instanceof SpringRestTemplate and
177+
this
178+
.hasName([
179+
"doExecute", "postForEntity", "postForLocation", "postForObject", "put", "exchange",
180+
"execute", "getForEntity", "getForObject", "patchForObject"
181+
])
182+
}
183+
184+
/**
185+
* Gets the argument which corresponds to a URL argument
186+
* passed as a `java.net.___URL` object or as a string or the like
187+
*/
188+
Argument getUrlArgument(MethodAccess ma) {
189+
// doExecute(URI url, HttpMethod method, RequestCallback requestCallback,
190+
// ResponseExtractor<T> responseExtractor)
191+
result = ma.getArgument(0)
192+
}
193+
}

‎java/ql/src/experimental/Security/CWE/CWE-522/InsecureBasicAuth.ql

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import java
1212
import semmle.code.java.frameworks.Networking
13+
import semmle.code.java.frameworks.ApacheHttp
1314
import semmle.code.java.dataflow.TaintTracking
1415
import DataFlow::PathGraph
1516

@@ -21,19 +22,6 @@ private string getPrivateHostRegex() {
2122
"(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?"
2223
}
2324

24-
/**
25-
* The Java class `org.apache.http.client.methods.HttpRequestBase`. Popular subclasses include `HttpGet`, `HttpPost`, and `HttpPut`.
26-
* And the Java class `org.apache.http.message.BasicHttpRequest`.
27-
*/
28-
class ApacheHttpRequest extends RefType {
29-
ApacheHttpRequest() {
30-
this
31-
.getASourceSupertype*()
32-
.hasQualifiedName("org.apache.http.client.methods", "HttpRequestBase") or
33-
this.getASourceSupertype*().hasQualifiedName("org.apache.http.message", "BasicHttpRequest")
34-
}
35-
}
36-
3725
/**
3826
* Class of Java URL constructor.
3927
*/

‎java/ql/src/semmle/code/java/frameworks/ApacheHttp.qll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
/**
2+
* Provides classes and predicates related to `org.apache.http.*`.
3+
*/
4+
15
import java
26

37
class ApacheHttpGetParams extends Method {
@@ -13,3 +17,26 @@ class ApacheHttpEntityGetContent extends Method {
1317
this.getName() = "getContent"
1418
}
1519
}
20+
21+
/**
22+
* An HTTP request as represented by the Apache HTTP Client library. This is
23+
* either `org.apache.http.client.methods.HttpRequestBase`,
24+
* `org.apache.http.message.BasicHttpRequest`, or one of their subclasses.
25+
*/
26+
class ApacheHttpRequest extends RefType {
27+
ApacheHttpRequest() {
28+
this
29+
.getASourceSupertype*()
30+
.hasQualifiedName("org.apache.http.client.methods", "HttpRequestBase") or
31+
this.getASourceSupertype*().hasQualifiedName("org.apache.http.message", "BasicHttpRequest")
32+
}
33+
}
34+
35+
/**
36+
* The `org.apache.http.client.methods.RequestBuilder` class.
37+
*/
38+
class TypeApacheHttpRequestBuilder extends Class {
39+
TypeApacheHttpRequestBuilder() {
40+
this.hasQualifiedName("org.apache.http.client.methods", "RequestBuilder")
41+
}
42+
}

‎java/ql/src/semmle/code/java/frameworks/JaxWS.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,13 @@ class JaxRsResponseBuilder extends Class {
170170
JaxRsResponseBuilder() { this.hasQualifiedName("javax.ws.rs.core", "ResponseBuilder") }
171171
}
172172

173+
/**
174+
* The class `javax.ws.rs.client.Client`.
175+
*/
176+
class JaxRsClient extends RefType {
177+
JaxRsClient() { this.hasQualifiedName("javax.ws.rs.client", "Client") }
178+
}
179+
173180
/**
174181
* A constructor that may be called by a JaxRS container to construct an instance to inject into a
175182
* resource method or resource class constructor.

0 commit comments

Comments
 (0)