Skip to content

Commit dd4405c

Browse files
authored
Merge pull request #22578 from theinfosecguy/java-uri-host-allowlist
Java: Improve URI host allowlist checks
2 parents cf4d0e1 + dac9f1b commit dd4405c

6 files changed

Lines changed: 825 additions & 9 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The queries `java/ssrf` and `java/unvalidated-url-redirection` recognize case-insensitive comparisons of `URI.getHost()` with fixed string allowlist entries, as well as equality and case-insensitive comparisons when the host is stored in a local variable.

‎java/ql/lib/semmle/code/java/security/RequestForgery.qll‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import semmle.code.java.frameworks.Properties
1313
private import semmle.code.java.controlflow.Guards
1414
private import semmle.code.java.dataflow.StringPrefixes
1515
private import semmle.code.java.dataflow.ExternalFlow
16+
private import semmle.code.java.dataflow.SSA
1617
private import semmle.code.java.security.Sanitizers
1718

1819
/**
@@ -122,11 +123,21 @@ private class ExternalRequestForgerySanitizer extends RequestForgerySanitizer {
122123
ExternalRequestForgerySanitizer() { barrierNode(this, "request-forgery") }
123124
}
124125

125-
/**
126-
* A comparison on the host of a url, that is a sanitizer for URL redirects.
127-
* E.g. `"example.org".equals(url.getHost())"`
128-
*/
126+
/** Gets a host read, following local assignments with a single explicit SSA definition. */
127+
private MethodCall getCheckedHost(Expr checked) {
128+
result = checked.getUnderlyingExpr() and
129+
result.getMethod().hasQualifiedName("java.net", "URI", "getHost")
130+
or
131+
exists(SsaExplicitWrite def |
132+
checked.getUnderlyingExpr().(VarRead).getVariable() instanceof LocalScopeVariable and
133+
def.getARead() = checked.getUnderlyingExpr() and
134+
result = getCheckedHost(def.getValue())
135+
)
136+
}
137+
138+
/** Holds if a comparison validates the host of a URI. */
129139
private predicate isHostComparisonSanitizer(Guard guard, Expr e, boolean branch) {
140+
// Direct equality checks also accept nonconstant comparison operands.
130141
guard =
131142
any(MethodCall equalsCall |
132143
equalsCall.getMethod().getName() = "equals" and
@@ -137,6 +148,21 @@ private predicate isHostComparisonSanitizer(Guard guard, Expr e, boolean branch)
137148
e = hostCall.getQualifier()
138149
)
139150
)
151+
or
152+
exists(MethodCall comparison, Expr checked, Expr allowed |
153+
guard = comparison and
154+
comparison.getMethod().getDeclaringType() instanceof TypeString and
155+
comparison.getMethod().hasName(["equals", "equalsIgnoreCase"]) and
156+
branch = true and
157+
(
158+
checked = comparison.getQualifier() and allowed = comparison.getArgument(0)
159+
or
160+
checked = comparison.getArgument(0) and allowed = comparison.getQualifier()
161+
) and
162+
// New cases require a fixed allowlist entry, not another untrusted value.
163+
exists(allowed.(CompileTimeConstantExpr).getStringValue()) and
164+
e = getCheckedHost(checked).getQualifier()
165+
)
140166
}
141167

