Skip to content

Commit b876431

Browse files
atorralbaatorralba
authored
Merge pull request #8706 from luchua-bc/java/unsafe-get-resource
Java: CWE-552 Add sources and sinks to to detect unsafe getResource calls in Java EE applications
2 parents b8fd07c + 920a7cd commit b876431

File tree

14 files changed

+668
-3
lines changed

14 files changed

+668
-3
lines changed

‎java/ql/lib/semmle/code/java/frameworks/Servlets.qll

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,3 +377,26 @@ class RequestDispatchMethod extends Method {
377377
this.hasName(["forward", "include"])
378378
}
379379
}
380+
381+
/**
382+
* The interface `javax.servlet.ServletContext`.
383+
*/
384+
class ServletContext extends RefType {
385+
ServletContext() { this.hasQualifiedName("javax.servlet", "ServletContext") }
386+
}
387+
388+
/** The `getResource` method of `ServletContext`. */
389+
class GetServletResourceMethod extends Method {
390+
GetServletResourceMethod() {
391+
this.getDeclaringType() instanceof ServletContext and
392+
this.hasName("getResource")
393+
}
394+
}
395+
396+
/** The `getResourceAsStream` method of `ServletContext`. */
397+
class GetServletResourceAsStreamMethod extends Method {
398+
GetServletResourceAsStreamMethod() {
399+
this.getDeclaringType() instanceof ServletContext and
400+
this.hasName("getResourceAsStream")
401+
}
402+
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// BAD: no URI validation
2+
URL url = request.getServletContext().getResource(requestUrl);
3+
url = getClass().getResource(requestUrl);
4+
InputStream in = url.openStream();
5+
6+
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
7+
in = getClass().getClassLoader().getResourceAsStream(requestPath);
8+
9+
// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
10+
// (alternatively use `Path.normalize` instead of checking for `..`)
11+
if (!requestPath.contains("..") && requestPath.startsWith("/trusted")) {
12+
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
13+
}
14+
15+
Path path = Paths.get(requestUrl).normalize().toRealPath();
16+
if (path.startsWith("/trusted")) {
17+
URL url = request.getServletContext().getResource(path.toString());
18+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ attacks. It also shows how to remedy the problem by validating the user input.
3636

3737
<sample src="UnsafeServletRequestDispatch.java" />
3838

39+
<p>The following examples show an HTTP request parameter or request path being used directly to
40+
retrieve a resource of a Java EE application without validating the input, which allows sensitive
41+
file exposure attacks. It also shows how to remedy the problem by validating the user input.
42+
</p>
43+
44+
<sample src="UnsafeResourceGet.java" />
45+
3946
</example>
4047
<references>
4148
<li>File Disclosure:
@@ -47,5 +54,8 @@ attacks. It also shows how to remedy the problem by validating the user input.
4754
<li>Micro Focus:
4855
<a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_j2ee">File Disclosure: J2EE</a>
4956
</li>
57+
<li>CVE-2015-5174:
58+
<a href="https://vuldb.com/?id.81084">Apache Tomcat 6.0/7.0/8.0/9.0 Servletcontext getResource/getResourceAsStream/getResourcePaths Path Traversal</a>
59+
</li>
5060
</references>
5161
</qhelp>

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import java
1414
import UnsafeUrlForward
1515
import semmle.code.java.dataflow.FlowSources
1616
import semmle.code.java.dataflow.TaintTracking
17+
import experimental.semmle.code.java.frameworks.Jsf
1718
import experimental.semmle.code.java.PathSanitizer
1819
import DataFlow::PathGraph
1920

@@ -43,6 +44,20 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
4344
override DataFlow::FlowFeature getAFeature() {
4445
result instanceof DataFlow::FeatureHasSourceCallContext
4546
}
47+
48+
override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
49+
exists(MethodAccess ma |
50+
(
51+
ma.getMethod() instanceof GetServletResourceMethod or
52+
ma.getMethod() instanceof GetFacesResourceMethod or
53+
ma.getMethod() instanceof GetClassResourceMethod or
54+
ma.getMethod() instanceof GetClassLoaderResourceMethod or
55+
ma.getMethod() instanceof GetWildflyResourceMethod
56+
) and
57+
ma.getArgument(0) = prev.asExpr() and
58+
ma = succ.asExpr()
59+
)
60+
}
4661
}
4762

4863
from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeUrlForwardFlowConfig conf

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

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import java
2+
private import experimental.semmle.code.java.frameworks.Jsf
23
private import semmle.code.java.dataflow.ExternalFlow
34
private import semmle.code.java.dataflow.FlowSources
45
private import semmle.code.java.dataflow.StringPrefixes
6+
private import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
57

68
/** A sink for unsafe URL forward vulnerabilities. */
79
abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
@@ -19,6 +21,84 @@ private class RequestDispatcherSink extends UnsafeUrlForwardSink {
1921
}
2022
}
2123

