Skip to content

Commit 8b080a9

Browse files
smowtonsmowton
committed
Convert request forgery tests to inline expectations; add missing models revealed by this process.
1 parent b66dcbe commit 8b080a9

File tree

10 files changed

+84
-229
lines changed

10 files changed

+84
-229
lines changed

‎java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ private predicate summaryModelCsv(string row) {
252252
"javax.xml.transform.stream;StreamSource;false;getInputStream;;;Argument[-1];ReturnValue;taint",
253253
"java.nio;ByteBuffer;false;get;;;Argument[-1];ReturnValue;taint",
254254
"java.net;URI;false;toURL;;;Argument[-1];ReturnValue;taint",
255+
"java.net;URI;false;toString;;;Argument[-1];ReturnValue;taint",
256+
"java.net;URI;false;toAsciiString;;;Argument[-1];ReturnValue;taint",
255257
"java.io;File;false;toURI;;;Argument[-1];ReturnValue;taint",
256258
"java.io;File;false;toPath;;;Argument[-1];ReturnValue;taint",
257259
"java.nio.file;Path;false;toFile;;;Argument[-1];ReturnValue;taint",

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,10 @@ private class ApacheHttpFlowStep extends SummaryModelCsv {
261261
"org.apache.hc.core5.util;CharArrayBuffer;true;toString;();;Argument[-1];ReturnValue;taint",
262262
"org.apache.hc.core5.util;CharArrayBuffer;true;substring;(int,int);;Argument[-1];ReturnValue;taint",
263263
"org.apache.hc.core5.util;CharArrayBuffer;true;subSequence;(int,int);;Argument[-1];ReturnValue;taint",
264-
"org.apache.hc.core5.util;CharArrayBuffer;true;substringTrimmed;(int,int);;Argument[-1];ReturnValue;taint"
264+
"org.apache.hc.core5.util;CharArrayBuffer;true;substringTrimmed;(int,int);;Argument[-1];ReturnValue;taint",
265+
"org.apache.http.message;BasicRequestLine;false;BasicRequestLine;;;Argument[1];Argument[-1];taint",
266+
"org.apache.http;RequestLine;true;getUri;;;Argument[-1];ReturnValue;taint",
267+
"org.apache.http;RequestLine;true;toString;;;Argument[-1];ReturnValue;taint"
265268
]
266269
}
267270
}

‎java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ private class UrlOpenSink extends SinkModelCsv {
5353
"org.springframework.http;RequestEntity;false;put;;;Argument[0];open-url",
5454
"org.springframework.http;RequestEntity;false;method;;;Argument[1];open-url",
5555
"org.springframework.http;RequestEntity;false;RequestEntity;(HttpMethod,URI);;Argument[1];open-url",
56-
"org.springframework.http;RequestEntity;false;RequestEntity;(MultiValueMap,HttpMethod,URI);;Argument[2];open-url",
56+
"org.springframework.http;RequestEntity;false;RequestEntity;(MultiValueMap<String,String>,HttpMethod,URI);;Argument[2];open-url",
5757
"org.springframework.http;RequestEntity;false;RequestEntity;(T,HttpMethod,URI);;Argument[2];open-url",
5858
"org.springframework.http;RequestEntity;false;RequestEntity;(T,HttpMethod,URI,Type);;Argument[2];open-url",
59-
"org.springframework.http;RequestEntity;false;RequestEntity;(T,MultiValueMap,HttpMethod,URI);;Argument[3];open-url",
60-
"org.springframework.http;RequestEntity;false;RequestEntity;(T,MultiValueMap,HttpMethod,URI,Type);;Argument[3];open-url"
59+
"org.springframework.http;RequestEntity;false;RequestEntity;(T,MultiValueMap<String,String>,HttpMethod,URI);;Argument[3];open-url",
60+
"org.springframework.http;RequestEntity;false;RequestEntity;(T,MultiValueMap<String,String>,HttpMethod,URI,Type);;Argument[3];open-url"
6161
]
6262
}
6363
}

‎java/ql/test/query-tests/security/CWE-918/JaxWsSSRF.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
1919
throws ServletException, IOException {
2020
Client client = ClientBuilder.newClient();
2121
String url = request.getParameter("url");
22-
client.target(url);
22+
client.target(url); // $ SSRF
2323
}
2424

2525
}

‎java/ql/test/query-tests/security/CWE-918/RequestForgery.expected

Lines changed: 0 additions & 149 deletions
Large diffs are not rendered by default.

‎java/ql/test/query-tests/security/CWE-918/RequestForgery.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
1919
URI uri = new URI(request.getParameter("uri"));
2020
// BAD: a request parameter is incorporated without validation into a Http
2121
// request
22-
HttpRequest r = HttpRequest.newBuilder(uri).build();
22+
HttpRequest r = HttpRequest.newBuilder(uri).build(); // $ SSRF
2323
client.send(r, null);
2424

2525
// GOOD: sanitisation by concatenation with a prefix that prevents targeting an arbitrary host.
@@ -73,47 +73,47 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
7373
// BAD: cases where a string that would sanitise is used, but occurs in the wrong
7474
// place to sanitise user input:
7575
String unsafeUri3 = request.getParameter("baduri3") + "https://example.com/";
76-
HttpRequest unsafer3 = HttpRequest.newBuilder(new URI(unsafeUri3)).build();
76+
HttpRequest unsafer3 = HttpRequest.newBuilder(new URI(unsafeUri3)).build(); // $ SSRF
7777
client.send(unsafer3, null);
7878

7979
String unsafeUri4 = ("someprefix" + request.getParameter("baduri4")) + "https://example.com/";
80-
HttpRequest unsafer4 = HttpRequest.newBuilder(new URI(unsafeUri4)).build();
80+
HttpRequest unsafer4 = HttpRequest.newBuilder(new URI(unsafeUri4)).build(); // $ SSRF
8181
client.send(unsafer4, null);
8282

8383
StringBuilder unsafeUri5 = new StringBuilder();
8484
unsafeUri5.append(request.getParameter("baduri5")).append("https://example.com/");
85-
HttpRequest unsafer5 = HttpRequest.newBuilder(new URI(unsafeUri5.toString())).build();
85+
HttpRequest unsafer5 = HttpRequest.newBuilder(new URI(unsafeUri5.toString())).build(); // $ SSRF
8686
client.send(unsafer5, null);
8787

8888
StringBuilder unafeUri5a = new StringBuilder(request.getParameter("uri5a"));
8989
unafeUri5a.append("https://example.com/");
90-
HttpRequest unsafer5a = HttpRequest.newBuilder(new URI(unafeUri5a.toString())).build();
90+
HttpRequest unsafer5a = HttpRequest.newBuilder(new URI(unafeUri5a.toString())).build(); // $ SSRF
9191
client.send(unsafer5a, null);
9292

9393
StringBuilder unsafeUri5b = (new StringBuilder(request.getParameter("uri5b"))).append("dir/");
9494
unsafeUri5b.append("https://example.com/");
95-
HttpRequest unsafer5b = HttpRequest.newBuilder(new URI(unsafeUri5b.toString())).build();
95+
HttpRequest unsafer5b = HttpRequest.newBuilder(new URI(unsafeUri5b.toString())).build(); // $ SSRF
9696
client.send(unsafer5b, null);
9797

9898
StringBuilder unsafeUri5c = (new StringBuilder("https")).append(request.getParameter("uri5c"));
9999
unsafeUri5c.append("://example.com/dir/");
100-
HttpRequest unsafer5c = HttpRequest.newBuilder(new URI(unsafeUri5c.toString())).build();
100+
HttpRequest unsafer5c = HttpRequest.newBuilder(new URI(unsafeUri5c.toString())).build(); // $ SSRF
101101
client.send(unsafer5c, null);
102102

103103
String unsafeUri6 = String.format("%shttps://example.com/", request.getParameter("baduri6"));
104-
HttpRequest unsafer6 = HttpRequest.newBuilder(new URI(unsafeUri6)).build();
104+
HttpRequest unsafer6 = HttpRequest.newBuilder(new URI(unsafeUri6)).build(); // $ SSRF
105105
client.send(unsafer6, null);
106106

