Skip to content

Commit 2525cfd

Browse files
author
Porcupiney Hairs
committed
include suggestions from review.
1 parent 38de9b6 commit 2525cfd

File tree

13 files changed

+196
-179
lines changed

13 files changed

+196
-179
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import java
22
import semmle.code.java.dataflow.FlowSources
3-
import semmle.code.java.frameworks.javase.URI
4-
import semmle.code.java.frameworks.javase.URL
53
import semmle.code.java.frameworks.javase.Http
64
import semmle.code.java.dataflow.DataFlow
75

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
import java
44
import semmle.code.java.frameworks.Networking
5-
import semmle.code.java.frameworks.javase.URI
6-
import semmle.code.java.frameworks.javase.URL
5+
import semmle.code.java.frameworks.ApacheHttp
6+
import semmle.code.java.frameworks.spring.Spring
77
import semmle.code.java.frameworks.JaxWS
88
import semmle.code.java.frameworks.javase.Http
99
import semmle.code.java.dataflow.DataFlow
@@ -34,8 +34,8 @@ module RequestForgery {
3434
*/
3535
private class ApacheSetUri extends Sink {
3636
ApacheSetUri() {
37-
exists(MethodAccess ma |
38-
ma.getReceiverType() instanceof TypeApacheHttpRequest and
37+
exists(MethodAccess ma, TypeApacheHttpRequestBase t |
38+
ma.getReceiverType().extendsOrImplements(t) and
3939
ma.getMethod().hasName("setURI")
4040
|
4141
this.asExpr() = ma.getArgument(0)
@@ -49,7 +49,9 @@ module RequestForgery {
4949
*/
5050
private class ApacheHttpRequestInstantiation extends Sink {
5151
ApacheHttpRequestInstantiation() {
52-
exists(ClassInstanceExpr c | c.getConstructedType() instanceof TypeApacheHttpRequest |
52+
exists(ClassInstanceExpr c, TypeApacheHttpRequestBase t |
53+
c.getConstructedType().extendsOrImplements(t)
54+
|
5355
this.asExpr() = c.getArgument(0)
5456
)
5557
}
@@ -115,8 +117,7 @@ module RequestForgery {
115117
*/
116118
private class JaxRsClientTarget extends Sink {
117119
JaxRsClientTarget() {
118-
exists(MethodAccess ma, JaxRsClient t |
119-
// ma.getMethod().getDeclaringType().getQualifiedName() ="javax.ws.rs.client.Client" and
120+
exists(MethodAccess ma |
120121
ma.getMethod().getDeclaringType() instanceof JaxRsClient and
121122
ma.getMethod().hasName("target")
122123
|
@@ -131,7 +132,12 @@ module RequestForgery {
131132
*/
132133
private class RequestEntityUriArg extends Sink {
133134
RequestEntityUriArg() {
134-
exists(SpringRequestEntityInstanceExpr e | e.getUriArg() = this.asExpr())
135+
exists(ClassInstanceExpr e, Argument a |
136+
e.getConstructedType() instanceof SpringRequestEntity and
137+
e.getAnArgument() = a and
138+
a.getType() instanceof TypeUri and
139+
this.asExpr() = a
140+
)
135141
}
136142
}
137143
}

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,6 @@ class TypeApacheHttpRequestBase extends RefType {
2727
}
2828
}
2929

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-
3930
/* A class representing the `RequestBuilder` class of the Apache Http Client library */
4031
class TypeApacheHttpRequestBuilder extends Class {
4132
TypeApacheHttpRequestBuilder() {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ class JaxRsResponseBuilder extends Class {
171171
}
172172

173173
/**
174-
* The class `javax.ws.rs.client.Client`
174+
* The class `javax.ws.rs.client.Client`.
175175
*/
176176
class JaxRsClient extends RefType {
177177
JaxRsClient() { this.hasQualifiedName("javax.ws.rs.client", "Client") }

java/ql/src/semmle/code/java/frameworks/Networking.qll

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import semmle.code.java.Type
66

7+
// import semmle.code.java.dataflow.FlowSources
78
/** The type `java.net.URLConnection`. */
89
class TypeUrlConnection extends RefType {
910
TypeUrlConnection() { hasQualifiedName("java.net", "URLConnection") }
@@ -41,3 +42,88 @@ class SocketGetInputStreamMethod extends Method {
4142
hasNoParameters()
4243
}
4344
}
45+
46+
/** Any expresion or call which returns a new URI. */
47+
abstract class UriCreation extends Top {
48+
/**
49+
* Returns the host of the newly created URI.
50+
* In the case where the host is specified separately, this returns only the host.
51+
* In the case where the uri is parsed from an input string,
52+
* such as in `URI(`http://foo.com/mypath')`,
53+
* this returns the entire argument passed i.e. `http://foo.com/mypath'.
54+
*/
55+
abstract Expr hostArg();
56+
}
57+
58+
/** An URI constructor expression */
59+
class UriConstructor extends ClassInstanceExpr, UriCreation {
60+
UriConstructor() { this.getConstructor().getDeclaringType().getQualifiedName() = "java.net.URI" }
61+
62+
override Expr hostArg() {
63+
// URI​(String str)
64+
result = this.getArgument(0) and this.getNumArgument() = 1
65+
or
66+
// URI(String scheme, String ssp, String fragment)
67+
// URI​(String scheme, String host, String path, String fragment)
68+
// URI​(String scheme, String authority, String path, String query, String fragment)
69+
result = this.getArgument(1) and this.getNumArgument() = [3, 4, 5]
70+
or
71+
// URI​(String scheme, String userInfo, String host, int port, String path, String query,
72+
// String fragment)
73+
result = this.getArgument(2) and this.getNumArgument() = 7
74+
}
75+
}
76+
77+
class UriCreate extends Call, UriCreation {
78+
UriCreate() {
79+
this.getCallee().getName() = "create" and
80+
this.getCallee().getDeclaringType() instanceof TypeUri
81+
}
82+
83+
override Expr hostArg() { result = this.getArgument(0) }
84+
}
85+
86+
/* An URL constructor expression */
87+
class UrlConstructor extends ClassInstanceExpr {
88+
UrlConstructor() { this.getConstructor().getDeclaringType().getQualifiedName() = "java.net.URL" }
89+
90+
Expr hostArg() {
91+
// URL(String spec)
92+
this.getNumArgument() = 1 and result = this.getArgument(0)
93+
or
94+
// URL(String protocol, String host, int port, String file)
95+
// URL(String protocol, String host, int port, String file, URLStreamHandler handler)
96+
this.getNumArgument() = [4, 5] and result = this.getArgument(1)
97+
or
98+
// URL(String protocol, String host, String file)
99+
// but not
100+
// URL(URL context, String spec, URLStreamHandler handler)
101+
(
102+
this.getNumArgument() = 3 and
103+
this.getConstructor().getParameter(2).getType() instanceof TypeString
104+
) and
105+
result = this.getArgument(1)
106+
}
107+
108+
Expr protocolArg() {
109+
// In all cases except where the first parameter is a URL, the argument
110+
// containing the protocol is the first one, otherwise it is the second.
111+
if this.getConstructor().getParameter(0).getType().getName() = "URL"
112+
then result = this.getArgument(1)
113+
else result = this.getArgument(0)
114+
}
115+
}
116+
117+
class UrlOpenStreamMethod extends Method {
118+
UrlOpenStreamMethod() {
119+
this.getDeclaringType() instanceof TypeUrl and
120+
this.getName() = "openStream"
121+
}
122+
}
123+
124+
class UrlOpenConnectionMethod extends Method {
125+
UrlOpenConnectionMethod() {
126+
this.getDeclaringType() instanceof TypeUrl and
127+
this.getName() = "openConnection"
128+
}
129+
}

java/ql/src/semmle/code/java/frameworks/javase/Http.qll

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
/**
2+
* Provides classes for identifying methods called by the Java net Http package.
3+
*/
4+
15
import java
2-
import semmle.code.java.dataflow.FlowSources
36

4-
/** A class representing `HttpRequest.Builder`. */
7+
/** The interface representing `HttpRequest.Builder`. */
58
class TypeHttpRequestBuilder extends Interface {
69
TypeHttpRequestBuilder() { hasQualifiedName("java.net.http", "HttpRequest$Builder") }
710
}
@@ -11,7 +14,7 @@ class TypeHttpRequest extends Interface {
1114
TypeHttpRequest() { hasQualifiedName("java.net.http", "HttpRequest") }
1215
}
1316

14-
/** A class representing `java.net.http.HttpRequest$Builder`'s `uri` method. */
17+
/** The `uri` method on `java.net.http.HttpRequest.Builder`. */
1518
class HttpBuilderUri extends Method {
1619
HttpBuilderUri() {
1720
this.getDeclaringType() instanceof TypeHttpRequestBuilder and

java/ql/src/semmle/code/java/frameworks/javase/URI.qll

Lines changed: 0 additions & 43 deletions
This file was deleted.

java/ql/src/semmle/code/java/frameworks/javase/URL.qll

Lines changed: 0 additions & 47 deletions
This file was deleted.

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,3 @@ class SpringResponseEntityBodyBuilder extends Interface {
3939
class SpringHttpHeaders extends Class {
4040
SpringHttpHeaders() { this.hasQualifiedName("org.springframework.http", "HttpHeaders") }
4141
}
42-
43-
/** Models `org.springframework.http.RequestEntity`s instantiation expressions. */
44-
class SpringRequestEntityInstanceExpr extends ClassInstanceExpr {
45-
int numArgs;
46-
47-
SpringRequestEntityInstanceExpr() {
48-
this.getConstructedType() instanceof SpringRequestEntity and
49-
numArgs = this.getNumArgument()
50-
}
51-
52-
Argument getUriArg() {
53-
exists(Argument a | this.getAnArgument() = a and a.getType() instanceof TypeUri | result = a)
54-
}
55-
}

java/ql/src/semmle/code/java/frameworks/spring/SpringWebClient.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ class SpringWebClient extends Interface {
3333
* which take an URL as an argument.
3434
*/
3535
abstract class SpringRestTemplateUrlMethods extends Method {
36-
/** Gets the argument which corresponds to a URL */
36+
/**
37+
* Gets the argument which corresponds to a URL argument
38+
* passed as a `java.net.URL` object or as a string or the like
39+
*/
3740
abstract Argument getUrlArgument(MethodAccess ma);
3841
}
3942

0 commit comments

Comments
 (0)