24+
/** The `getResource` method of `Class`. */
25+
class GetClassResourceMethod extends Method {
26+
GetClassResourceMethod() {
27+
this.getDeclaringType() instanceof TypeClass and
28+
this.hasName("getResource")
29+
}
30+
}
31+
32+
/** The `getResourceAsStream` method of `Class`. */
33+
class GetClassResourceAsStreamMethod extends Method {
34+
GetClassResourceAsStreamMethod() {
35+
this.getDeclaringType() instanceof TypeClass and
36+
this.hasName("getResourceAsStream")
37+
}
38+
}
39+
40+
/** The `getResource` method of `ClassLoader`. */
41+
class GetClassLoaderResourceMethod extends Method {
42+
GetClassLoaderResourceMethod() {
43+
this.getDeclaringType() instanceof ClassLoaderClass and
44+
this.hasName("getResource")
45+
}
46+
}
47+
48+
/** The `getResourceAsStream` method of `ClassLoader`. */
49+
class GetClassLoaderResourceAsStreamMethod extends Method {
50+
GetClassLoaderResourceAsStreamMethod() {
51+
this.getDeclaringType() instanceof ClassLoaderClass and
52+
this.hasName("getResourceAsStream")
53+
}
54+
}
55+
56+
/** The JBoss class `FileResourceManager`. */
57+
class FileResourceManager extends RefType {
58+
FileResourceManager() {
59+
this.hasQualifiedName("io.undertow.server.handlers.resource", "FileResourceManager")
60+
}
61+
}
62+
63+
/** The JBoss method `getResource` of `FileResourceManager`. */
64+
class GetWildflyResourceMethod extends Method {
65+
GetWildflyResourceMethod() {
66+
this.getDeclaringType().getASupertype*() instanceof FileResourceManager and
67+
this.hasName("getResource")
68+
}
69+
}
70+
71+
/** The JBoss class `VirtualFile`. */
72+
class VirtualFile extends RefType {
73+
VirtualFile() { this.hasQualifiedName("org.jboss.vfs", "VirtualFile") }
74+
}
75+
76+
/** The JBoss method `getChild` of `FileResourceManager`. */
77+
class GetVirtualFileChildMethod extends Method {
78+
GetVirtualFileChildMethod() {
79+
this.getDeclaringType().getASupertype*() instanceof VirtualFile and
80+
this.hasName("getChild")
81+
}
82+
}
83+
84+
/** An argument to `getResource()` or `getResourceAsStream()`. */
85+
private class GetResourceSink extends UnsafeUrlForwardSink {
86+
GetResourceSink() {
87+
sinkNode(this, "open-url")
88+
or
89+
exists(MethodAccess ma |
90+
(
91+
ma.getMethod() instanceof GetServletResourceAsStreamMethod or
92+
ma.getMethod() instanceof GetFacesResourceAsStreamMethod or
93+
ma.getMethod() instanceof GetClassResourceAsStreamMethod or
94+
ma.getMethod() instanceof GetClassLoaderResourceAsStreamMethod or
95+
ma.getMethod() instanceof GetVirtualFileChildMethod
96+
) and
97+
ma.getArgument(0) = this.asExpr()
98+
)
99+
}
100+
}
101+
22102
/** An argument to `new ModelAndView` or `ModelAndView.setViewName`. */
23103
private class SpringModelAndViewSink extends UnsafeUrlForwardSink {
24104
SpringModelAndViewSink() {
@@ -80,15 +160,18 @@ private class ServletGetPathSource extends SourceModelCsv {
80160
}
81161
}
82162

83-
/** Taint model related to `java.nio.file.Path`. */
163+
/** Taint model related to `java.nio.file.Path` and `io.undertow.server.handlers.resource.Resource`. */
84164
private class FilePathFlowStep extends SummaryModelCsv {
85165
override predicate row(string row) {
86166
row =
87167
[
88168
"java.nio.file;Paths;true;get;;;Argument[0..1];ReturnValue;taint",
89169
"java.nio.file;Path;true;resolve;;;Argument[-1..0];ReturnValue;taint",
90170
"java.nio.file;Path;true;normalize;;;Argument[-1];ReturnValue;taint",
91-
"java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint"
171+
"java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint",
172+
"io.undertow.server.handlers.resource;Resource;true;getFile;;;Argument[-1];ReturnValue;taint",
173+
"io.undertow.server.handlers.resource;Resource;true;getFilePath;;;Argument[-1];ReturnValue;taint",
174+
"io.undertow.server.handlers.resource;Resource;true;getPath;;;Argument[-1];ReturnValue;taint"
92175
]
93176
}
94177
}

‎java/ql/src/experimental/semmle/code/java/frameworks/Jsf.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* Provides classes and predicates for working with the Java Server Faces (JSF).
3+
*/
4+
5+
import java
6+
7+
/**
8+
* The JSF class `ExternalContext` for processing HTTP requests.
9+
*/
10+
class ExternalContext extends RefType {
11+
ExternalContext() {
12+
this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ExternalContext")
13+
}
14+
}
15+
16+
/**
17+
* The method `getResource()` declared in JSF `ExternalContext`.
18+
*/
19+
class GetFacesResourceMethod extends Method {
20+
GetFacesResourceMethod() {
21+
this.getDeclaringType().getASupertype*() instanceof ExternalContext and
22+
this.hasName("getResource")
23+
}
24+
}
25+
26+
/**
27+
* The method `getResourceAsStream()` declared in JSF `ExternalContext`.
28+
*/
29+
class GetFacesResourceAsStreamMethod extends Method {
30+
GetFacesResourceAsStreamMethod() {
31+
this.getDeclaringType().getASupertype*() instanceof ExternalContext and
32+
this.hasName("getResourceAsStream")
33+
}
34+
}

0 commit comments

Comments
 (0)