hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 09:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add sinks for getClass() and getClassLoader()

Browse Source
This commit is contained in:
luchua-bc
2022-04-11 21:03:48 +00:00
parent eccd97c7b7
commit 7029802f3b
4 changed files with 127 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/ql/src/experimental/Security/CWE/CWE-552/UnsafeResourceGet.java
View File

@@ -1,8 +1,10 @@
// BAD: no URI validation
URL url = servletContext.getResource(requestUrl);
url = getClass().getResource(requestUrl);
InputStream in = url.openStream();
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
in = getClass().getClassLoader().getResourceAsStream(requestPath);
// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
// (alternatively use `Path.normalize` instead of checking for `..`)

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

@@ -19,6 +19,22 @@ private class RequestDispatcherSink extends UnsafeUrlForwardSink {
}
}
/** The `getResource` and `getResourceAsStream` methods of `Class`. */
class GetClassResourceMethod extends Method {
GetClassResourceMethod() {
this.getSourceDeclaration().getDeclaringType().hasQualifiedName("java.lang", "Class") and
this.hasName(["getResource", "getResourceAsStream"])
}
}
/** The `getResource` and `getResourceAsStream` methods of `ClassLoader`. */
class GetClassLoaderResourceMethod extends Method {
GetClassLoaderResourceMethod() {
this.getDeclaringType().hasQualifiedName("java.lang", "ClassLoader") and
this.hasName(["getResource", "getResourceAsStream"])
}
}
/** The JBoss class `FileResourceManager`. */
class FileResourceManager extends RefType {
FileResourceManager() {
@@ -54,6 +70,8 @@ private class GetResourceSink extends UnsafeUrlForwardSink {
(
ma.getMethod() instanceof GetServletResourceMethod or
ma.getMethod() instanceof GetFacesResourceMethod or
ma.getMethod() instanceof GetClassResourceMethod or
ma.getMethod() instanceof GetClassLoaderResourceMethod or
ma.getMethod() instanceof GetWildflyResourceMethod or
ma.getMethod() instanceof GetVirtualFileMethod
) and

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

@@ -1,3 +1,5 @@
package com.example;
import java.io.InputStream;
import java.io.IOException;
import java.nio.file.Path;
@@ -13,6 +15,8 @@ import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
public class UnsafeResourceGet extends HttpServlet {
private static final String BASE_PATH = "/pages";
@Override
// BAD: getResource constructed from `ServletContext` without input validation
protected void doGet(HttpServletRequest request, HttpServletResponse response)
@@ -23,6 +27,7 @@ public class UnsafeResourceGet extends HttpServlet {
ServletConfig cfg = getServletConfig();
ServletContext sc = cfg.getServletContext();
// A sample request /fake.jsp/../WEB-INF/web.xml can load the web.xml file
URL url = sc.getResource(requestUrl);
InputStream in = url.openStream();
@@ -43,13 +48,15 @@ public class UnsafeResourceGet extends HttpServlet {
ServletContext sc = cfg.getServletContext();
Path path = Paths.get(requestUrl).normalize().toRealPath();
URL url = sc.getResource(path.toString());
if (path.startsWith(BASE_PATH)) {
URL url = sc.getResource(path.toString());
InputStream in = url.openStream();
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
while ((bytesRead = in.read(buf)) != -1) {
out.write(buf, 0, bytesRead);
InputStream in = url.openStream();
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
while ((bytesRead = in.read(buf)) != -1) {
out.write(buf, 0, bytesRead);
}
}
}
@@ -63,6 +70,7 @@ public class UnsafeResourceGet extends HttpServlet {
ServletConfig cfg = getServletConfig();
ServletContext sc = cfg.getServletContext();
// A sample request /fake.jsp/../WEB-INF/web.xml can load the web.xml file
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
@@ -89,4 +97,81 @@ public class UnsafeResourceGet extends HttpServlet {
}
}
}
@Override
// BAD: getResource constructed from `Class` without input validation
protected void doHead(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
String requestUrl = request.getParameter("requestURL");
ServletOutputStream out = response.getOutputStream();
// A sample request /fake.jsp/../../../WEB-INF/web.xml can load the web.xml file
// Note the class is in two levels of subpackages and `Class.loadResource` starts from its own directory
URL url = getClass().getResource(requestUrl);
InputStream in = url.openStream();
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
while ((bytesRead = in.read(buf)) != -1) {
out.write(buf, 0, bytesRead);
}
}
// GOOD: getResource constructed from `Class` with input validation
protected void doHeadGood(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
String requestUrl = request.getParameter("requestURL");
ServletOutputStream out = response.getOutputStream();
Path path = Paths.get(requestUrl).normalize().toRealPath();
if (path.startsWith(BASE_PATH)) {
URL url = getClass().getResource(path.toString());
InputStream in = url.openStream();
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
while ((bytesRead = in.read(buf)) != -1) {
out.write(buf, 0, bytesRead);
}
}
}
@Override
// BAD: getResourceAsStream constructed from `ClassLoader` without input validation
protected void doPut(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
String requestPath = request.getParameter("requestPath");
ServletOutputStream out = response.getOutputStream();
ServletConfig cfg = getServletConfig();
ServletContext sc = cfg.getServletContext();
// A sample request /fake.jsp/../../../WEB-INF/web.xml can load the web.xml file
// Note the class is in two levels of subpackages and `ClassLoader.getResourceAsStream` starts from its own directory
InputStream in = getClass().getClassLoader().getResourceAsStream(requestPath);
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
while ((bytesRead = in.read(buf)) != -1) {
out.write(buf, 0, bytesRead);
}
}
// GOOD: getResourceAsStream constructed from `ClassLoader` with input validation
protected void doPutGood(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
String requestPath = request.getParameter("requestPath");
ServletOutputStream out = response.getOutputStream();
ServletConfig cfg = getServletConfig();
ServletContext sc = cfg.getServletContext();
if (!requestPath.contains("..") && requestPath.startsWith("/trusted")) {
InputStream in = getClass().getClassLoader().getResourceAsStream(requestPath);
byte[] buf = new byte[4 * 1024]; // 4K buffer
int bytesRead;
while ((bytesRead = in.read(buf)) != -1) {
out.write(buf, 0, bytesRead);
}
}
}
}

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

@@ -1,7 +1,9 @@
edges
| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | UnsafeRequestPath.java:23:33:23:36 | path |
| UnsafeResourceGet.java:20:23:20:56 | getParameter(...) : String | UnsafeResourceGet.java:26:28:26:37 | requestUrl |
| UnsafeResourceGet.java:60:24:60:58 | getParameter(...) : String | UnsafeResourceGet.java:66:68:66:78 | requestPath |
| UnsafeResourceGet.java:24:23:24:56 | getParameter(...) : String | UnsafeResourceGet.java:31:28:31:37 | requestUrl |
| UnsafeResourceGet.java:67:24:67:58 | getParameter(...) : String | UnsafeResourceGet.java:74:68:74:78 | requestPath |
| UnsafeResourceGet.java:105:23:105:56 | getParameter(...) : String | UnsafeResourceGet.java:110:36:110:45 | requestUrl |
| UnsafeResourceGet.java:143:24:143:58 | getParameter(...) : String | UnsafeResourceGet.java:151:68:151:78 | requestPath |
| UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL |
| UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | UnsafeServletRequestDispatch.java:48:56:48:64 | returnURL |
| UnsafeServletRequestDispatch.java:71:17:71:44 | getParameter(...) : String | UnsafeServletRequestDispatch.java:76:53:76:56 | path |
@@ -21,10 +23,14 @@ edges
nodes
| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | semmle.label | getServletPath(...) : String |
| UnsafeRequestPath.java:23:33:23:36 | path | semmle.label | path |
| UnsafeResourceGet.java:20:23:20:56 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeResourceGet.java:26:28:26:37 | requestUrl | semmle.label | requestUrl |
| UnsafeResourceGet.java:60:24:60:58 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeResourceGet.java:66:68:66:78 | requestPath | semmle.label | requestPath |
| UnsafeResourceGet.java:24:23:24:56 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeResourceGet.java:31:28:31:37 | requestUrl | semmle.label | requestUrl |
| UnsafeResourceGet.java:67:24:67:58 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeResourceGet.java:74:68:74:78 | requestPath | semmle.label | requestPath |
| UnsafeResourceGet.java:105:23:105:56 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeResourceGet.java:110:36:110:45 | requestUrl | semmle.label | requestUrl |
| UnsafeResourceGet.java:143:24:143:58 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeResourceGet.java:151:68:151:78 | requestPath | semmle.label | requestPath |
| UnsafeServletRequestDispatch.java:23:22:23:54 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| UnsafeServletRequestDispatch.java:32:51:32:59 | returnURL | semmle.label | returnURL |
| UnsafeServletRequestDispatch.java:42:22:42:54 | getParameter(...) : String | semmle.label | getParameter(...) : String |
@@ -55,8 +61,10 @@ nodes
subpaths
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |