Skip to content

Commit 38de9b6

Browse files
author
Porcupiney Hairs
committed
add request forgery query
1 parent 1e048d8 commit 38de9b6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+1771
-26
lines changed

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

Lines changed: 1 addition & 12 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.javase.URL
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

‎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: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Server Sider Request Forgery (SSRF) from remote source
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::RequestForgery
16+
import DataFlow::PathGraph
17+
18+
from DataFlow::PathNode source, DataFlow::PathNode sink, RequestForgeryRemoteConfiguration conf
19+
where conf.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink, "Potential server side request forgery due to $@.",
21+
source.getNode(), "a user-provided value"

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
import semmle.code.java.frameworks.javase.URI
4+
import semmle.code.java.frameworks.javase.URL
5+
import semmle.code.java.frameworks.javase.Http
6+
import semmle.code.java.dataflow.DataFlow
7+
8+
module RequestForgery {
9+
import RequestForgeryCustomizations::RequestForgery
10+
11+
/**
12+
* A taint-tracking configuration for reasoning about request forgery.
13+
*/
14+
class RequestForgeryRemoteConfiguration extends TaintTracking::Configuration {
15+
RequestForgeryRemoteConfiguration() { this = "Server Side Request Forgery" }
16+
17+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
18+
19+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
20+
21+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
22+
additionalStep(pred, succ)
23+
}
24+
}
25+
}
26+
27+
predicate additionalStep(DataFlow::Node pred, DataFlow::Node succ) {
28+
// propagate to a URI when its host is assigned to
29+
exists(UriConstructor c | c.hostArg() = pred.asExpr() | succ.asExpr() = c)
30+
or
31+
// propagate to a URL when its host is assigned to
32+
exists(UrlConstructor c | c.hostArg() = pred.asExpr() | succ.asExpr() = c)
33+
or
34+
// propagate to a RequestEntity when its url is assigned to
35+
exists(MethodAccess m |
36+
m.getMethod().getDeclaringType() instanceof SpringRequestEntity and
37+
(
38+
m.getMethod().hasName(["get", "post", "head", "delete", "options", "patch", "put"]) and
39+
m.getArgument(0) = pred.asExpr() and
40+
m = succ.asExpr()
41+
)
42+
or
43+
m.getMethod().hasName("method") and
44+
m.getArgument(1) = pred.asExpr() and
45+
m = succ.asExpr()
46+
)
47+
or
48+
// propagate from a `RequestEntity<>$BodyBuilder` to a `RequestEntity`
49+
// when the builder is tainted
50+
exists(MethodAccess m, RefType t |
51+
m.getMethod().getDeclaringType() = t and
52+
t.hasQualifiedName("org.springframework.http", "RequestEntity<>$BodyBuilder") and
53+
m.getMethod().hasName("body") and
54+
m.getQualifier() = pred.asExpr() and
55+
m = succ.asExpr()
56+
)
57+
}

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/** A module to reason about request forgery vulnerabilities. */
2+
3+
import java
4+
import semmle.code.java.frameworks.Networking
5+
import semmle.code.java.frameworks.javase.URI
6+
import semmle.code.java.frameworks.javase.URL
7+
import semmle.code.java.frameworks.JaxWS
8+
import semmle.code.java.frameworks.javase.Http
9+
import semmle.code.java.dataflow.DataFlow
10+
11+
/** A module to reason about request forgery vulnerabilities. */
12+
module RequestForgery {
13+
/** A data flow sink for request forgery vulnerabilities. */
14+
abstract class Sink extends DataFlow::Node { }
15+
16+
/**
17+
* An argument to an url `openConnection` or `openStream` call
18+
* taken as a sink for request forgery vulnerabilities.
19+
*/
20+
private class UrlOpen extends Sink {
21+
UrlOpen() {
22+
exists(MethodAccess ma |
23+
ma.getMethod() instanceof UrlOpenConnectionMethod or
24+
ma.getMethod() instanceof UrlOpenStreamMethod
25+
|
26+
this.asExpr() = ma.getQualifier()
27+
)
28+
}
29+
}
30+
31+
/**
32+
* An argument to an Apache `setURI` call taken as a
33+
* sink for request forgery vulnerabilities.
34+
*/
35+
private class ApacheSetUri extends Sink {
36+
ApacheSetUri() {
37+
exists(MethodAccess ma |
38+
ma.getReceiverType() instanceof TypeApacheHttpRequest and
39+
ma.getMethod().hasName("setURI")
40+
|
41+
this.asExpr() = ma.getArgument(0)
42+
)
43+
}
44+
}
45+
46+
/**
47+
* An argument to any Apache Request Instantiation call taken as a
48+
* sink for request forgery vulnerabilities.
49+
*/
50+
private class ApacheHttpRequestInstantiation extends Sink {
51+
ApacheHttpRequestInstantiation() {
52+
exists(ClassInstanceExpr c | c.getConstructedType() instanceof TypeApacheHttpRequest |
53+
this.asExpr() = c.getArgument(0)
54+
)
55+
}
56+
}
57+
58+
/**
59+
* An argument to a Apache RequestBuilder method call taken as a
60+
* sink for request forgery vulnerabilities.
61+
*/
62+
private class ApacheHttpRequestBuilderArgument extends Sink {
63+
ApacheHttpRequestBuilderArgument() {
64+
exists(MethodAccess ma |
65+
ma.getReceiverType() instanceof TypeApacheHttpRequestBuilder and
66+
ma.getMethod().hasName(["setURI", "get", "post", "put", "optons", "head", "delete"])
67+
|
68+
this.asExpr() = ma.getArgument(0)
69+
)
70+
}
71+
}
72+
73+
/**
74+
* An argument to any Java.net.http.request Instantiation call taken as a
75+
* sink for request forgery vulnerabilities.
76+
*/
77+
private class HttpRequestNewBuilder extends Sink {
78+
HttpRequestNewBuilder() {
79+
exists(MethodAccess call |
80+
call.getCallee().hasName("newBuilder") and
81+
call.getMethod().getDeclaringType().getName() = "HttpRequest"
82+
|
83+
this.asExpr() = call.getArgument(0)
84+
)
85+
}
86+
}
87+
88+
/**
89+
* An argument to an Http Builder `uri` call taken as a
90+
* sink for request forgery vulnerabilities.
91+
*/
92+
private class HttpBuilderUriArgument extends Sink {
93+
HttpBuilderUriArgument() {
94+
exists(MethodAccess ma | ma.getMethod() instanceof HttpBuilderUri |
95+
this.asExpr() = ma.getArgument(0)
96+
)
97+
}
98+
}
99+
100+
/**
101+
* An argument to a Spring Rest Template method call taken as a
102+
* sink for request forgery vulnerabilities.
103+
*/
104+
private class SpringRestTemplateArgument extends Sink {
105+
SpringRestTemplateArgument() {
106+
exists(MethodAccess ma |
107+
this.asExpr() = ma.getMethod().(SpringRestTemplateUrlMethods).getUrlArgument(ma)
108+
)
109+
}
110+
}
111+
112+
/**
113+
* An argument to `javax.ws.rs.Client`s `target` method call taken as a
114+
* sink for request forgery vulnerabilities.
115+
*/
116+
private class JaxRsClientTarget extends Sink {
117+
JaxRsClientTarget() {
118+
exists(MethodAccess ma, JaxRsClient t |
119+
// ma.getMethod().getDeclaringType().getQualifiedName() ="javax.ws.rs.client.Client" and
120+
ma.getMethod().getDeclaringType() instanceof JaxRsClient and
121+
ma.getMethod().hasName("target")
122+
|
123+
this.asExpr() = ma.getArgument(0)
124+
)
125+
}
126+
}
127+
128+
/**
129+
* A URI argument to `org.springframework.http.RequestEntity`s constructor call
130+
* taken as a sink for request forgery vulnerabilities.
131+
*/
132+
private class RequestEntityUriArg extends Sink {
133+
RequestEntityUriArg() {
134+
exists(SpringRequestEntityInstanceExpr e | e.getUriArg() = this.asExpr())
135+
}
136+
}
137+
}

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

Lines changed: 2 additions & 14 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
*/
@@ -167,7 +155,7 @@ class HttpURLOpenMethod extends Method {
167155
/** Constructor of `ApacheHttpRequest` */
168156
predicate apacheHttpRequest(DataFlow::Node node1, DataFlow::Node node2) {
169157
exists(ConstructorCall cc |
170-
cc.getConstructedType() instanceof ApacheHttpRequest and
158+
cc.getConstructedType() instanceof TypeApacheHttpRequestBase and
171159
node2.asExpr() = cc and
172160
cc.getAnArgument() = node1.asExpr()
173161
)

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,32 @@ class ApacheHttpEntityGetContent extends Method {
1313
this.getName() = "getContent"
1414
}
1515
}
16+
17+
/**
18+
* A class derived from the `HttpRequestBase` or the `BasicHttpRequest`
19+
* class of the Apache Http Client `org.apache.http` library
20+
*/
21+
class TypeApacheHttpRequestBase extends RefType {
22+
TypeApacheHttpRequestBase() {
23+
this
24+
.getASourceSupertype*()
25+
.hasQualifiedName("org.apache.http.client.methods", "HttpRequestBase") or
26+
this.getASourceSupertype*().hasQualifiedName("org.apache.http.message", "BasicHttpRequest")
27+
}
28+
}
29+
30+
/*
31+
* Any class which can be used to make an HTTP request using the Apache Http Client library
32+
* Examples include `HttpGet`,`HttpPost` etc.
33+
*/
34+
35+
class TypeApacheHttpRequest extends Class {
36+
TypeApacheHttpRequest() { exists(TypeApacheHttpRequestBase t | this.extendsOrImplements(t)) }
37+
}
38+
39+
/* A class representing the `RequestBuilder` class of the Apache Http Client library */
40+
class TypeApacheHttpRequestBuilder extends Class {
41+
TypeApacheHttpRequestBuilder() {
42+
hasQualifiedName("org.apache.http.client.methods", "RequestBuilder")
43+
}
44+
}

‎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)