Skip to content

Commit 40c932a

Browse files
Jami CogswellJami Cogswell
authored andcommitted
Java: move UrlForward.qll code to UrlForwardQuery.qll
1 parent 2391fe7 commit 40c932a

File tree

2 files changed

+172
-178
lines changed

2 files changed

+172
-178
lines changed

java/ql/lib/semmle/code/java/security/UrlForward.qll

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

java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll

Lines changed: 172 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,177 @@
1-
/** Provides a taint-tracking configuration for reasoning about URL forwarding. */
1+
/** Provides classes and a taint-tracking configuration to reason about unsafe URL forwarding. */
22

33
import java
4-
import semmle.code.java.security.UrlForward
5-
import semmle.code.java.dataflow.FlowSources
6-
import semmle.code.java.security.PathSanitizer
4+
private import semmle.code.java.dataflow.ExternalFlow
5+
private import semmle.code.java.dataflow.FlowSources
6+
private import semmle.code.java.dataflow.StringPrefixes
7+
private import semmle.code.java.security.PathSanitizer
8+
private import semmle.code.java.controlflow.Guards
9+
private import semmle.code.java.security.Sanitizers
10+
11+
/** A URL forward sink. */
12+
abstract class UrlForwardSink extends DataFlow::Node { }
13+
14+
/**
15+
* A default sink representing methods susceptible to URL
16+
* forwarding attacks.
17+
*/
18+
private class DefaultUrlForwardSink extends UrlForwardSink {
19+
DefaultUrlForwardSink() { sinkNode(this, "url-forward") }
20+
}
21+
22+
/**
23+
* An expression appended (perhaps indirectly) to `"forward:"`
24+
* and reachable from a Spring entry point.
25+
*/
26+
private class SpringUrlForwardPrefixSink extends UrlForwardSink {
27+
SpringUrlForwardPrefixSink() {
28+
any(SpringRequestMappingMethod srmm).polyCalls*(this.getEnclosingCallable()) and
29+
appendedToForwardPrefix(this)
30+
}
31+
}
32+
33+
pragma[nomagic]
34+
private predicate appendedToForwardPrefix(DataFlow::ExprNode exprNode) {
35+
exists(ForwardPrefix fp | exprNode.asExpr() = fp.getAnAppendedExpression())
36+
}
37+
38+
private class ForwardPrefix extends InterestingPrefix {
39+
ForwardPrefix() { this.getStringValue() = "forward:" }
40+
41+
override int getOffset() { result = 0 }
42+
}
43+
44+
/** A URL forward barrier. */
45+
abstract class UrlForwardBarrier extends DataFlow::Node { }
46+
47+
private class PrimitiveBarrier extends UrlForwardBarrier instanceof SimpleTypeSanitizer { }
48+
49+
/**
50+
* A barrier for values appended to a "redirect:" prefix.
51+
* These results are excluded because they should be handled
52+
* by the `java/unvalidated-url-redirection` query instead.
53+
*/
54+
private class RedirectPrefixBarrier extends UrlForwardBarrier {
55+
RedirectPrefixBarrier() { this.asExpr() = any(RedirectPrefix fp).getAnAppendedExpression() }
56+
}
57+
58+
private class RedirectPrefix extends InterestingPrefix {
59+
RedirectPrefix() { this.getStringValue() = "redirect:" }
60+
61+
override int getOffset() { result = 0 }
62+
}
63+
64+
/**
65+
* A value that is the result of prepending a string that prevents
66+
* any value from controlling the path of a URL.
67+
*/
68+
private class FollowsBarrierPrefix extends UrlForwardBarrier {
69+
FollowsBarrierPrefix() { this.asExpr() = any(BarrierPrefix fp).getAnAppendedExpression() }
70+
}
71+
72+
private class BarrierPrefix extends InterestingPrefix {
73+
int offset;
74+
75+
BarrierPrefix() {
76+
// Matches strings that look like when prepended to untrusted input, they will restrict
77+
// the path of a URL: for example, anything containing `?` or `#`.
78+
exists(this.getStringValue().regexpFind("[?#]", 0, offset))
79+
or
80+
this.(CharacterLiteral).getValue() = ["?", "#"] and offset = 0
81+
}
82+
83+
override int getOffset() { result = offset }
84+
}
85+
86+
/**
87+
* A barrier that protects against path injection vulnerabilities
88+
* while accounting for URL encoding.
89+
*/
90+
private class UrlPathBarrier extends UrlForwardBarrier instanceof PathInjectionSanitizer {
91+
UrlPathBarrier() {
92+
this instanceof ExactPathMatchSanitizer or
93+
this instanceof NoUrlEncodingBarrier or
94+
this instanceof FullyDecodesUrlBarrier
95+
}
96+
}
97+
98+
/** A call to a method that decodes a URL. */
99+
abstract class UrlDecodeCall extends MethodCall { }
100+
101+
private class DefaultUrlDecodeCall extends UrlDecodeCall {
102+
DefaultUrlDecodeCall() {
103+
this.getMethod() instanceof UrlDecodeMethod or
104+
this.getMethod().hasQualifiedName("org.eclipse.jetty.util.URIUtil", "URIUtil", "decodePath")
105+
}
106+
}
107+
108+
/** A repeated call to a method that decodes a URL. */
109+
abstract class RepeatedUrlDecodeCall extends MethodCall { }
110+
111+
private class DefaultRepeatedUrlDecodeCall extends RepeatedUrlDecodeCall instanceof UrlDecodeCall {
112+
DefaultRepeatedUrlDecodeCall() { this.getAnEnclosingStmt() instanceof LoopStmt }
113+
}
114+
115+
/** A method call that checks a string for URL encoding. */
116+
abstract class CheckUrlEncodingCall extends MethodCall { }
117+
118+
private class DefaultCheckUrlEncodingCall extends CheckUrlEncodingCall {
119+
DefaultCheckUrlEncodingCall() {
120+
this.getMethod() instanceof StringContainsMethod and
121+
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "%"
122+
}
123+
}
124+
125+
/** A guard that looks for a method call that checks for URL encoding. */
126+
private class CheckUrlEncodingGuard extends Guard instanceof CheckUrlEncodingCall {
127+
Expr getCheckedExpr() { result = this.(MethodCall).getQualifier() }
128+
}
129+
130+
/** Holds if `g` is guard for a URL that does not contain URL encoding. */
131+
private predicate noUrlEncodingGuard(Guard g, Expr e, boolean branch) {
132+
e = g.(CheckUrlEncodingGuard).getCheckedExpr() and
133+
branch = false
134+
or
135+
branch = false and
136+
g.(Expr).getType() instanceof BooleanType and
137+
(
138+
exists(CheckUrlEncodingCall call, AssignExpr ae |
139+
ae.getSource() = call and
140+
e = call.getQualifier() and
141+
g = ae.getDest()
142+
)
143+
or
144+
exists(CheckUrlEncodingCall call, LocalVariableDeclExpr vde |
145+
vde.getInitOrPatternSource() = call and
146+
e = call.getQualifier() and
147+
g = vde.getAnAccess()
148+
)
149+
)
150+
}
151+
152+
/** A barrier for URLs that do not contain URL encoding. */
153+
private class NoUrlEncodingBarrier extends DataFlow::Node {
154+
NoUrlEncodingBarrier() { this = DataFlow::BarrierGuard<noUrlEncodingGuard/3>::getABarrierNode() }
155+
}
156+
157+
/** Holds if `g` is guard for a URL that is fully decoded. */
158+
private predicate fullyDecodesUrlGuard(Expr e) {
159+
exists(CheckUrlEncodingGuard g, RepeatedUrlDecodeCall decodeCall |
160+
e = g.getCheckedExpr() and
161+
g.controls(decodeCall.getBasicBlock(), true)
162+
)
163+
}
164+
165+
/** A barrier for URLs that are fully decoded. */
166+
private class FullyDecodesUrlBarrier extends DataFlow::Node {
167+
FullyDecodesUrlBarrier() {
168+
exists(Variable v, Expr e | this.asExpr() = v.getAnAccess() |
169+
fullyDecodesUrlGuard(e) and
170+
e = v.getAnAccess() and
171+
e.getBasicBlock().bbDominates(this.asExpr().getBasicBlock())
172+
)
173+
}
174+
}
7175

8176
/**
9177
* A taint-tracking configuration for reasoning about URL forwarding.

0 commit comments

Comments
 (0)