@@ -18,24 +18,33 @@ import semmle.code.java.controlflow.Guards
1818import DataFlow:: PathGraph
1919
2020/**
21- * Holds if `ma` is a method call of matching with a path string, probably a whitelisted one.
21+ * Holds if `ma` is a call to a method that checks exact match of string, probably a whitelisted one.
22+ */
23+ predicate isExactStringPathMatch ( MethodAccess ma ) {
24+ ma .getMethod ( ) .getDeclaringType ( ) instanceof TypeString and
25+ ma .getMethod ( ) .getName ( ) = [ "equals" , "equalsIgnoreCase" ]
26+ }
27+
28+ /**
29+ * Holds if `ma` is a call to a method that checks a path string, probably a whitelisted one.
2230 */
2331predicate isStringPathMatch ( MethodAccess ma ) {
2432 ma .getMethod ( ) .getDeclaringType ( ) instanceof TypeString and
25- ma .getMethod ( ) .getName ( ) = [ "startsWith" , "matches" , "regionMatches" ]
33+ ma .getMethod ( ) .getName ( ) =
34+ [ "contains" , "startsWith" , "matches" , "regionMatches" , "indexOf" , "lastIndexOf" ]
2635}
2736
2837/**
29- * Holds if `ma` is a method call of `java.nio.file.Path` which matches with another
30- * path, probably a whitelisted one.
38+ * Holds if `ma` is a call to a method of `java.nio.file.Path` that checks a path, probably
39+ * a whitelisted one.
3140 */
3241predicate isFilePathMatch ( MethodAccess ma ) {
3342 ma .getMethod ( ) .getDeclaringType ( ) instanceof TypePath and
3443 ma .getMethod ( ) .getName ( ) = "startsWith"
3544}
3645
3746/**
38- * Holds if `ma` is a method call that checks an input doesn't match using the `!`
47+ * Holds if `ma` is a call to a method that checks an input doesn't match using the `!`
3948 * logical negation expression.
4049 */
4150predicate checkNoPathMatch ( MethodAccess ma ) {
@@ -46,7 +55,7 @@ predicate checkNoPathMatch(MethodAccess ma) {
4655}
4756
4857/**
49- * Holds if `ma` is a method call to check special characters `..` used in path traversal.
58+ * Holds if `ma` is a call to a method that checks special characters `..` used in path traversal.
5059 */
5160predicate isPathTraversalCheck ( MethodAccess ma ) {
5261 ma .getMethod ( ) .getDeclaringType ( ) instanceof TypeString and
@@ -55,7 +64,7 @@ predicate isPathTraversalCheck(MethodAccess ma) {
5564}
5665
5766/**
58- * Holds if `ma` is a method call to decode a url string or check url encoding.
67+ * Holds if `ma` is a call to a method that decodes a URL string or check URL encoding.
5968 */
6069predicate isPathDecoding ( MethodAccess ma ) {
6170 // Search the special character `%` used in url encoding
@@ -68,44 +77,65 @@ predicate isPathDecoding(MethodAccess ma) {
6877 ma .getMethod ( ) .hasName ( "decode" )
6978}
7079
71- private class PathMatchSanitizer extends DataFlow:: Node {
80+ /** The Java method `normalize` of `java.nio.file.Path`. */
81+ class PathNormalizeMethod extends Method {
82+ PathNormalizeMethod ( ) {
83+ this .getDeclaringType ( ) .hasQualifiedName ( "java.nio.file" , "Path" ) and
84+ this .hasName ( "normalize" )
85+ }
86+ }
87+
88+ /**
89+ * Sanitizer to check the following scenarios in a web application:
90+ * 1. Exact string match
91+ * 2. String startsWith or match check with path traversal validation
92+ * 3. String not startsWith or not match check with decoding processing
93+ * 4. java.nio.file.Path startsWith check having path normalization
94+ */
95+ private class PathMatchSanitizer extends DataFlow:: BarrierGuard {
7296 PathMatchSanitizer ( ) {
73- exists ( MethodAccess ma |
74- (
75- isStringPathMatch ( ma ) and
76- exists ( MethodAccess ma2 |
77- isPathTraversalCheck ( ma2 ) and
78- ma .getQualifier ( ) .( VarAccess ) .getVariable ( ) .getAnAccess ( ) = ma2 .getQualifier ( )
79- )
80- or
81- isFilePathMatch ( ma )
82- ) and
83- (
84- not checkNoPathMatch ( ma )
85- or
86- // non-match check needs decoding e.g. !path.startsWith("/WEB-INF/") won't detect /%57EB-INF/web.xml, which will be decoded and served by RequestDispatcher
87- checkNoPathMatch ( ma ) and
88- exists ( MethodAccess ma2 |
89- isPathDecoding ( ma2 ) and
90- ma .getQualifier ( ) .( VarAccess ) .getVariable ( ) .getAnAccess ( ) = ma2 .getQualifier ( )
91- )
92- ) and
93- this .asExpr ( ) = ma .getQualifier ( )
97+ isExactStringPathMatch ( this )
98+ or
99+ isStringPathMatch ( this ) and
100+ not checkNoPathMatch ( this ) and
101+ exists ( MethodAccess tma |
102+ isPathTraversalCheck ( tma ) and
103+ DataFlow:: localExprFlow ( this .( MethodAccess ) .getQualifier ( ) , tma .getQualifier ( ) )
104+ )
105+ or
106+ checkNoPathMatch ( this ) and
107+ exists ( MethodAccess dma |
108+ isPathDecoding ( dma ) and
109+ DataFlow:: localExprFlow ( dma , this .( MethodAccess ) .getQualifier ( ) )
110+ )
111+ or
112+ isFilePathMatch ( this ) and
113+ exists ( MethodAccess pma |
114+ pma .getMethod ( ) instanceof PathNormalizeMethod and
115+ DataFlow:: localExprFlow ( pma , this .( MethodAccess ) .getQualifier ( ) )
116+ )
117+ }
118+
119+ override predicate checks ( Expr e , boolean branch ) {
120+ e = this .( MethodAccess ) .getQualifier ( ) and
121+ (
122+ branch = true and not checkNoPathMatch ( this )
123+ or
124+ branch = false and checkNoPathMatch ( this )
94125 )
95126 }
96127}
97128
98129/**
99- * Holds if `ma` is a method call to check string content, which means an input string is not
130+ * Holds if `ma` is a call to a method that checks string content, which means an input string is not
100131 * blindly trusted and helps to reduce FPs.
101132 */
102133predicate checkStringContent ( MethodAccess ma , Expr expr ) {
103134 ma .getMethod ( ) .getDeclaringType ( ) instanceof TypeString and
104135 ma .getMethod ( )
105136 .hasName ( [
106- "charAt" , "contains" , "equals" , "equalsIgnoreCase" , "getBytes" , "getChars" , "indexOf" ,
107- "lastIndexOf" , "length" , "matches" , "regionMatches" , "replace" , "replaceAll" ,
108- "replaceFirst" , "substring"
137+ "charAt" , "getBytes" , "getChars" , "length" , "replace" , "replaceAll" , "replaceFirst" ,
138+ "substring"
109139 ] ) and
110140 expr = ma .getQualifier ( )
111141 or
@@ -145,23 +175,6 @@ private class NullOrEmptyCheckSanitizer extends DataFlow::Node {
145175 NullOrEmptyCheckSanitizer ( ) { isNullOrEmptyCheck ( this .asExpr ( ) ) }
146176}
147177
148- /** Holds if `ma` is a virtual method call of Map::get or Object::toString. */
149- predicate isVirtualMethod ( MethodAccess ma , Expr expr ) {
150- ma .getMethod ( ) .getDeclaringType ( ) instanceof TypeObject and
151- ma .getMethod ( ) .hasName ( "toString" ) and
152- ( expr = ma or expr = ma .getQualifier ( ) )
153- or
154- (
155- ma .getMethod ( ) .getDeclaringType ( ) .getASupertype * ( ) .hasQualifiedName ( "java.util" , "Map" ) and
156- ma .getMethod ( ) .hasName ( [ "get" , "getOrDefault" ] )
157- ) and
158- ( expr = ma or expr = ma .getAnArgument ( ) )
159- }
160-
161- private class VirtualMethodSanitizer extends DataFlow:: Node {
162- VirtualMethodSanitizer ( ) { exists ( MethodAccess ma | isVirtualMethod ( ma , this .asExpr ( ) ) ) }
163- }
164-
165178class UnsafeUrlForwardFlowConfig extends TaintTracking:: Configuration {
166179 UnsafeUrlForwardFlowConfig ( ) { this = "UnsafeUrlForwardFlowConfig" }
167180
@@ -181,10 +194,16 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
181194
182195 override predicate isSanitizer ( DataFlow:: Node node ) {
183196 node instanceof UnsafeUrlForwardSanitizer or
184- node instanceof PathMatchSanitizer or
185197 node instanceof StringOperationSanitizer or
186- node instanceof NullOrEmptyCheckSanitizer or
187- node instanceof VirtualMethodSanitizer
198+ node instanceof NullOrEmptyCheckSanitizer
199+ }
200+
201+ override predicate isSanitizerGuard ( DataFlow:: BarrierGuard guard ) {
202+ guard instanceof PathMatchSanitizer
203+ }
204+
205+ override DataFlow:: FlowFeature getAFeature ( ) {
206+ result instanceof DataFlow:: FeatureHasSourceCallContext
188207 }
189208}
190209
0 commit comments