Skip to content

Commit b821f91

Browse files
committed
Address issues with matching empty host and host in a concatenated string
1 parent 9a8eed8 commit b821f91

File tree

3 files changed

+21
-25
lines changed

3 files changed

+21
-25
lines changed

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

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class HttpStringLiteral extends StringLiteral {
8787
HttpStringLiteral() {
8888
// Match URLs with the HTTP protocol and without private IP addresses to reduce false positives.
8989
exists(string s | this.getRepresentedString() = s |
90-
s.regexpMatch("(?i)http://[\\[:a-zA-Z0-9].*") and
90+
s.regexpMatch("(?i)http://[\\[a-zA-Z0-9].*") and
9191
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
9292
)
9393
}
@@ -106,37 +106,33 @@ predicate concatHttpString(Expr protocol, Expr host) {
106106
.(CompileTimeConstantExpr)
107107
.getStringValue()
108108
.regexpMatch("(?i)http(://)?")
109-
) and // Not empty host string
110-
(
111-
host.(CompileTimeConstantExpr).getStringValue().length() > 0 or
112-
host
113-
.(VarAccess)
114-
.getVariable()
115-
.getAnAssignedValue()
116-
.(CompileTimeConstantExpr)
117-
.getStringValue()
118-
.length() > 0
119109
) and
120-
not (
121-
host.(CompileTimeConstantExpr).getStringValue().regexpMatch(getPrivateHostRegex()) or
122-
host
123-
.(VarAccess)
124-
.getVariable()
125-
.getAnAssignedValue()
126-
.(CompileTimeConstantExpr)
127-
.getStringValue()
128-
.regexpMatch(getPrivateHostRegex())
110+
not exists(string hostString |
111+
hostString = host.(CompileTimeConstantExpr).getStringValue() or
112+
hostString =
113+
host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
114+
|
115+
hostString.length() = 0 or // Empty host is loopback address
116+
hostString.regexpMatch(getPrivateHostRegex())
129117
)
130118
}
131119

120+
/** Gets the leftmost operand in a concatenated string */
121+
Expr getLeftmostConcatOperand(Expr expr) {
122+
if expr instanceof AddExpr
123+
then result = getLeftmostConcatOperand(expr.(AddExpr).getLeftOperand())
124+
else result = expr
125+
}
126+
132127
/**
133128
* String concatenated with `HttpStringLiteral`.
134129
*/
135130
class HttpString extends Expr {
136131
HttpString() {
137132
this instanceof HttpStringLiteral
138133
or
139-
concatHttpString(this.(AddExpr).getLeftOperand(), this.(AddExpr).getRightOperand())
134+
concatHttpString(this.(AddExpr).getLeftOperand(),
135+
getLeftmostConcatOperand(this.(AddExpr).getRightOperand()))
140136
}
141137
}
142138

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
edges
22
| InsecureBasicAuth.java:20:39:20:52 | ... + ... : String | InsecureBasicAuth.java:28:3:28:6 | post |
3-
| InsecureBasicAuth.java:35:19:35:73 | "http://www.example.com:dashboardPort/payment/retrieve" : String | InsecureBasicAuth.java:38:3:38:5 | get |
3+
| InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" : String | InsecureBasicAuth.java:38:3:38:5 | get |
44
| InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:54:3:54:6 | post |
55
| InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:71:3:71:6 | post |
66
| InsecureBasicAuth.java:78:47:78:52 | "http" : String | InsecureBasicAuth.java:86:3:86:6 | post |
@@ -13,7 +13,7 @@ edges
1313
nodes
1414
| InsecureBasicAuth.java:20:39:20:52 | ... + ... : String | semmle.label | ... + ... : String |
1515
| InsecureBasicAuth.java:28:3:28:6 | post | semmle.label | post |
16-
| InsecureBasicAuth.java:35:19:35:73 | "http://www.example.com:dashboardPort/payment/retrieve" : String | semmle.label | "http://www.example.com:dashboardPort/payment/retrieve" : String |
16+
| InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" : String | semmle.label | "http://www.example.com:8000/payment/retrieve" : String |
1717
| InsecureBasicAuth.java:38:3:38:5 | get | semmle.label | get |
1818
| InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | semmle.label | "http://www.example.com/rest/getuser.do?uid=abcdx" : String |
1919
| InsecureBasicAuth.java:54:3:54:6 | post | semmle.label | post |
@@ -33,7 +33,7 @@ nodes
3333
| InsecureBasicAuth.java:149:3:149:6 | conn | semmle.label | conn |
3434
#select
3535
| InsecureBasicAuth.java:28:3:28:6 | post | InsecureBasicAuth.java:20:39:20:52 | ... + ... : String | InsecureBasicAuth.java:28:3:28:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:20:39:20:52 | ... + ... | HTTP url |
36-
| InsecureBasicAuth.java:38:3:38:5 | get | InsecureBasicAuth.java:35:19:35:73 | "http://www.example.com:dashboardPort/payment/retrieve" : String | InsecureBasicAuth.java:38:3:38:5 | get | Insecure basic authentication from $@. | InsecureBasicAuth.java:35:19:35:73 | "http://www.example.com:dashboardPort/payment/retrieve" | HTTP url |
36+
| InsecureBasicAuth.java:38:3:38:5 | get | InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" : String | InsecureBasicAuth.java:38:3:38:5 | get | Insecure basic authentication from $@. | InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" | HTTP url |
3737
| InsecureBasicAuth.java:54:3:54:6 | post | InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:54:3:54:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
3838
| InsecureBasicAuth.java:71:3:71:6 | post | InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:71:3:71:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
3939
| InsecureBasicAuth.java:86:3:86:6 | post | InsecureBasicAuth.java:78:47:78:52 | "http" : String | InsecureBasicAuth.java:86:3:86:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:78:47:78:52 | "http" | HTTP url |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ public void testHttpUrlConnection3(String username, String password) {
156156
String host = "LOCALHOST";
157157
String authString = username + ":" + password;
158158
String encoding = Base64.getEncoder().encodeToString(authString.getBytes("UTF-8"));
159-
HttpURLConnection conn = (HttpURLConnection) new URL("http://"+host+"/rest/getuser.do"+"?uid=abcdx").openConnection();
159+
HttpURLConnection conn = (HttpURLConnection) new URL("http://"+(((host+"/rest/getuser.do")+"?uid=abcdx"))).openConnection();
160160
conn.setRequestMethod("POST");
161161
conn.setDoOutput(true);
162162
conn.setRequestProperty("Authorization", "Basic " + encoding);

0 commit comments

Comments
 (0)