107107
String unsafeUri7 = String.format("%s/%s", request.getParameter("baduri7"), "https://example.com");
108-
HttpRequest unsafer7 = HttpRequest.newBuilder(new URI(unsafeUri7)).build();
108+
HttpRequest unsafer7 = HttpRequest.newBuilder(new URI(unsafeUri7)).build(); // $ SSRF
109109
client.send(unsafer7, null);
110110

111111
String unsafeUri8 = String.format("%s%s", request.getParameter("baduri8"), "https://example.com/");
112-
HttpRequest unsafer8 = HttpRequest.newBuilder(new URI(unsafeUri8)).build();
112+
HttpRequest unsafer8 = HttpRequest.newBuilder(new URI(unsafeUri8)).build(); // $ SSRF
113113
client.send(unsafer8, null);
114114

115115
String unsafeUri9 = request.getParameter("baduri9") + "/" + String.format("http://%s", "myserver.com");
116-
HttpRequest unsafer9 = HttpRequest.newBuilder(new URI(unsafeUri9)).build();
116+
HttpRequest unsafer9 = HttpRequest.newBuilder(new URI(unsafeUri9)).build(); // $ SSRF
117117
client.send(unsafer9, null);
118118

119119
} catch (Exception e) {

‎java/ql/test/query-tests/security/CWE-918/RequestForgery.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎java/ql/test/query-tests/security/CWE-918/RequestForgery2.java

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -63,47 +63,47 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
6363
// URL(URL context, String spec, URLStreamHandler handler)
6464
URL url6 = new URL(url3, "spec", new Helper2());
6565

66-
URLConnection c1 = url1.openConnection();
66+
URLConnection c1 = url1.openConnection(); // $ SSRF
6767
SocketAddress sa = new SocketAddress() {
6868
};
69-
URLConnection c2 = url1.openConnection(new Proxy(Type.HTTP, sa));
70-
InputStream c3 = url1.openStream();
69+
URLConnection c2 = url1.openConnection(new Proxy(Type.HTTP, sa)); // $ SSRF
70+
InputStream c3 = url1.openStream(); // $ SSRF
7171

7272
// java.net.http
7373
HttpClient client = HttpClient.newHttpClient();
74-
HttpRequest request2 = HttpRequest.newBuilder().uri(uri2).build();
75-
HttpRequest request3 = HttpRequest.newBuilder(uri).build();
74+
HttpRequest request2 = HttpRequest.newBuilder().uri(uri2).build(); // $ SSRF
75+
HttpRequest request3 = HttpRequest.newBuilder(uri).build(); // $ SSRF
7676

7777
// Apache HTTPlib
78-
HttpGet httpGet = new HttpGet(uri);
78+
HttpGet httpGet = new HttpGet(uri); // $ SSRF
7979
HttpGet httpGet2 = new HttpGet();
80-
httpGet2.setURI(uri2);
81-
82-
new HttpHead(uri);
83-
new HttpPost(uri);
84-
new HttpPut(uri);
85-
new HttpDelete(uri);
86-
new HttpOptions(uri);
87-
new HttpTrace(uri);
88-
new HttpPatch(uri);
89-
90-
new BasicHttpRequest(new BasicRequestLine("GET", uri2.toString(), null));
91-
new BasicHttpRequest("GET", uri2.toString());
92-
new BasicHttpRequest("GET", uri2.toString(), null);
93-
94-
new BasicHttpEntityEnclosingRequest(new BasicRequestLine("GET", uri2.toString(), null));
95-
new BasicHttpEntityEnclosingRequest("GET", uri2.toString());
96-
new BasicHttpEntityEnclosingRequest("GET", uri2.toString(), null);
97-
98-
RequestBuilder.get(uri2);
99-
RequestBuilder.post(uri2);
100-
RequestBuilder.put(uri2);
101-
RequestBuilder.delete(uri2);
102-
RequestBuilder.options(uri2);
103-
RequestBuilder.head(uri2);
104-
RequestBuilder.trace(uri2);
105-
RequestBuilder.patch(uri2);
106-
RequestBuilder.get("").setUri(uri2);
80+
httpGet2.setURI(uri2); // $ SSRF
81+
82+
new HttpHead(uri); // $ SSRF
83+
new HttpPost(uri); // $ SSRF
84+
new HttpPut(uri); // $ SSRF
85+
new HttpDelete(uri); // $ SSRF
86+
new HttpOptions(uri); // $ SSRF
87+
new HttpTrace(uri); // $ SSRF
88+
new HttpPatch(uri); // $ SSRF
89+
90+
new BasicHttpRequest(new BasicRequestLine("GET", uri2.toString(), null)); // $ SSRF
91+
new BasicHttpRequest("GET", uri2.toString()); // $ SSRF
92+
new BasicHttpRequest("GET", uri2.toString(), null); // $ SSRF
93+
94+
new BasicHttpEntityEnclosingRequest(new BasicRequestLine("GET", uri2.toString(), null)); // $ SSRF
95+
new BasicHttpEntityEnclosingRequest("GET", uri2.toString()); // $ SSRF
96+
new BasicHttpEntityEnclosingRequest("GET", uri2.toString(), null); // $ SSRF
97+
98+
RequestBuilder.get(uri2); // $ SSRF
99+
RequestBuilder.post(uri2); // $ SSRF
100+
RequestBuilder.put(uri2); // $ SSRF
101+
RequestBuilder.delete(uri2); // $ SSRF
102+
RequestBuilder.options(uri2); // $ SSRF
103+
RequestBuilder.head(uri2); // $ SSRF
104+
RequestBuilder.trace(uri2); // $ SSRF
105+
RequestBuilder.patch(uri2); // $ SSRF
106+
RequestBuilder.get("").setUri(uri2); // $ SSRF
107107

108108
} catch (Exception e) {
109109
// TODO: handle exception

‎java/ql/test/query-tests/security/CWE-918/SpringSSRF.java

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,69 +30,69 @@ protected void doGet(HttpServletRequest request2, HttpServletResponse response2)
3030
try {
3131
{
3232
ResponseEntity<String> response =
33-
restTemplate.getForEntity(fooResourceUrl + "/1", String.class);
33+
restTemplate.getForEntity(fooResourceUrl + "/1", String.class); // $ SSRF
3434
}
3535

3636
{
3737
ResponseEntity<String> response =
38-
restTemplate.exchange(fooResourceUrl, HttpMethod.POST, request, String.class);
38+
restTemplate.exchange(fooResourceUrl, HttpMethod.POST, request, String.class); // $ SSRF
3939
}
4040
{
4141
ResponseEntity<String> response =
42-
restTemplate.execute(fooResourceUrl, HttpMethod.POST, null, null, "test");
42+
restTemplate.execute(fooResourceUrl, HttpMethod.POST, null, null, "test"); // $ SSRF
4343
}
4444
{
4545
String response =
46-
restTemplate.getForObject(fooResourceUrl, String.class, "test");
46+
restTemplate.getForObject(fooResourceUrl, String.class, "test"); // $ SSRF
4747
}
4848
{
4949
String body = new String("body");
5050
URI uri = new URI(fooResourceUrl);
5151
RequestEntity<String> requestEntity =
52-
RequestEntity.post(uri).body(body);
52+
RequestEntity.post(uri).body(body); // $ SSRF
5353
ResponseEntity<String> response = restTemplate.exchange(requestEntity, String.class);
54-
RequestEntity.get(uri);
55-
RequestEntity.put(uri);
56-
RequestEntity.delete(uri);
57-
RequestEntity.options(uri);
58-
RequestEntity.patch(uri);
59-
RequestEntity.head(uri);
60-
RequestEntity.method(null, uri);
54+
RequestEntity.get(uri); // $ SSRF
55+
RequestEntity.put(uri); // $ SSRF
56+
RequestEntity.delete(uri); // $ SSRF
57+
RequestEntity.options(uri); // $ SSRF
58+
RequestEntity.patch(uri); // $ SSRF
59+
RequestEntity.head(uri); // $ SSRF
60+
RequestEntity.method(null, uri); // $ SSRF
6161
}
6262
{
63-
String response = restTemplate.patchForObject(fooResourceUrl, new String("object"),
63+
String response = restTemplate.patchForObject(fooResourceUrl, new String("object"), // $ SSRF
6464
String.class, "hi");
6565
}
6666
{
67-
ResponseEntity<String> response = restTemplate.postForEntity(new URI(fooResourceUrl),
67+
ResponseEntity<String> response = restTemplate.postForEntity(new URI(fooResourceUrl), // $ SSRF
6868
new String("object"), String.class);
6969
}
7070
{
71-
URI response = restTemplate.postForLocation(fooResourceUrl, new String("object"));
71+
URI response = restTemplate.postForLocation(fooResourceUrl, new String("object")); // $ SSRF
7272
}
7373
{
7474
String response =
75-
restTemplate.postForObject(fooResourceUrl, new String("object"), String.class);
75+
restTemplate.postForObject(fooResourceUrl, new String("object"), String.class); // $ SSRF
7676
}
7777
{
78-
restTemplate.put(fooResourceUrl, new String("object"));
78+
restTemplate.put(fooResourceUrl, new String("object")); // $ SSRF
7979
}
8080
{
8181
URI uri = new URI(fooResourceUrl);
8282
MultiValueMap<String, String> headers = null;
8383
java.lang.reflect.Type type = null;
84-
new RequestEntity<String>(null, uri);
85-
new RequestEntity<String>(headers, null, uri);
86-
new RequestEntity<String>("body", null, uri);
87-
new RequestEntity<String>("body", headers, null, uri);
88-
new RequestEntity<String>("body", null, uri, type);
89-
new RequestEntity<String>("body", headers, null, uri, type);
84+
new RequestEntity<String>(null, uri); // $ SSRF
85+
new RequestEntity<String>(headers, null, uri); // $ SSRF
86+
new RequestEntity<String>("body", null, uri); // $ SSRF
87+
new RequestEntity<String>("body", headers, null, uri); // $ SSRF
88+
new RequestEntity<String>("body", null, uri, type); // $ SSRF
89+
new RequestEntity<String>("body", headers, null, uri, type); // $ SSRF
9090
}
9191
{
9292
URI uri = new URI(fooResourceUrl);
93-
restTemplate.delete(uri);
94-
restTemplate.headForHeaders(uri);
95-
restTemplate.optionsForAllow(uri);
93+
restTemplate.delete(uri); // $ SSRF
94+
restTemplate.headForHeaders(uri); // $ SSRF
95+
restTemplate.optionsForAllow(uri); // $ SSRF
9696
}
9797
} catch (org.springframework.web.client.RestClientException | java.net.URISyntaxException e) {}
9898
}

‎java/ql/test/stubs/apache-http-4.4.13/org/apache/http/message/BasicHttpEntityEnclosingRequest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
* @author <a href="mailto:oleg at ural.ru">Oleg Kalnichevski</a>
4444
*
4545
* @version $Revision: 618017 $
46-
*
46+
*
4747
* @since 4.0
4848
*
4949
* @deprecated Please use {@link java.net.URL#openConnection} instead. Please
@@ -54,15 +54,15 @@
5454
@Deprecated
5555
public class BasicHttpEntityEnclosingRequest extends BasicHttpRequest implements HttpEntityEnclosingRequest {
5656
public BasicHttpEntityEnclosingRequest(final String method, final String uri) {
57-
super(method, uri);
57+
super(null);
5858
}
5959

6060
public BasicHttpEntityEnclosingRequest(final String method, final String uri, final ProtocolVersion ver) {
61-
super(method, uri, ver);
61+
super(null);
6262
}
6363

6464
public BasicHttpEntityEnclosingRequest(final RequestLine requestline) {
65-
super(requestline);
65+
super(null);
6666
}
6767

6868
public HttpEntity getEntity() {

0 commit comments

Comments
 (0)