142168
/**
Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
import java.net.URI;
2+
import java.util.Arrays;
3+
import javax.servlet.http.HttpServletRequest;
4+
import javax.servlet.http.HttpServletResponse;
5+
6+
public class HostComparisonRedirect {
7+
public void unguarded(HttpServletRequest request, HttpServletResponse response) throws Exception {
8+
URI uri = new URI(request.getParameter("url")); // $ Source
9+
response.sendRedirect(uri.toString()); // $ Alert
10+
}
11+
12+
public void directEquals(HttpServletRequest request, HttpServletResponse response) throws Exception {
13+
URI uri = new URI(request.getParameter("url"));
14+
if ("example.com".equals(uri.getHost())) {
15+
response.sendRedirect(uri.toString());
16+
}
17+
}
18+
19+
public void directEqualsIgnoreCase(HttpServletRequest request, HttpServletResponse response) throws Exception {
20+
URI uri = new URI(request.getParameter("url"));
21+
if ("example.com".equalsIgnoreCase(uri.getHost())) {
22+
response.sendRedirect(uri.toString());
23+
}
24+
}
25+
26+
public void reversedEqualsIgnoreCase(HttpServletRequest request, HttpServletResponse response) throws Exception {
27+
URI uri = new URI(request.getParameter("url"));
28+
if (uri.getHost().equalsIgnoreCase("example.com")) {
29+
response.sendRedirect(uri.toString());
30+
}
31+
}
32+
33+
public void localEquals(HttpServletRequest request, HttpServletResponse response) throws Exception {
34+
URI uri = new URI(request.getParameter("url"));
35+
String host = uri.getHost();
36+
if ("example.com".equals(host)) {
37+
response.sendRedirect(uri.toString());
38+
}
39+
}
40+
41+
public void localEqualsIgnoreCase(HttpServletRequest request, HttpServletResponse response) throws Exception {
42+
URI uri = new URI(request.getParameter("url"));
43+
String host = uri.getHost();
44+
if ("example.com".equalsIgnoreCase(host)) {
45+
response.sendRedirect(uri.toString());
46+
}
47+
}
48+
49+
public void streamEquals(HttpServletRequest request, HttpServletResponse response) throws Exception {
50+
URI uri = new URI(request.getParameter("url")); // $ SPURIOUS: Source
51+
if (Arrays.stream(new String[]{"example.com"}).anyMatch(uri.getHost()::equals)) {
52+
response.sendRedirect(uri.toString()); // $ SPURIOUS: Alert
53+
}
54+
}
55+
56+
public void reportedStreamHelper(HttpServletRequest request, HttpServletResponse response) throws Exception {
57+
URI uri = new URI(request.getParameter("url")); // $ SPURIOUS: Source
58+
if (isAllowedStream(uri.getHost(), "example.com")) {
59+
response.sendRedirect(uri.toString()); // $ SPURIOUS: Alert
60+
}
61+
}
62+
63+
private static boolean isAllowedStream(String host, String... allowedHosts) {
64+
return Arrays.stream(allowedHosts).anyMatch(host::equalsIgnoreCase);
65+
}
66+
67+
public void reportedLoopHelper(HttpServletRequest request, HttpServletResponse response) throws Exception {
68+
URI uri = new URI(request.getParameter("url")); // $ SPURIOUS: Source
69+
if (isAllowedLoop(uri.getHost(), "example.com")) {
70+
response.sendRedirect(uri.toString()); // $ SPURIOUS: Alert
71+
}
72+
}
73+
74+
private static boolean isAllowedLoop(String host, String... allowedHosts) {
75+
for (String allowed : allowedHosts) {
76+
if (allowed.equalsIgnoreCase(host)) {
77+
return true;
78+
}
79+
}
80+
return false;
81+
}
82+
83+
public void wrongBranch(HttpServletRequest request, HttpServletResponse response) throws Exception {
84+
URI uri = new URI(request.getParameter("url")); // $ Source
85+
if (!"example.com".equals(uri.getHost())) {
86+
response.sendRedirect(uri.toString()); // $ Alert
87+
}
88+
}
89+
90+
public void differentUri(HttpServletRequest request, HttpServletResponse response) throws Exception {
91+
URI checked = new URI(request.getParameter("checked"));
92+
URI used = new URI(request.getParameter("url")); // $ Source
93+
if ("example.com".equals(checked.getHost())) {
94+
response.sendRedirect(used.toString()); // $ Alert
95+
}
96+
}
97+
98+
public void constantUrl(HttpServletResponse response) throws Exception {
99+
response.sendRedirect("https://example.com/");
100+
}
101+
102+
public void caseInsensitiveWrongBranch(HttpServletRequest request, HttpServletResponse response) throws Exception {
103+
URI uri = new URI(request.getParameter("url")); // $ Source
104+
if (!"example.com".equalsIgnoreCase(uri.getHost())) {
105+
response.sendRedirect(uri.toString()); // $ Alert
106+
}
107+
}
108+
109+
public void caseInsensitiveDifferentUri(HttpServletRequest request, HttpServletResponse response) throws Exception {
110+
URI checked = new URI(request.getParameter("checked"));
111+
URI used = new URI(request.getParameter("url")); // $ Source
112+
if ("example.com".equalsIgnoreCase(checked.getHost())) {
113+
response.sendRedirect(used.toString()); // $ Alert
114+
}
115+
}
116+
117+
public void reassignedHost(HttpServletRequest request, HttpServletResponse response) throws Exception {
118+
URI uri = new URI(request.getParameter("url")); // $ Source
119+
String host = uri.getHost();
120+
host = "example.com";
121+
if ("example.com".equalsIgnoreCase(host)) {
122+
response.sendRedirect(uri.toString()); // $ Alert
123+
}
124+
}
125+
126+
public void conditionallyReassignedHost(HttpServletRequest request, HttpServletResponse response, boolean skip) throws Exception {
127+
URI uri = new URI(request.getParameter("url")); // $ Source
128+
String host = uri.getHost();
129+
if (skip) {
130+
host = "example.com";
131+
}
132+
if ("example.com".equalsIgnoreCase(host)) {
133+
response.sendRedirect(uri.toString()); // $ Alert
134+
}
135+
}
136+
137+
public void reassignedUri(HttpServletRequest request, HttpServletResponse response) throws Exception {
138+
URI uri = new URI("https://example.com/");
139+
String host = uri.getHost();
140+
uri = new URI(request.getParameter("url")); // $ Source
141+
if ("example.com".equalsIgnoreCase(host)) {
142+
response.sendRedirect(uri.toString()); // $ Alert
143+
}
144+
}
145+
146+
public void untrustedComparison(HttpServletRequest request, HttpServletResponse response) throws Exception {
147+
URI uri = new URI(request.getParameter("url")); // $ Source
148+
if (request.getParameter("allowed").equalsIgnoreCase(uri.getHost())) {
149+
response.sendRedirect(uri.toString()); // $ Alert
150+
}
151+
}
152+
153+
public void unrelatedComparison(HttpServletRequest request, HttpServletResponse response) throws Exception {
154+
URI uri = new URI(request.getParameter("url")); // $ Source
155+
if (new FakeComparison().equalsIgnoreCase(uri.getHost())) {
156+
response.sendRedirect(uri.toString()); // $ Alert
157+
}
158+
}
159+
160+
private static class FakeComparison {
161+
boolean equalsIgnoreCase(String host) {
162+
return true;
163+
}
164+
}
165+
166+
public void aliasChain(HttpServletRequest request, HttpServletResponse response) throws Exception {
167+
URI uri = new URI(request.getParameter("url"));
168+
String host = uri.getHost();
169+
String alias = host;
170+
String secondAlias = alias;
171+
if ("example.com".equalsIgnoreCase(secondAlias)) {
172+
response.sendRedirect(uri.toString());
173+
}
174+
}
175+
176+
public void finallyOverwrite(HttpServletRequest request, HttpServletResponse response, boolean skip) throws Exception {
177+
URI uri = new URI(request.getParameter("url")); // $ Source
178+
String host = uri.getHost();
179+
try {
180+
if (skip) return;
181+
host = "example.com";
182+
} finally {
183+
if ("example.com".equalsIgnoreCase(host)) {
184+
response.sendRedirect(uri.toString()); // $ Alert
185+
}
186+
}
187+
}
188+
189+
public void rejectionReturns(HttpServletRequest request, HttpServletResponse response) throws Exception {
190+
URI uri = new URI(request.getParameter("url"));
191+
String host = uri.getHost();
192+
if (!"example.com".equalsIgnoreCase(host)) return;
193+
response.sendRedirect(uri.toString());
194+
}
195+
196+
private static final String ALLOWED_HOST = "example.com";
197+
198+
public void constantAllowlistField(HttpServletRequest request, HttpServletResponse response) throws Exception {
199+
URI uri = new URI(request.getParameter("url"));
200+
String host = uri.getHost();
201+
if (ALLOWED_HOST.equalsIgnoreCase(host)) {
202+
response.sendRedirect(uri.toString());
203+
}
204+
}
205+
206+
public void orBypass(HttpServletRequest request, HttpServletResponse response, boolean skip) throws Exception {
207+
URI uri = new URI(request.getParameter("url")); // $ Source
208+
String host = uri.getHost();
209+
if ("example.com".equalsIgnoreCase(host) || skip) {
210+
response.sendRedirect(uri.toString()); // $ Alert
211+
}
212+
}
213+
214+
public void assignmentCast(HttpServletRequest request, HttpServletResponse response) throws Exception {
215+
URI uri = new URI(request.getParameter("url"));
216+
Object value = uri.getHost();
217+
String host = (String) value;
218+
if ("example.com".equalsIgnoreCase(host)) {
219+
response.sendRedirect(uri.toString());
220+
}
221+
}
222+
223+
public void comparisonCast(HttpServletRequest request, HttpServletResponse response) throws Exception {
224+
URI uri = new URI(request.getParameter("url"));
225+
Object host = uri.getHost();
226+
if ("example.com".equalsIgnoreCase((String) host)) {
227+
response.sendRedirect(uri.toString());
228+
}
229+
}
230+
231+
public void castHostOverwrite(HttpServletRequest request, HttpServletResponse response) throws Exception {
232+
URI uri = new URI(request.getParameter("url")); // $ Source
233+
Object host = uri.getHost();
234+
host = "example.com";
235+
if ("example.com".equalsIgnoreCase((String) host)) {
236+
response.sendRedirect(uri.toString()); // $ Alert
237+
}
238+
}
239+
240+
public void castHostConditionalOverwrite(HttpServletRequest request, HttpServletResponse response, boolean skip) throws Exception {
241+
URI uri = new URI(request.getParameter("url")); // $ Source
242+
Object host = uri.getHost();
243+
if (skip) host = "example.com";
244+
if ("example.com".equalsIgnoreCase((String) host)) {
245+
response.sendRedirect(uri.toString()); // $ Alert
246+
}
247+
}
248+
}

0 commit comments

Comments
 (0)