Skip to content

Commit 7029802

Browse files
committed
Add sinks for getClass() and getClassLoader()
1 parent eccd97c commit 7029802

File tree

4 files changed

+127
-14
lines changed

4 files changed

+127
-14
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// BAD: no URI validation
22
URL url = servletContext.getResource(requestUrl);
3+
url = getClass().getResource(requestUrl);
34
InputStream in = url.openStream();
45

56
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
7+
in = getClass().getClassLoader().getResourceAsStream(requestPath);
68

79
// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
810
// (alternatively use `Path.normalize` instead of checking for `..`)

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,22 @@ private class RequestDispatcherSink extends UnsafeUrlForwardSink {
1919
}
2020
}
2121

22+
/** The `getResource` and `getResourceAsStream` methods of `Class`. */
23+
class GetClassResourceMethod extends Method {
24+
GetClassResourceMethod() {
25+
this.getSourceDeclaration().getDeclaringType().hasQualifiedName("java.lang", "Class") and
26+
this.hasName(["getResource", "getResourceAsStream"])
27+
}
28+
}
29+
30+
/** The `getResource` and `getResourceAsStream` methods of `ClassLoader`. */
31+
class GetClassLoaderResourceMethod extends Method {
32+
GetClassLoaderResourceMethod() {
33+
this.getDeclaringType().hasQualifiedName("java.lang", "ClassLoader") and
34+
this.hasName(["getResource", "getResourceAsStream"])
35+
}
36+
}
37+
2238
/** The JBoss class `FileResourceManager`. */
2339
class FileResourceManager extends RefType {
2440
FileResourceManager() {
@@ -54,6 +70,8 @@ private class GetResourceSink extends UnsafeUrlForwardSink {
5470
(
5571
ma.getMethod() instanceof GetServletResourceMethod or
5672
ma.getMethod() instanceof GetFacesResourceMethod or
73+
ma.getMethod() instanceof GetClassResourceMethod or
74+
ma.getMethod() instanceof GetClassLoaderResourceMethod or
5775
ma.getMethod() instanceof GetWildflyResourceMethod or
5876
ma.getMethod() instanceof GetVirtualFileMethod
5977
) and

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

Lines changed: 91 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
package com.example;
2+
13
import java.io.InputStream;
24
import java.io.IOException;
35
import java.nio.file.Path;
@@ -13,6 +15,8 @@
1315
import javax.servlet.ServletContext;
1416

1517
public class UnsafeResourceGet extends HttpServlet {
18+
private static final String BASE_PATH = "/pages";
19+
1620
@Override
1721
// BAD: getResource constructed from `ServletContext` without input validation
1822
protected void doGet(HttpServletRequest request, HttpServletResponse response)
@@ -23,6 +27,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
2327
ServletConfig cfg = getServletConfig();
2428
ServletContext sc = cfg.getServletContext();
2529

30+
// A sample request /fake.jsp/../WEB-INF/web.xml can load the web.xml file
2631
URL url = sc.getResource(requestUrl);
2732

2833
InputStream in = url.openStream();
@@ -43,13 +48,15 @@ protected void doGetGood(HttpServletRequest request, HttpServletResponse respons
4348
ServletContext sc = cfg.getServletContext();
4449

4550
Path path = Paths.get(requestUrl).normalize().toRealPath();
46-
URL url = sc.getResource(path.toString());
51+
if (path.startsWith(BASE_PATH)) {
52+
URL url = sc.getResource(path.toString());
4753

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);
54+
InputStream in = url.openStream();
55+
byte[] buf = new byte[4 * 1024]; // 4K buffer
56+
int bytesRead;
57+
while ((bytesRead = in.read(buf)) != -1) {
58+
out.write(buf, 0, bytesRead);
59+
}
5360
}
5461
}
5562

@@ -63,6 +70,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
6370
ServletConfig cfg = getServletConfig();
6471
ServletContext sc = cfg.getServletContext();
6572

73+
// A sample request /fake.jsp/../WEB-INF/web.xml can load the web.xml file
6674
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
6775
byte[] buf = new byte[4 * 1024]; // 4K buffer
6876
int bytesRead;
@@ -89,4 +97,81 @@ protected void doPostGood(HttpServletRequest request, HttpServletResponse respon
8997
}
9098
}
9199
}
100+
101+
@Override
102+
// BAD: getResource constructed from `Class` without input validation
103+
protected void doHead(HttpServletRequest request, HttpServletResponse response)
104+
throws ServletException, IOException {
105+
String requestUrl = request.getParameter("requestURL");
106+
ServletOutputStream out = response.getOutputStream();
107+
108+
// A sample request /fake.jsp/../../../WEB-INF/web.xml can load the web.xml file
109+
// Note the class is in two levels of subpackages and `Class.loadResource` starts from its own directory
110+
URL url = getClass().getResource(requestUrl);
111+
112+
InputStream in = url.openStream();
113+
byte[] buf = new byte[4 * 1024]; // 4K buffer
114+
int bytesRead;
115+
while ((bytesRead = in.read(buf)) != -1) {
116+
out.write(buf, 0, bytesRead);
117+
}
118+
}
119+
120+
// GOOD: getResource constructed from `Class` with input validation
121+
protected void doHeadGood(HttpServletRequest request, HttpServletResponse response)
122+
throws ServletException, IOException {
123+
String requestUrl = request.getParameter("requestURL");
124+
ServletOutputStream out = response.getOutputStream();
125+
126+
Path path = Paths.get(requestUrl).normalize().toRealPath();
127+
if (path.startsWith(BASE_PATH)) {
128+
URL url = getClass().getResource(path.toString());
129+
130+
InputStream in = url.openStream();
131+
byte[] buf = new byte[4 * 1024]; // 4K buffer
132+
int bytesRead;
133+
while ((bytesRead = in.read(buf)) != -1) {
134+
out.write(buf, 0, bytesRead);
135+
}
136+
}
137+
}
138+
139+
@Override
140+
// BAD: getResourceAsStream constructed from `ClassLoader` without input validation
141+
protected void doPut(HttpServletRequest request, HttpServletResponse response)
142+
throws ServletException, IOException {
143+
String requestPath = request.getParameter("requestPath");
144+
ServletOutputStream out = response.getOutputStream();
145+
146+
ServletConfig cfg = getServletConfig();
147+
ServletContext sc = cfg.getServletContext();
148+
149+
// A sample request /fake.jsp/../../../WEB-INF/web.xml can load the web.xml file
150+
// Note the class is in two levels of subpackages and `ClassLoader.getResourceAsStream` starts from its own directory
151+
InputStream in = getClass().getClassLoader().getResourceAsStream(requestPath);
152+
byte[] buf = new byte[4 * 1024]; // 4K buffer
153+
int bytesRead;
154+
while ((bytesRead = in.read(buf)) != -1) {
155+
out.write(buf, 0, bytesRead);
156+
}
157+
}
158+
159+
// GOOD: getResourceAsStream constructed from `ClassLoader` with input validation
160+
protected void doPutGood(HttpServletRequest request, HttpServletResponse response)
161+
throws ServletException, IOException {
162+
String requestPath = request.getParameter("requestPath");
163+
ServletOutputStream out = response.getOutputStream();
164+
165+
ServletConfig cfg = getServletConfig();
166+
ServletContext sc = cfg.getServletContext();
167+
168+
if (!requestPath.contains("..") && requestPath.startsWith("/trusted")) {
169+
InputStream in = getClass().getClassLoader().getResourceAsStream(requestPath);
170+
byte[] buf = new byte[4 * 1024]; // 4K buffer
171+
int bytesRead;
172+
while ((bytesRead = in.read(buf)) != -1) {
173+
out.write(buf, 0, bytesRead);
174+
}
175+
}
176+
}
92177
}

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
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 |
3+
| UnsafeResourceGet.java:24:23:24:56 | getParameter(...) : String | UnsafeResourceGet.java:31:28:31:37 | requestUrl |
4+
| UnsafeResourceGet.java:67:24:67:58 | getParameter(...) : String | UnsafeResourceGet.java:74:68:74:78 | requestPath |
5+
| UnsafeResourceGet.java:105:23:105:56 | getParameter(...) : String | UnsafeResourceGet.java:110:36:110:45 | requestUrl |
6+
| UnsafeResourceGet.java:143:24:143:58 | getParameter(...) : String | UnsafeResourceGet.java:151:68:151:78 | requestPath |
57
| UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL |
68
| UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:48:56:48:64 | returnURL |
79
| UnsafeServletRequestDispatch.java:71:17:71:44 | getParameter(...) : String | UnsafeServletRequestDispatch.java:76:53:76:56 | path |
@@ -21,10 +23,14 @@ edges
2123
nodes
2224
| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | semmle.label | getServletPath(...) : String |
2325
| 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 |
26+
| UnsafeResourceGet.java:24:23:24:56 | getParameter(...) : String | semmle.label | getParameter(...) : String |
27+
| UnsafeResourceGet.java:31:28:31:37 | requestUrl | semmle.label | requestUrl |
28+
| UnsafeResourceGet.java:67:24:67:58 | getParameter(...) : String | semmle.label | getParameter(...) : String |
29+
| UnsafeResourceGet.java:74:68:74:78 | requestPath | semmle.label | requestPath |
30+
| UnsafeResourceGet.java:105:23:105:56 | getParameter(...) : String | semmle.label | getParameter(...) : String |
31+
| UnsafeResourceGet.java:110:36:110:45 | requestUrl | semmle.label | requestUrl |
32+
| UnsafeResourceGet.java:143:24:143:58 | getParameter(...) : String | semmle.label | getParameter(...) : String |
33+
| UnsafeResourceGet.java:151:68:151:78 | requestPath | semmle.label | requestPath |
2834
| UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | semmle.label | getParameter(...) : String |
2935
| UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL | semmle.label | returnURL |
3036
| UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | semmle.label | getParameter(...) : String |
@@ -55,8 +61,10 @@ nodes
5561
subpaths
5662
#select
5763
| 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 |
64+
| UnsafeResourceGet.java:31:28:31:37 | requestUrl | UnsafeResourceGet.java:24:23:24:56 | getParameter(...) : String | UnsafeResourceGet.java:31:28:31:37 | requestUrl | Potentially untrusted URL forward due to $@. | UnsafeResourceGet.java:24:23:24:56 | getParameter(...) | user-provided value |
65+
| UnsafeResourceGet.java:74:68:74:78 | requestPath | UnsafeResourceGet.java:67:24:67:58 | getParameter(...) : String | UnsafeResourceGet.java:74:68:74:78 | requestPath | Potentially untrusted URL forward due to $@. | UnsafeResourceGet.java:67:24:67:58 | getParameter(...) | user-provided value |
66+
| UnsafeResourceGet.java:110:36:110:45 | requestUrl | UnsafeResourceGet.java:105:23:105:56 | getParameter(...) : String | UnsafeResourceGet.java:110:36:110:45 | requestUrl | Potentially untrusted URL forward due to $@. | UnsafeResourceGet.java:105:23:105:56 | getParameter(...) | user-provided value |
67+
| UnsafeResourceGet.java:151:68:151:78 | requestPath | UnsafeResourceGet.java:143:24:143:58 | getParameter(...) : String | UnsafeResourceGet.java:151:68:151:78 | requestPath | Potentially untrusted URL forward due to $@. | UnsafeResourceGet.java:143:24:143:58 | getParameter(...) | user-provided value |
6068
| 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 |
6169
| 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 |
6270
| 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)