Skip to content

Commit b6886b8

Browse files
atorralbaatorralba
committed
Move code to qll file
1 parent 81feaae commit b6886b8

File tree

2 files changed

+122
-121
lines changed

2 files changed

+122
-121
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.ql‎

Lines changed: 2 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* @name Unsafe url forward or dispatch from remote source
2+
* @name Unsafe URL forward or dispatch from remote source
33
* @description URL forward or dispatch based on unvalidated user-input
44
* may cause file information disclosure.
55
* @kind path-problem
@@ -13,127 +13,8 @@
1313
import java
1414
import UnsafeUrlForward
1515
import semmle.code.java.dataflow.FlowSources
16-
import semmle.code.java.frameworks.Servlets
17-
import semmle.code.java.controlflow.Guards
18-
import semmle.code.java.dataflow.NullGuards
1916
import DataFlow::PathGraph
2017

21-
/**
22-
* Holds if `ma` is a call to a method that checks exact match of string.
23-
*/
24-
predicate isExactStringPathMatch(MethodAccess ma) {
25-
ma.getMethod().getDeclaringType() instanceof TypeString and
26-
ma.getMethod().getName() = ["equals", "equalsIgnoreCase"]
27-
}
28-
29-
/**
30-
* Holds if `ma` is a call to a method that checks a path string.
31-
*/
32-
predicate isStringPathMatch(MethodAccess ma) {
33-
ma.getMethod().getDeclaringType() instanceof TypeString and
34-
ma.getMethod().getName() =
35-
["contains", "startsWith", "matches", "regionMatches", "indexOf", "lastIndexOf"]
36-
}
37-
38-
/**
39-
* Holds if `ma` is a call to a method of `java.nio.file.Path` that checks a path.
40-
*/
41-
predicate isFilePathMatch(MethodAccess ma) {
42-
ma.getMethod().getDeclaringType() instanceof TypePath and
43-
ma.getMethod().getName() = "startsWith"
44-
}
45-
46-
/**
47-
* Holds if `ma` protects against path traversal, by either:
48-
* * looking for the literal `..`
49-
* * performing path normalization
50-
*/
51-
predicate isPathTraversalCheck(MethodAccess ma, Expr checked) {
52-
ma.getMethod().getDeclaringType() instanceof TypeString and
53-
ma.getMethod().hasName(["contains", "indexOf"]) and
54-
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = ".." and
55-
ma.(Guard).controls(checked.getBasicBlock(), false)
56-
or
57-
ma.getMethod() instanceof PathNormalizeMethod and
58-
checked = ma
59-
}
60-
61-
/**
62-
* Holds if `ma` protects against double URL encoding, by either:
63-
* * looking for the literal `%`
64-
* * performing URL decoding
65-
*/
66-
predicate isURLEncodingCheck(MethodAccess ma, Expr checked) {
67-
// Search the special character `%` used in url encoding
68-
ma.getMethod().getDeclaringType() instanceof TypeString and
69-
ma.getMethod().hasName(["contains", "indexOf"]) and
70-
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "%" and
71-
ma.(Guard).controls(checked.getBasicBlock(), false)
72-
or
73-
// Call to `URLDecoder` assuming the implementation handles double encoding correctly
74-
ma.getMethod().getDeclaringType().hasQualifiedName("java.net", "URLDecoder") and
75-
ma.getMethod().hasName("decode") and
76-
checked = ma
77-
}
78-
79-
/** The Java method `normalize` of `java.nio.file.Path`. */
80-
class PathNormalizeMethod extends Method {
81-
PathNormalizeMethod() {
82-
this.getDeclaringType().hasQualifiedName("java.nio.file", "Path") and
83-
this.hasName("normalize")
84-
}
85-
}
86-
87-
private predicate isDisallowedWord(CompileTimeConstantExpr word) {
88-
word.getStringValue().matches(["%WEB-INF%", "%META-INF%", "%..%"])
89-
}
90-
91-
private predicate isAllowListCheck(MethodAccess ma) {
92-
(isStringPathMatch(ma) or isFilePathMatch(ma)) and
93-
not isDisallowedWord(ma.getAnArgument())
94-
}
95-
96-
private predicate isDisallowListCheck(MethodAccess ma) {
97-
(isStringPathMatch(ma) or isFilePathMatch(ma)) and
98-
isDisallowedWord(ma.getAnArgument())
99-
}
100-
101-
/**
102-
* A guard that checks a path with the following methods:
103-
* 1. Exact string match
104-
* 2. Path matches allowed values (needs to protect against path traversal)
105-
* 3. Path matches disallowed values (needs to protect against URL encoding)
106-
*/
107-
private class PathMatchGuard extends DataFlow::BarrierGuard {
108-
PathMatchGuard() {
109-
isExactStringPathMatch(this) or isStringPathMatch(this) or isFilePathMatch(this)
110-
}
111-
112-
override predicate checks(Expr e, boolean branch) {
113-
e = this.(MethodAccess).getQualifier() and
114-
(
115-
isExactStringPathMatch(this) and
116-
branch = true
117-
or
118-
isAllowListCheck(this) and
119-
exists(MethodAccess ma, Expr checked | isPathTraversalCheck(ma, checked) |
120-
DataFlow::localExprFlow(checked, e)
121-
or
122-
ma.getParent*().(BinaryExpr) = this.(MethodAccess).getParent*()
123-
) and
124-
branch = true
125-
or
126-
isDisallowListCheck(this) and
127-
exists(MethodAccess ma, Expr checked | isURLEncodingCheck(ma, checked) |
128-
DataFlow::localExprFlow(checked, e)
129-
or
130-
ma.getParent*().(BinaryExpr) = this.(MethodAccess).getParent*()
131-
) and
132-
branch = false
133-
)
134-
}
135-
}
136-
13718
class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
13819
UnsafeUrlForwardFlowConfig() { this = "UnsafeUrlForwardFlowConfig" }
13920

