Skip to content

Commit eccd97c

Browse files
luchua-bcluchua-bc
committed
Query to detect unsafe getResource calls in Java EE applications
1 parent 626770a commit eccd97c

File tree

7 files changed

+207
-0
lines changed

7 files changed

+207
-0
lines changed

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

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

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

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

‎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>
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.qll

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import java
2+
import experimental.semmle.code.java.frameworks.Jsf
23
import semmle.code.java.dataflow.FlowSources
34
private import semmle.code.java.dataflow.StringPrefixes
45

@@ -18,6 +19,49 @@ private class RequestDispatcherSink extends UnsafeUrlForwardSink {
1819
}
1920
}
2021

22+
/** The JBoss class `FileResourceManager`. */
23+
class FileResourceManager extends RefType {
24+
FileResourceManager() {
25+
this.hasQualifiedName("io.undertow.server.handlers.resource", "FileResourceManager")
26+
}
27+
}
28+
29+
/** The JBoss method `getResource` of `FileResourceManager`. */
30+
class GetWildflyResourceMethod extends Method {
31+
GetWildflyResourceMethod() {
32+
this.getDeclaringType().getASupertype*() instanceof FileResourceManager and
33+
this.hasName("getResource")
34+
}
35+
}
36+
37+
/** The JBoss class `VirtualFile`. */
38+
class VirtualFile extends RefType {
39+
VirtualFile() { this.hasQualifiedName("org.jboss.vfs", "VirtualFile") }
40+
}
41+
42+
/** The JBoss method `getChild` of `FileResourceManager`. */
43+
class GetVirtualFileMethod extends Method {
44+
GetVirtualFileMethod() {
45+
this.getDeclaringType().getASupertype*() instanceof VirtualFile and
46+
this.hasName("getChild")
47+
}
48+
}
49+
50+
/** An argument to `getResource()` or `getResourceAsStream()`. */
51+
private class GetResourceSink extends UnsafeUrlForwardSink {
52+
GetResourceSink() {
53+
exists(MethodAccess ma |
54+
(
55+
ma.getMethod() instanceof GetServletResourceMethod or
56+
ma.getMethod() instanceof GetFacesResourceMethod or
57+
ma.getMethod() instanceof GetWildflyResourceMethod or
58+
ma.getMethod() instanceof GetVirtualFileMethod
59+
) and
60+
ma.getArgument(0) = this.asExpr()
61+
)
62+
}
63+
}
64+
2165
/** An argument to `new ModelAndView` or `ModelAndView.setViewName`. */
2266
private class SpringModelAndViewSink extends UnsafeUrlForwardSink {
2367
SpringModelAndViewSink() {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Provides classes and predicates for working with the Java Server Faces (JSF).
3+
*/
4+
5+
import semmle.code.java.Type
6+
7+
/**
8+
* The JSF class `FacesContext` 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 methods `getResource()` and `getResourceAsStream()` declared in JSF `ExternalContext`.
18+
*/
19+
class GetFacesResourceMethod extends Method {
20+
GetFacesResourceMethod() {
21+
this.getDeclaringType().getASupertype*() instanceof ExternalContext and
22+
this.hasName(["getResource", "getResourceAsStream"])
23+
}
24+
}

‎java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceGet.java

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import java.io.InputStream;
2+
import java.io.IOException;
3+
import java.nio.file.Path;
4+
import java.nio.file.Paths;
5+
import java.net.URL;
6+
7+
import javax.servlet.http.HttpServlet;
8+
import javax.servlet.http.HttpServletRequest;
9+
import javax.servlet.http.HttpServletResponse;
10+
import javax.servlet.ServletOutputStream;
11+
import javax.servlet.ServletException;
12+
import javax.servlet.ServletConfig;
13+
import javax.servlet.ServletContext;
14+
15+
public class UnsafeResourceGet extends HttpServlet {
16+
@Override
17+
// BAD: getResource constructed from `ServletContext` without input validation
18+
protected void doGet(HttpServletRequest request, HttpServletResponse response)
19+
throws ServletException, IOException {
20+
String requestUrl = request.getParameter("requestURL");
21+
ServletOutputStream out = response.getOutputStream();
22+
23+
ServletConfig cfg = getServletConfig();
24+
ServletContext sc = cfg.getServletContext();
25+
26+
URL url = sc.getResource(requestUrl);
27+
28+
InputStream in = url.openStream();
29+
byte[] buf = new byte[4 * 1024]; // 4K buffer
30+
int bytesRead;
31+
while ((bytesRead = in.read(buf)) != -1) {
32+
out.write(buf, 0, bytesRead);
33+
}
34+
}
35+
36+
// GOOD: getResource constructed from `ServletContext` with input validation
37+
protected void doGetGood(HttpServletRequest request, HttpServletResponse response)
38+
throws ServletException, IOException {
39+
String requestUrl = request.getParameter("requestURL");
40+
ServletOutputStream out = response.getOutputStream();
41+
42+
ServletConfig cfg = getServletConfig();
43+
ServletContext sc = cfg.getServletContext();
44+
45+
Path path = Paths.get(requestUrl).normalize().toRealPath();
46+
URL url = sc.getResource(path.toString());
47+
48+
InputStream in = url.openStream();
49+
byte[] buf = new byte[4 * 1024]; // 4K buffer
50+
int bytesRead;
51+
while ((bytesRead = in.read(buf)) != -1) {
52+
out.write(buf, 0, bytesRead);
53+
}
54+
}
55+
56+
@Override
57+
// BAD: getResourceAsStream constructed from `ServletContext` without input validation
58+
protected void doPost(HttpServletRequest request, HttpServletResponse response)
59+
throws ServletException, IOException {
60+
String requestPath = request.getParameter("requestPath");
61+
ServletOutputStream out = response.getOutputStream();
62+
63+
ServletConfig cfg = getServletConfig();
64+
ServletContext sc = cfg.getServletContext();
65+
66+
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
67+
byte[] buf = new byte[4 * 1024]; // 4K buffer
68+
int bytesRead;
69+
while ((bytesRead = in.read(buf)) != -1) {
70+
out.write(buf, 0, bytesRead);
71+
}
72+
}
73+
74+
// GOOD: getResourceAsStream constructed from `ServletContext` with input validation
75+
protected void doPostGood(HttpServletRequest request, HttpServletResponse response)
76+
throws ServletException, IOException {
77+
String requestPath = request.getParameter("requestPath");
78+
ServletOutputStream out = response.getOutputStream();
79+
80+
ServletConfig cfg = getServletConfig();
81+
ServletContext sc = cfg.getServletContext();
82+
83+
if (!requestPath.contains("..") && requestPath.startsWith("/trusted")) {
84+
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
85+
byte[] buf = new byte[4 * 1024]; // 4K buffer
86+
int bytesRead;
87+
while ((bytesRead = in.read(buf)) != -1) {
88+
out.write(buf, 0, bytesRead);
89+
}
90+
}
91+
}
92+
}

‎java/ql/test/experimental/query-tests/security/CWE-552/UnsafeUrlForward.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
edges
22
| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | UnsafeRequestPath.java:23:33:23:36 | path |
3+
| UnsafeResourceGet.java:20:23:20:56 | getParameter(...) : String | UnsafeResourceGet.java:26:28:26:37 | requestUrl |
4+
| UnsafeResourceGet.java:60:24:60:58 | getParameter(...) : String | UnsafeResourceGet.java:66:68:66:78 | requestPath |
35
| UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL |
46
| UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:48:56:48:64 | returnURL |
57
| UnsafeServletRequestDispatch.java:71:17:71:44 | getParameter(...) : String | UnsafeServletRequestDispatch.java:76:53:76:56 | path |
@@ -19,6 +21,10 @@ edges
1921
nodes
2022
| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | semmle.label | getServletPath(...) : String |
2123
| UnsafeRequestPath.java:23:33:23:36 | path | semmle.label | path |
24+
| UnsafeResourceGet.java:20:23:20:56 | getParameter(...) : String | semmle.label | getParameter(...) : String |
25+
| UnsafeResourceGet.java:26:28:26:37 | requestUrl | semmle.label | requestUrl |
26+
| UnsafeResourceGet.java:60:24:60:58 | getParameter(...) : String | semmle.label | getParameter(...) : String |
27+
| UnsafeResourceGet.java:66:68:66:78 | requestPath | semmle.label | requestPath |
2228
| UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | semmle.label | getParameter(...) : String |
2329
| UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL | semmle.label | returnURL |
2430
| UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | semmle.label | getParameter(...) : String |
@@ -49,6 +55,8 @@ nodes
4955
subpaths
5056
#select
5157
| UnsafeRequestPath.java:23:33:23:36 | path | UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | UnsafeRequestPath.java:23:33:23:36 | path | Potentially untrusted URL forward due to $@. | UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) | user-provided value |
58+
| UnsafeResourceGet.java:26:28:26:37 | requestUrl | UnsafeResourceGet.java:20:23:20:56 | getParameter(...) : String | UnsafeResourceGet.java:26:28:26:37 | requestUrl | Potentially untrusted URL forward due to $@. | UnsafeResourceGet.java:20:23:20:56 | getParameter(...) | user-provided value |
59+
| UnsafeResourceGet.java:66:68:66:78 | requestPath | UnsafeResourceGet.java:60:24:60:58 | getParameter(...) : String | UnsafeResourceGet.java:66:68:66:78 | requestPath | Potentially untrusted URL forward due to $@. | UnsafeResourceGet.java:60:24:60:58 | getParameter(...) | user-provided value |
5260
| UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL | UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL | Potentially untrusted URL forward due to $@. | UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) | user-provided value |
5361
| UnsafeServletRequestDispatch.java:48:56:48:64 | returnURL | UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:48:56:48:64 | returnURL | Potentially untrusted URL forward due to $@. | UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) | user-provided value |
5462
| UnsafeServletRequestDispatch.java:76:53:76:56 | path | UnsafeServletRequestDispatch.java:71:17:71:44 | getParameter(...) : String | UnsafeServletRequestDispatch.java:76:53:76:56 | path | Potentially untrusted URL forward due to $@. | UnsafeServletRequestDispatch.java:71:17:71:44 | getParameter(...) | user-provided value |

0 commit comments

Comments
 (0)