Skip to content

Commit 81de1b1

Browse files
luchua-bcluchua-bc
committed
Revamp the source of path query
1 parent 5520504 commit 81de1b1

File tree

2 files changed

+179
-61
lines changed

2 files changed

+179
-61
lines changed

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

Lines changed: 147 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,47 @@ class ApacheHttpRequest extends RefType {
3232
class URLConstructor extends ClassInstanceExpr {
3333
URLConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl }
3434

35-
Expr stringArg() {
36-
// URLs constructed with any of the four string constructors below:
37-
// URL(String spec), URL(String protocol, String host, int port, String file), URL(String protocol, String host, int port, String file, URLStreamHandler handler), URL(String protocol, String host, String file)
35+
// URLs constructed with the string constructor `URL(String spec)`
36+
Expr specArg() {
3837
this.getConstructor().getParameter(0).getType() instanceof TypeString and
39-
result = this.getArgument(0) // First argument contains the protocol part.
38+
(
39+
this.getConstructor().getNumberOfParameters() = 1 and
40+
result = this.getArgument(0) // First argument contains the whole spec.
41+
)
42+
}
43+
44+
// URLs constructed with any of the three string constructors below:
45+
// `URL(String protocol, String host, int port, String file)`,
46+
// `URL(String protocol, String host, int port, String file, URLStreamHandler handler)`,
47+
// `URL(String protocol, String host, String file)`
48+
Expr protocolArg() {
49+
this.getConstructor().getParameter(0).getType() instanceof TypeString and
50+
(
51+
this.getConstructor().getNumberOfParameters() > 1 and
52+
result = this.getArgument(0) // First argument contains the protocol part.
53+
)
54+
}
55+
56+
Expr hostArg() {
57+
this.getConstructor().getParameter(0).getType() instanceof TypeString and
58+
(
59+
this.getConstructor().getNumberOfParameters() > 1 and
60+
result = this.getArgument(1) // Second argument contains the host part.
61+
)
4062
}
4163
}
4264

4365
/**
44-
* The type `java.net.URLConnection`.
66+
* Gets a regular expression for matching private hosts.
4567
*/
46-
class TypeHttpUrlConnection extends RefType {
47-
TypeHttpUrlConnection() {
48-
this.getASourceSupertype*().hasQualifiedName("java.net", "HttpURLConnection")
49-
}
68+
private string getPrivateHostRegex() {
69+
result = "localhost(/.*)?" or
70+
result = "127\\.0\\.0\\.1(/.*)?" or // IPv4 patterns
71+
result = "10(\\.[0-9]+){3}(/.*)?" or
72+
result = "172\\.16(\\.[0-9]+){2}(/.*)?" or
73+
result = "192.168(\\.[0-9]+){2}(/.*)?" or
74+
result = "\\[0:0:0:0:0:0:0:1\\](/.*)?" or // IPv6 patterns
75+
result = "\\[::1\\](/.*)?"
5076
}
5177

5278
/**
@@ -56,50 +82,55 @@ class HttpString extends StringLiteral {
5682
HttpString() {
5783
// Match URLs with the HTTP protocol and without private IP addresses to reduce false positives.
5884
exists(string s | this.getRepresentedString() = s |
59-
s = "http"
85+
s.regexpMatch("(?i)http")
6086
or
61-
s.matches("http://%") and
62-
not s.matches("%/localhost%") and
63-
not s.matches("%/127.0.0.1%") and
64-
not s.matches("%/10.%") and
65-
not s.matches("%/172.16.%") and
66-
not s.matches("%/192.168.%")
87+
s.regexpMatch("(?i)http://.*") and
88+
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
6789
)
6890
}
6991
}
7092

93+
/**
94+
* Checks both parts of protocol and host.
95+
*/
96+
predicate concatHttpString(Expr protocol, Expr host) {
97+
(
98+
protocol.(CompileTimeConstantExpr).getStringValue().regexpMatch("(?i)http(://)?") or
99+
protocol
100+
.(VarAccess)
101+
.getVariable()
102+
.getAnAssignedValue()
103+
.(CompileTimeConstantExpr)
104+
.getStringValue()
105+
.regexpMatch("(?i)http(://)?")
106+
) and
107+
not (
108+
host.(CompileTimeConstantExpr).getStringValue().regexpMatch(getPrivateHostRegex()) or
109+
host
110+
.(VarAccess)
111+
.getVariable()
112+
.getAnAssignedValue()
113+
.(CompileTimeConstantExpr)
114+
.getStringValue()
115+
.regexpMatch(getPrivateHostRegex())
116+
)
117+
}
118+
71119
/**
72120
* String concatenated with `HttpString`.
73121
*/
74122
predicate builtFromHttpStringConcat(Expr expr) {
75123
expr instanceof HttpString
76124
or
77-
builtFromHttpStringConcat(expr.(AddExpr).getLeftOperand())
125+
expr.(VarAccess).getVariable().getAnAssignedValue() instanceof HttpString
78126
or
79-
exists(Expr other | builtFromHttpStringConcat(other) |
80-
exists(Variable var | var.getAnAssignedValue() = other and var.getAnAccess() = expr)
81-
)
127+
concatHttpString(expr.(AddExpr).getLeftOperand(), expr.(AddExpr).getRightOperand())
82128
or
83-
exists(Expr other | builtFromHttpStringConcat(other) |
84-
exists(ConstructorCall cc |
85-
(
86-
cc.getConstructedType().hasQualifiedName("org.apache.http.message", "BasicRequestLine") and
87-
cc.getArgument(1) = other // BasicRequestLine(String method, String uri, ProtocolVersion version)
88-
or
89-
cc.getConstructedType().hasQualifiedName("java.net", "URI") and
90-
cc.getArgument(0) = other // URI(String str) or URI(String scheme, ...)
91-
) and
92-
(
93-
cc = expr
94-
or
95-
exists(Variable var, VariableAssign va |
96-
var.getAnAccess() = expr and
97-
va.getDestVar() = var and
98-
va.getSource() = cc
99-
)
100-
)
101-
)
102-
)
129+
concatHttpString(expr.(AddExpr).getLeftOperand().(AddExpr).getLeftOperand(),
130+
expr.(AddExpr).getLeftOperand().(AddExpr).getRightOperand())
131+
or
132+
concatHttpString(expr.(AddExpr).getLeftOperand(),
133+
expr.(AddExpr).getRightOperand().(AddExpr).getLeftOperand())
103134
}
104135

105136
/**
@@ -133,41 +164,98 @@ class HttpURLOpenMethod extends Method {
133164
class BasicAuthFlowConfig extends TaintTracking::Configuration {
134165
BasicAuthFlowConfig() { this = "InsecureBasicAuth::BasicAuthFlowConfig" }
135166

136-
override predicate isSource(DataFlow::Node src) {
167+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof HttpString }
168+
169+
override predicate isSink(DataFlow::Node sink) {
170+
exists(MethodAccess ma |
171+
sink.asExpr() = ma.getQualifier() and
172+
(
173+
ma.getMethod().hasName("addHeader") or
174+
ma.getMethod().hasName("setHeader") or
175+
ma.getMethod().hasName("setRequestProperty")
176+
) and
177+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and
178+
builtFromBasicAuthStringConcat(ma.getArgument(1))
179+
)
180+
}
181+
182+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
137183
exists(ConstructorCall cc |
184+
cc.getConstructedType() instanceof ApacheHttpRequest and
185+
node2.asExpr() = cc and
138186
(
139-
cc.getConstructedType() instanceof ApacheHttpRequest and
140-
cc = src.asExpr() and
187+
cc.getAnArgument() = node1.asExpr() and
141188
builtFromHttpStringConcat(cc.getAnArgument())
189+
or
190+
exists(ConstructorCall mcc |
191+
(
192+
mcc.getConstructedType().hasQualifiedName("org.apache.http.message", "BasicRequestLine") and
193+
mcc.getArgument(1) = node1.asExpr() and // `BasicRequestLine(String method, String uri, ProtocolVersion version)
194+
builtFromHttpStringConcat(mcc.getArgument(1))
195+
or
196+
mcc.getConstructedType().hasQualifiedName("java.net", "URI") and
197+
(
198+
mcc.getNumArgument() = 1 and
199+
mcc.getArgument(0) = node1.asExpr() and
200+
builtFromHttpStringConcat(mcc.getArgument(0)) // `URI(String str)`
201+
or
202+
mcc.getNumArgument() = 4 and
203+
mcc.getArgument(0) = node1.asExpr() and
204+
concatHttpString(mcc.getArgument(0), mcc.getArgument(1)) // `URI(String scheme, String host, String path, String fragment)`
205+
or
206+
mcc.getNumArgument() = 7 and
207+
mcc.getArgument(0) = node1.asExpr() and
208+
concatHttpString(mcc.getArgument(2), mcc.getArgument(1)) // `URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment)`
209+
)
210+
) and
211+
(
212+
cc.getAnArgument() = mcc
213+
or
214+
exists(VarAccess va |
215+
cc.getAnArgument() = va and va.getVariable().getAnAssignedValue() = mcc
216+
)
217+
)
218+
)
219+
or
220+
exists(StaticMethodAccess ma |
221+
ma.getMethod().getDeclaringType().hasQualifiedName("java.net", "URI") and
222+
ma.getMethod().hasName("create") and
223+
builtFromHttpStringConcat(ma.getArgument(0)) and
224+
node1.asExpr() = ma.getArgument(0) and
225+
(
226+
cc.getArgument(0) = ma
227+
or
228+
exists(VarAccess va |
229+
cc.getArgument(0) = va and va.getVariable().getAnAssignedValue() = ma
230+
)
231+
)
232+
)
142233
)
143234
)
144235
or
145236
exists(MethodAccess ma |
146237
ma.getMethod() instanceof HttpURLOpenMethod and
147-
ma = src.asExpr() and
238+
ma = node2.asExpr() and
148239
(
149-
builtFromHttpStringConcat(ma.getQualifier().(URLConstructor).stringArg()) or
240+
node1.asExpr() = ma.getQualifier().(URLConstructor).getArgument(0) and
241+
(
242+
builtFromHttpStringConcat(ma.getQualifier().(URLConstructor).specArg()) or
243+
concatHttpString(ma.getQualifier().(URLConstructor).protocolArg(),
244+
ma.getQualifier().(URLConstructor).hostArg())
245+
)
246+
or
150247
exists(URLConstructor uc, VarAccess va |
248+
node1.asExpr() = uc.getAnArgument() and
151249
uc = va.getVariable().getAnAssignedValue() and
152250
ma.getQualifier() = va and
153-
builtFromHttpStringConcat(uc.stringArg())
251+
(
252+
builtFromHttpStringConcat(uc.specArg()) or
253+
concatHttpString(uc.protocolArg(), uc.hostArg())
254+
)
154255
)
155256
)
156257
)
157258
}
158-
159-
override predicate isSink(DataFlow::Node sink) {
160-
exists(MethodAccess ma |
161-
sink.asExpr() = ma.getQualifier() and
162-
(
163-
ma.getMethod().hasName("addHeader") or
164-
ma.getMethod().hasName("setHeader") or
165-
ma.getMethod().hasName("setRequestProperty")
166-
) and
167-
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and
168-
builtFromBasicAuthStringConcat(ma.getArgument(1))
169-
)
170-
}
171259
}
172260

173261
from DataFlow::PathNode source, DataFlow::PathNode sink, BasicAuthFlowConfig config

‎java/ql/test/experimental/query-tests/security/CWE-522/InsecureBasicAuth.java

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public void testApacheHttpRequest2(String url) throws java.io.IOException {
4343
*/
4444
public void testApacheHttpRequest3(String username, String password) {
4545
String uriStr = "http://www.example.com/rest/getuser.do?uid=abcdx";
46-
HttpRequestBase post = new HttpPost(new URI(uriStr));
46+
HttpRequestBase post = new HttpPost(URI.create(uriStr));
4747
post.setHeader("Accept", "application/json");
4848
post.setHeader("Content-type", "application/json");
4949

@@ -88,7 +88,7 @@ public void testApacheHttpRequest5(String username, String password) {
8888
}
8989

9090
/**
91-
* Test basic authentication with Java HTTP URL connection.
91+
* Test basic authentication with Java HTTP URL connection using the `URL(String spec)` constructor.
9292
*/
9393
public void testHttpUrlConnection(String username, String password) {
9494
String urlStr = "http://www.example.com/rest/getuser.do?uid=abcdx";
@@ -100,4 +100,34 @@ public void testHttpUrlConnection(String username, String password) {
100100
conn.setDoOutput(true);
101101
conn.setRequestProperty("Authorization", "Basic " + encoding);
102102
}
103+
104+
/**
105+
* Test basic authentication with Java HTTP URL connection using the `URL(String protocol, String host, String file)` constructor.
106+
*/
107+
public void testHttpUrlConnection2(String username, String password) {
108+
String host = "www.example.com";
109+
String path = "/rest/getuser.do?uid=abcdx";
110+
String protocol = "http";
111+
String authString = username + ":" + password;
112+
String encoding = Base64.getEncoder().encodeToString(authString.getBytes("UTF-8"));
113+
URL url = new URL(protocol, host, path);
114+
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
115+
conn.setRequestMethod("POST");
116+
conn.setDoOutput(true);
117+
conn.setRequestProperty("Authorization", "Basic " + encoding);
118+
}
119+
120+
/**
121+
* Test basic authentication with Java HTTP URL connection using a constructor with private URL.
122+
*/
123+
public void testHttpUrlConnection3(String username, String password) {
124+
String host = "localhost";
125+
String urlStr = "http://"+host+"/rest/getuser.do?uid=abcdx";
126+
String authString = username + ":" + password;
127+
String encoding = Base64.getEncoder().encodeToString(authString.getBytes("UTF-8"));
128+
HttpURLConnection conn = (HttpURLConnection) new URL("http://"+host+"/rest/getuser.do?uid=abcdx").openConnection();
129+
conn.setRequestMethod("POST");
130+
conn.setDoOutput(true);
131+
conn.setRequestProperty("Authorization", "Basic " + encoding);
132+
}
103133
}

0 commit comments

Comments
 (0)