@@ -154,7 +35,7 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
15435
override predicate isSanitizer(DataFlow::Node node) { node instanceof UnsafeUrlForwardSanitizer }
15536

15637
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
157-
guard instanceof PathMatchGuard
38+
guard instanceof UnsafeUrlForwardBarrierGuard
15839
}
15940

16041
override DataFlow::FlowFeature getAFeature() {

‎java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.qll‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import java
22
import DataFlow
3+
import semmle.code.java.controlflow.Guards
34
import semmle.code.java.dataflow.FlowSources
45
import semmle.code.java.frameworks.Servlets
56
import semmle.code.java.frameworks.spring.SpringWeb
@@ -30,6 +31,125 @@ private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
3031
FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
3132
}
3233

34+
/** A barrier guard that protects against URL forward vulnerabilities. */
35+
abstract class UnsafeUrlForwardBarrierGuard extends DataFlow::BarrierGuard { }
36+
37+
/**
38+
* Holds if `ma` is a call to a method that checks exact match of string.
39+
*/
40+
private predicate isExactStringPathMatch(MethodAccess ma) {
41+
ma.getMethod().getDeclaringType() instanceof TypeString and
42+
ma.getMethod().getName() = ["equals", "equalsIgnoreCase"]
43+
}
44+
45+
/**
46+
* Holds if `ma` is a call to a method that checks a path string.
47+
*/
48+
private predicate isStringPathMatch(MethodAccess ma) {
49+
ma.getMethod().getDeclaringType() instanceof TypeString and
50+
ma.getMethod().getName() =
51+
["contains", "startsWith", "matches", "regionMatches", "indexOf", "lastIndexOf"]
52+
}
53+
54+
/**
55+
* Holds if `ma` is a call to a method of `java.nio.file.Path` that checks a path.
56+
*/
57+
private predicate isFilePathMatch(MethodAccess ma) {
58+
ma.getMethod().getDeclaringType() instanceof TypePath and
59+
ma.getMethod().getName() = "startsWith"
60+
}
61+
62+
/**
63+
* Holds if `ma` protects against path traversal, by either:
64+
* * looking for the literal `..`
65+
* * performing path normalization
66+
*/
67+
private predicate isPathTraversalCheck(MethodAccess ma, Expr checked) {
68+
ma.getMethod().getDeclaringType() instanceof TypeString and
69+
ma.getMethod().hasName(["contains", "indexOf"]) and
70+
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = ".." and
71+
ma.(Guard).controls(checked.getBasicBlock(), false)
72+
or
73+
ma.getMethod() instanceof PathNormalizeMethod and
74+
checked = ma
75+
}
76+
77+
/**
78+
* Holds if `ma` protects against double URL encoding, by either:
79+
* * looking for the literal `%`
80+
* * performing URL decoding
81+
*/
82+
private predicate isURLEncodingCheck(MethodAccess ma, Expr checked) {
83+
// Search the special character `%` used in url encoding
84+
ma.getMethod().getDeclaringType() instanceof TypeString and
85+
ma.getMethod().hasName(["contains", "indexOf"]) and
86+
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "%" and
87+
ma.(Guard).controls(checked.getBasicBlock(), false)
88+
or
89+
// Call to `URLDecoder` assuming the implementation handles double encoding correctly
90+
ma.getMethod().getDeclaringType().hasQualifiedName("java.net", "URLDecoder") and
91+
ma.getMethod().hasName("decode") and
92+
checked = ma
93+
}
94+
95+
/** The Java method `normalize` of `java.nio.file.Path`. */
96+
private class PathNormalizeMethod extends Method {
97+
PathNormalizeMethod() {
98+
this.getDeclaringType().hasQualifiedName("java.nio.file", "Path") and
99+
this.hasName("normalize")
100+
}
101+
}
102+
103+
private predicate isDisallowedWord(CompileTimeConstantExpr word) {
104+
word.getStringValue().matches(["%WEB-INF%", "%META-INF%", "%..%"])
105+
}
106+
107+
private predicate isAllowListCheck(MethodAccess ma) {
108+
(isStringPathMatch(ma) or isFilePathMatch(ma)) and
109+
not isDisallowedWord(ma.getAnArgument())
110+
}
111+
112+
private predicate isDisallowListCheck(MethodAccess ma) {
113+
(isStringPathMatch(ma) or isFilePathMatch(ma)) and
114+
isDisallowedWord(ma.getAnArgument())
115+
}
116+
117+
/**
118+
* A guard that checks a path with the following methods:
119+
* 1. Exact string match
120+
* 2. Path matches allowed values (needs to protect against path traversal)
121+
* 3. Path matches disallowed values (needs to protect against URL encoding)
122+
*/
123+
private class PathMatchGuard extends UnsafeUrlForwardBarrierGuard {
124+
PathMatchGuard() {
125+
isExactStringPathMatch(this) or isStringPathMatch(this) or isFilePathMatch(this)
126+
}
127+
128+
override predicate checks(Expr e, boolean branch) {
129+
e = this.(MethodAccess).getQualifier() and
130+
(
131+
isExactStringPathMatch(this) and
132+
branch = true
133+
or
134+
isAllowListCheck(this) and
135+
exists(MethodAccess ma, Expr checked | isPathTraversalCheck(ma, checked) |
136+
DataFlow::localExprFlow(checked, e)
137+
or
138+
ma.getParent*().(BinaryExpr) = this.(MethodAccess).getParent*()
139+
) and
140+
branch = true
141+
or
142+
isDisallowListCheck(this) and
143+
exists(MethodAccess ma, Expr checked | isURLEncodingCheck(ma, checked) |
144+
DataFlow::localExprFlow(checked, e)
145+
or
146+
ma.getParent*().(BinaryExpr) = this.(MethodAccess).getParent*()
147+
) and
148+
branch = false
149+
)
150+
}
151+
}
152+
33153
abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
34154

35155
/** An argument to `getRequestDispatcher`. */

0 commit comments

Comments
 (0)