Skip to content

Commit 4b25532

Browse files
author
Porcupiney Hairs
committed
include suggestions from review.
1 parent eb6d611 commit 4b25532

File tree

17 files changed

+40
-48
lines changed

17 files changed

+40
-48
lines changed

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

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

1212
import java
1313
import semmle.code.java.dataflow.TaintTracking
14-
import semmle.code.java.frameworks.javase.URL
14+
import semmle.code.java.frameworks.Networking
1515
import DataFlow::PathGraph
1616

1717
class HTTPString extends StringLiteral {
@@ -52,7 +52,7 @@ class HTTPStringToURLOpenMethodFlowConfig extends TaintTracking::Configuration {
5252
}
5353

5454
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
55-
exists(URLConstructor u |
55+
exists(UrlConstructor u |
5656
node1.asExpr() = u.protocolArg() and
5757
node2.asExpr() = u
5858
)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ module RequestForgery {
2424

2525
predicate additionalStep(DataFlow::Node pred, DataFlow::Node succ) {
2626
// propagate to a URI when its host is assigned to
27-
exists(UriConstructor c | c.hostArg() = pred.asExpr() | succ.asExpr() = c)
27+
exists(UriCreation c | c.getHostArg() = pred.asExpr() | succ.asExpr() = c)
2828
or
2929
// propagate to a URL when its host is assigned to
30-
exists(UrlConstructor c | c.hostArg() = pred.asExpr() | succ.asExpr() = c)
30+
exists(UrlConstructor c | c.getHostArg() = pred.asExpr() | succ.asExpr() = c)
3131
or
3232
// propagate to a RequestEntity when its url is assigned to
3333
exists(MethodAccess m |

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

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ module RequestForgery {
3434
*/
3535
private class ApacheSetUri extends Sink {
3636
ApacheSetUri() {
37-
exists(MethodAccess ma, TypeApacheHttpRequestBase t |
38-
ma.getReceiverType().extendsOrImplements(t) and
37+
exists(MethodAccess ma |
38+
ma.getReceiverType() instanceof ApacheHttpRequest and
3939
ma.getMethod().hasName("setURI")
4040
|
4141
this.asExpr() = ma.getArgument(0)
@@ -49,9 +49,7 @@ module RequestForgery {
4949
*/
5050
private class ApacheHttpRequestInstantiation extends Sink {
5151
ApacheHttpRequestInstantiation() {
52-
exists(ClassInstanceExpr c, TypeApacheHttpRequestBase t |
53-
c.getConstructedType().extendsOrImplements(t)
54-
|
52+
exists(ClassInstanceExpr c | c.getConstructedType() instanceof ApacheHttpRequest |
5553
this.asExpr() = c.getArgument(0)
5654
)
5755
}
@@ -149,25 +147,9 @@ module RequestForgery {
149147
class SpringRestTemplateUrlMethods extends Method {
150148
SpringRestTemplateUrlMethods() {
151149
this.getDeclaringType() instanceof SpringRestTemplate and
152-
this.hasName("doExecute")
153-
or
154-
this.hasName("postForEntity")
155-
or
156-
this.hasName("postForLocation")
157-
or
158-
this.hasName("postForObject")
159-
or
160-
this.hasName("put")
161-
or
162-
this.hasName("exchange")
163-
or
164-
this.hasName("execute")
165-
or
166-
this.hasName("getForEntity")
167-
or
168-
this.hasName("getForObject")
169-
or
170-
this.hasName("patchForObject")
150+
this
151+
.hasName(["doExecute", "postForEntity", "postForLocation", "postForObject", "put",
152+
"exchange", "execute", "getForEntity", "getForObject", "patchForObject"])
171153
}
172154

173155
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class HttpURLOpenMethod extends Method {
155155
/** Constructor of `ApacheHttpRequest` */
156156
predicate apacheHttpRequest(DataFlow::Node node1, DataFlow::Node node2) {
157157
exists(ConstructorCall cc |
158-
cc.getConstructedType() instanceof TypeApacheHttpRequestBase and
158+
cc.getConstructedType() instanceof ApacheHttpRequest and
159159
node2.asExpr() = cc and
160160
cc.getAnArgument() = node1.asExpr()
161161
)

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,19 @@ class ApacheHttpEntityGetContent extends Method {
1515
}
1616

1717
/**
18-
* A class derived from the `HttpRequestBase` or the `BasicHttpRequest`
18+
* Models any class derived from `HttpRequestBase` or the `BasicHttpRequest`
1919
* class of the Apache Http Client `org.apache.http` library
2020
*/
21-
class TypeApacheHttpRequestBase extends RefType {
22-
TypeApacheHttpRequestBase() {
21+
class ApacheHttpRequest extends RefType {
22+
ApacheHttpRequest() {
2323
this
2424
.getASourceSupertype*()
2525
.hasQualifiedName("org.apache.http.client.methods", "HttpRequestBase") or
2626
this.getASourceSupertype*().hasQualifiedName("org.apache.http.message", "BasicHttpRequest")
2727
}
2828
}
2929

30-
/* A class representing the `RequestBuilder` class of the Apache Http Client library */
30+
/** Models `RequestBuilder` class of the Apache Http Client library */
3131
class TypeApacheHttpRequestBuilder extends Class {
3232
TypeApacheHttpRequestBuilder() {
3333
hasQualifiedName("org.apache.http.client.methods", "RequestBuilder")

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,22 +43,27 @@ class SocketGetInputStreamMethod extends Method {
4343
}
4444

4545
/** Any expresion or call which returns a new URI. */
46-
abstract class UriCreation extends Top {
46+
class UriCreation extends Call {
47+
UriCreation() {
48+
this.getCallee().getDeclaringType() instanceof TypeUri and
49+
(this instanceof ClassInstanceExpr or this.getCallee().hasName("create"))
50+
}
51+
4752
/**
4853
* Returns the host of the newly created URI.
4954
* In the case where the host is specified separately, this returns only the host.
5055
* In the case where the uri is parsed from an input string,
5156
* such as in `URI(`http://foo.com/mypath')`,
5257
* this returns the entire argument passed i.e. `http://foo.com/mypath'.
5358
*/
54-
abstract Expr hostArg();
59+
Expr getHostArg() { none() }
5560
}
5661

5762
/** An URI constructor expression */
5863
class UriConstructor extends ClassInstanceExpr, UriCreation {
5964
UriConstructor() { this.getConstructor().getDeclaringType().getQualifiedName() = "java.net.URI" }
6065

61-
override Expr hostArg() {
66+
override Expr getHostArg() {
6267
// URI​(String str)
6368
result = this.getArgument(0) and this.getNumArgument() = 1
6469
or
@@ -73,20 +78,22 @@ class UriConstructor extends ClassInstanceExpr, UriCreation {
7378
}
7479
}
7580

81+
/** An URI create call */
7682
class UriCreate extends Call, UriCreation {
7783
UriCreate() {
7884
this.getCallee().getName() = "create" and
7985
this.getCallee().getDeclaringType() instanceof TypeUri
8086
}
8187

82-
override Expr hostArg() { result = this.getArgument(0) }
88+
override Expr getHostArg() { result = this.getArgument(0) }
8389
}
8490

8591
/* An URL constructor expression */
8692
class UrlConstructor extends ClassInstanceExpr {
8793
UrlConstructor() { this.getConstructor().getDeclaringType().getQualifiedName() = "java.net.URL" }
8894

89-
Expr hostArg() {
95+
/** Returns the host of the newly created URI. */
96+
Expr getHostArg() {
9097
// URL(String spec)
9198
this.getNumArgument() = 1 and result = this.getArgument(0)
9299
or
@@ -104,6 +111,7 @@ class UrlConstructor extends ClassInstanceExpr {
104111
result = this.getArgument(1)
105112
}
106113

114+
/** Returns the expression which corresponds to the protocol of the url. */
107115
Expr protocolArg() {
108116
// In all cases except where the first parameter is a URL, the argument
109117
// containing the protocol is the first one, otherwise it is the second.
@@ -113,13 +121,15 @@ class UrlConstructor extends ClassInstanceExpr {
113121
}
114122
}
115123

124+
/** Models the `openStream` method of `java.net.url`. */
116125
class UrlOpenStreamMethod extends Method {
117126
UrlOpenStreamMethod() {
118127
this.getDeclaringType() instanceof TypeUrl and
119128
this.getName() = "openStream"
120129
}
121130
}
122131

132+
/** Models the `openConnection` method of `java.net.url`. */
123133
class UrlOpenConnectionMethod extends Method {
124134
UrlOpenConnectionMethod() {
125135
this.getDeclaringType() instanceof TypeUrl and

java/ql/test/library-tests/frameworks/javase/Uri.java renamed to java/ql/test/library-tests/frameworks/Networking/Uri.java

File renamed without changes.

java/ql/test/library-tests/frameworks/javase/openConnection.expected renamed to java/ql/test/library-tests/frameworks/Networking/openConnection.expected

File renamed without changes.

java/ql/test/library-tests/frameworks/javase/openConnection.ql renamed to java/ql/test/library-tests/frameworks/Networking/openConnection.ql

File renamed without changes.

java/ql/test/library-tests/frameworks/javase/openStream.expected renamed to java/ql/test/library-tests/frameworks/Networking/openStream.expected

File renamed without changes.

0 commit comments

Comments
 (0)