From be830de2f9586821de6de81de6d3b869f1b58676 Mon Sep 17 00:00:00 2001 From: luchua-bc Date: Sun, 11 Jul 2021 20:22:03 +0000 Subject: [PATCH 1/2] Query to detect unsafe request dispatcher --- .../CWE/CWE-552/UnsafeRequestDispatch.java | 24 +++++++ .../CWE/CWE-552/UnsafeRequestDispatch.qhelp | 37 ++++++++++ .../CWE/CWE-552/UnsafeRequestDispatch.ql | 62 +++++++++++++++++ .../CWE-552/UnsafeRequestDispatch.expected | 15 ++++ .../CWE-552/UnsafeRequestDispatch.java | 69 +++++++++++++++++++ .../CWE-552/UnsafeRequestDispatch.qlref | 1 + .../query-tests/security/CWE-552/options | 1 + 7 files changed, 209 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.ql create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.qlref create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/options diff --git a/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.java b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.java new file mode 100644 index 000000000000..f9474db2bf11 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.java @@ -0,0 +1,24 @@ +public class UnsafeRequestDispatch extends HttpServlet { + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + String action = request.getParameter("action"); + String returnURL = request.getParameter("returnURL"); + + ServletConfig cfg = getServletConfig(); + if (action.equals("Login")) { + ServletContext sc = cfg.getServletContext(); + + // GOOD: Request dispatcher with a whitelisted URI + RequestDispatcher rd = sc.getRequestDispatcher("/Login.jsp"); + rd.forward(request, response); + } else { + ServletContext sc = cfg.getServletContext(); + + // BAD: Request dispatcher constructed from `ServletContext` with user controlled input + RequestDispatcher rd = sc.getRequestDispatcher(returnURL); + rd.forward(request, response); + } + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.qhelp b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.qhelp new file mode 100644 index 000000000000..ba39ecc45f20 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.qhelp @@ -0,0 +1,37 @@ + + + + + +

Directly incorporating user input into HTTP requests dispatched from the Java EE +RequestDispatcher without validating the input can allow any web +application resource such as configuration files and source code to be disclosed.

+ + + +

Unsanitized user provided data must not be used to construct the path passed to +the RequestDispatcher. To guard against sensitive file and directory exposure, +it is advisable to avoid putting user input directly into Java EE RequestDispatcher. +Instead, maintain a list of authorized resources on the server; then choose from that list based +on the user input provided.

+ + + +

The following example shows an HTTP request parameter being used directly in a request dispatcher +without validating the input, which allows sensitive file exposure attacks. +It also shows how to remedy the problem by validating the user input against a known fixed string. +

+ + + + +
  • Jakarta Javadoc: + Security vulnerability with unsafe usage of RequestDispatcher. +
    • +
  • Micro Focus: + File Disclosure: J2EE +
    • +     +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.ql b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.ql new file mode 100644 index 000000000000..6d53d3a76e6b --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.ql @@ -0,0 +1,62 @@ +/** + * @name Java EE directory and file exposure from remote source + * @description File and directory exposure based on unvalidated user-input may allow attackers + * to access arbitrary configuration files and source code of Java EE applications. + * @kind path-problem + * @id java/unsafe-request-dispatch + * @tags security + * external/cwe/cwe-552 + */ + +import java +import semmle.code.java.dataflow.FlowSources +import DataFlow::PathGraph + +/** The Java class `javax.servlet.RequestDispatcher` or `jakarta.servlet.RequestDispatcher`. */ +class RequestDispatcher extends RefType { + RequestDispatcher() { + this.hasQualifiedName("javax.servlet", "RequestDispatcher") or + this.hasQualifiedName("jakarta.servlet", "RequestDispatcher") + } +} + +/** The `getRequestDispatcher` method. */ +class GetRequestDispatcherMethod extends Method { + GetRequestDispatcherMethod() { + this.getReturnType() instanceof RequestDispatcher and + this.getName() = "getRequestDispatcher" + } +} + +/** The request dispatch method. */ +class RequestDispatchMethod extends Method { + RequestDispatchMethod() { + this.getDeclaringType() instanceof RequestDispatcher and + this.hasName(["forward", "include"]) + } +} + +/** Request dispatcher sink. */ +class RequestDispatcherSink extends DataFlow::Node { + RequestDispatcherSink() { + exists(MethodAccess ma, MethodAccess ma2 | + ma.getMethod() instanceof GetRequestDispatcherMethod and + this.asExpr() = ma.getArgument(0) and + ma2.getMethod() instanceof RequestDispatchMethod and + DataFlow::localExprFlow(ma, ma2.getQualifier()) + ) + } +} + +class RequestDispatchConfig extends TaintTracking::Configuration { + RequestDispatchConfig() { this = "RequestDispatchConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof RequestDispatcherSink } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, RequestDispatchConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Unsafe request dispatcher due to $@.", source.getNode(), + "user-provided value" diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.expected b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.expected new file mode 100644 index 000000000000..21735178b524 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.expected @@ -0,0 +1,15 @@ +edges +| UnsafeRequestDispatch.java:17:22:17:54 | getParameter(...) : String | UnsafeRequestDispatch.java:26:51:26:59 | returnURL | +| UnsafeRequestDispatch.java:36:22:36:54 | getParameter(...) : String | UnsafeRequestDispatch.java:42:56:42:64 | returnURL | +| UnsafeRequestDispatch.java:66:23:66:56 | getParameter(...) : String | UnsafeRequestDispatch.java:67:32:67:41 | includeURL | +nodes +| UnsafeRequestDispatch.java:17:22:17:54 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| UnsafeRequestDispatch.java:26:51:26:59 | returnURL | semmle.label | returnURL | +| UnsafeRequestDispatch.java:36:22:36:54 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| UnsafeRequestDispatch.java:42:56:42:64 | returnURL | semmle.label | returnURL | +| UnsafeRequestDispatch.java:66:23:66:56 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| UnsafeRequestDispatch.java:67:32:67:41 | includeURL | semmle.label | includeURL | +#select +| UnsafeRequestDispatch.java:26:51:26:59 | returnURL | UnsafeRequestDispatch.java:17:22:17:54 | getParameter(...) : String | UnsafeRequestDispatch.java:26:51:26:59 | returnURL | Unsafe request dispatcher due to $@. | UnsafeRequestDispatch.java:17:22:17:54 | getParameter(...) | user-provided value | +| UnsafeRequestDispatch.java:42:56:42:64 | returnURL | UnsafeRequestDispatch.java:36:22:36:54 | getParameter(...) : String | UnsafeRequestDispatch.java:42:56:42:64 | returnURL | Unsafe request dispatcher due to $@. | UnsafeRequestDispatch.java:36:22:36:54 | getParameter(...) | user-provided value | +| UnsafeRequestDispatch.java:67:32:67:41 | includeURL | UnsafeRequestDispatch.java:66:23:66:56 | getParameter(...) : String | UnsafeRequestDispatch.java:67:32:67:41 | includeURL | Unsafe request dispatcher due to $@. | UnsafeRequestDispatch.java:66:23:66:56 | getParameter(...) | user-provided value | diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.java b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.java new file mode 100644 index 000000000000..397b33ef423a --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.java @@ -0,0 +1,69 @@ +import java.io.IOException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.RequestDispatcher; +import javax.servlet.ServletException; +import javax.servlet.ServletConfig; +import javax.servlet.ServletContext; + +public class UnsafeRequestDispatch extends HttpServlet { + + @Override + // BAD: Request dispatcher constructed from `ServletContext` with user controlled input + protected void doGet(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + String action = request.getParameter("action"); + String returnURL = request.getParameter("returnURL"); + + ServletConfig cfg = getServletConfig(); + if (action.equals("Login")) { + ServletContext sc = cfg.getServletContext(); + RequestDispatcher rd = sc.getRequestDispatcher("/Login.jsp"); + rd.forward(request, response); + } else { + ServletContext sc = cfg.getServletContext(); + RequestDispatcher rd = sc.getRequestDispatcher(returnURL); + rd.forward(request, response); + } + } + + @Override + // BAD: Request dispatcher constructed from `HttpServletRequest` with user controlled input + protected void doPost(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + String action = request.getParameter("action"); + String returnURL = request.getParameter("returnURL"); + + if (action.equals("Login")) { + RequestDispatcher rd = request.getRequestDispatcher("/Login.jsp"); + rd.forward(request, response); + } else { + RequestDispatcher rd = request.getRequestDispatcher(returnURL); + rd.forward(request, response); + } + } + + @Override + // GOOD: Request dispatcher with a whitelisted URI + protected void doPut(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + String action = request.getParameter("action"); + + if (action.equals("Login")) { + RequestDispatcher rd = request.getRequestDispatcher("/Login.jsp"); + rd.forward(request, response); + } else if (action.equals("Register")) { + RequestDispatcher rd = request.getRequestDispatcher("/Register.jsp"); + rd.forward(request, response); + } + } + + @Override + // BAD: Request dispatcher constructed from `HttpServletRequest` with user controlled input + protected void doHead(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + String includeURL = request.getParameter("includeURL"); + request.getRequestDispatcher(includeURL).include(request, response); + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.qlref b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.qlref new file mode 100644 index 000000000000..b0ea9657bd5e --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeRequestDispatch.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-552/UnsafeRequestDispatch.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/options b/java/ql/test/experimental/query-tests/security/CWE-552/options new file mode 100644 index 000000000000..4dd270d77661 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4 \ No newline at end of file From 93ab08b50206ef7a4cbbdf050bb8ab4331dd215a Mon Sep 17 00:00:00 2001 From: luchua-bc Date: Sun, 18 Jul 2021 15:35:18 +0000 Subject: [PATCH 2/2] Add query for Java EE resource loading check --- .../CWE/CWE-552/UnsafeResourceLoad.ql | 112 ++++++++++++++++++ .../CWE-552/UnsafeResourceLoad.expected | 15 +++ .../security/CWE-552/UnsafeResourceLoad.java | 103 ++++++++++++++++ .../security/CWE-552/UnsafeResourceLoad.qlref | 1 + .../servlet/http/HttpServletRequest.java | 3 + 5 files changed, 234 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-552/UnsafeResourceLoad.ql create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeResourceLoad.ql b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeResourceLoad.ql new file mode 100644 index 000000000000..ab676649549c --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-552/UnsafeResourceLoad.ql @@ -0,0 +1,112 @@ +/** + * @name File loaded in Java EE applications that can be controlled by an attacker + * @description File name or path based on unvalidated user-input may allow attackers to access + * arbitrary configuration file and source code in Java EE applications. + * @kind path-problem + * @id java/unsafe-resource-load + * @tags security + * external/cwe/cwe-552 + * external/cwe/cwe-022 + * external/cwe/cwe-073 + */ + +import java +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking2 +import DataFlow::PathGraph + +/** The class `java.lang.ClassLoader`. */ +class ClassLoader extends RefType { + ClassLoader() { this.hasQualifiedName("java.lang", "ClassLoader") } +} + +/** The class `javax.servlet.ServletContext`. */ +class ServletContext extends RefType { + ServletContext() { this.hasQualifiedName("javax.servlet", "ServletContext") } +} + +/** The resource loading method. */ +class LoadResourceMethod extends Method { + LoadResourceMethod() { + ( + this.getDeclaringType().getASupertype*() instanceof ClassLoader or + this.getDeclaringType().getASupertype*() instanceof ServletContext + ) and + this.getName() = ["getResource", "getResourcePaths", "getResourceAsStream", "getResources"] + } +} + +/** + * Holds if the resource path in the string format is matched against another path, + * probably a whitelisted one, and doesn't contain `..` characters for path navigation. + */ +predicate isStringPathMatch(MethodAccess ma) { + ma.getMethod().getDeclaringType() instanceof TypeString and + ma.getMethod().getName() = ["startsWith", "equals"] and + exists(MethodAccess ma2 | + ma2.getMethod().getDeclaringType() instanceof TypeString and + ma2.getMethod().hasName("contains") and + ma2.getAnArgument().(StringLiteral).getValue() = ".." and + ma2.getQualifier().(VarAccess).getVariable().getAnAccess() = ma.getQualifier() + ) +} + +/** + * Holds if the resource path of the type `java.nio.file.Path` is matched against another + * path, probably a whitelisted one. + */ +predicate isFilePathMatch(MethodAccess ma) { + ma.getMethod().getDeclaringType() instanceof TypePath and + ma.getMethod().getName() = ["startsWith", "equals"] +} + +/** Taint configuration of unsafe resource loading. */ +class LoadResourceConfig extends TaintTracking::Configuration { + LoadResourceConfig() { this = "LoadResourceConfig" } + + override predicate isSource(DataFlow::Node source) { + source instanceof RemoteFlowSource and + not exists(CompareResourcePathConfig cc, DataFlow::Node sink | cc.hasFlow(source, sink)) + } + + /** Sink of resource loading. */ + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof LoadResourceMethod and + sink.asExpr() = ma.getArgument(0) + ) + } +} + +/** Taint configuration of resource path comparison. */ +class CompareResourcePathConfig extends TaintTracking2::Configuration { + CompareResourcePathConfig() { this = "CompareResourcePathConfig" } + + override predicate isSource(DataFlow2::Node source) { source instanceof RemoteFlowSource } + + /** Sink of path matching check. */ + override predicate isSink(DataFlow2::Node sink) { + exists(MethodAccess ma | + ( + isStringPathMatch(ma) or + isFilePathMatch(ma) + ) and + sink.asExpr() = ma.getQualifier() + ) + } + + /** Path normalization. */ + override predicate isAdditionalTaintStep(DataFlow2::Node node1, DataFlow2::Node node2) { + exists(MethodAccess ma | + ma.getMethod().getDeclaringType() instanceof TypePath and + ma.getMethod().getName() = ["get", "normalize", "resolve"] and + (node1.asExpr() = ma.getQualifier() or node1.asExpr() = ma.getArgument(0)) and + node2.asExpr() = ma + ) + } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, LoadResourceConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Unsafe request loading due to $@.", source.getNode(), + "user-provided value" diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.expected b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.expected new file mode 100644 index 000000000000..05a8e0403225 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.expected @@ -0,0 +1,15 @@ +edges +| UnsafeResourceLoad.java:23:17:23:40 | getParameter(...) : String | UnsafeResourceLoad.java:26:49:26:52 | path | +| UnsafeResourceLoad.java:35:17:35:40 | getParameter(...) : String | UnsafeResourceLoad.java:39:68:39:71 | path | +| UnsafeResourceLoad.java:87:17:87:40 | getParameter(...) : String | UnsafeResourceLoad.java:90:69:90:72 | path | +nodes +| UnsafeResourceLoad.java:23:17:23:40 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| UnsafeResourceLoad.java:26:49:26:52 | path | semmle.label | path | +| UnsafeResourceLoad.java:35:17:35:40 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| UnsafeResourceLoad.java:39:68:39:71 | path | semmle.label | path | +| UnsafeResourceLoad.java:87:17:87:40 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| UnsafeResourceLoad.java:90:69:90:72 | path | semmle.label | path | +#select +| UnsafeResourceLoad.java:26:49:26:52 | path | UnsafeResourceLoad.java:23:17:23:40 | getParameter(...) : String | UnsafeResourceLoad.java:26:49:26:52 | path | Unsafe request loading due to $@. | UnsafeResourceLoad.java:23:17:23:40 | getParameter(...) | user-provided value | +| UnsafeResourceLoad.java:39:68:39:71 | path | UnsafeResourceLoad.java:35:17:35:40 | getParameter(...) : String | UnsafeResourceLoad.java:39:68:39:71 | path | Unsafe request loading due to $@. | UnsafeResourceLoad.java:35:17:35:40 | getParameter(...) | user-provided value | +| UnsafeResourceLoad.java:90:69:90:72 | path | UnsafeResourceLoad.java:87:17:87:40 | getParameter(...) : String | UnsafeResourceLoad.java:90:69:90:72 | path | Unsafe request loading due to $@. | UnsafeResourceLoad.java:87:17:87:40 | getParameter(...) | user-provided value | diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.java b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.java new file mode 100644 index 000000000000..92a85dc23e02 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.java @@ -0,0 +1,103 @@ +import java.io.IOException; +import java.io.InputStream; + +import java.net.URL; +import java.net.URLConnection; +import java.nio.file.Path; +import java.nio.file.Paths; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.RequestDispatcher; +import javax.servlet.ServletException; +import javax.servlet.ServletConfig; +import javax.servlet.ServletContext; + +public class UnsafeResourceLoad extends HttpServlet { + private static final String BASE_PATH = "/pages"; + + @Override + // BAD: Request loading constructed from `ServletContext` with user controlled input + public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + String path = req.getParameter("path"); + + // /WEB-INF/web.xml or /pages/public_page.jsp/../../WEB-INF/web.xml works!!! + URL url = req.getServletContext().getResource(path); + + // create a urlconnection object + URLConnection urlConnection = url.openConnection(); + } + + @Override + // BAD: Request loading constructed from `ClassLoader` with user controlled input + public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + String path = req.getParameter("path"); + + // ../../WEB-INF/web.xml, or ../../pages/sec_page.jsp, or ../../pages/public_page.jsp/../../WEB-INF/web.xml works!!! + // Cannot access resources outside of the application's web root /webapps/s3-listener + InputStream in = getClass().getClassLoader().getResourceAsStream(path); + } + + private URL getResourceUrl(String resourcePath, HttpServletRequest req) throws IOException { + String resourceBasePath = req.getServletContext().getRealPath(BASE_PATH); + + String realPath = req.getServletContext().getRealPath(resourcePath); + + if (!isValidResourcePath(resourceBasePath, resourcePath)) { + return null; + } + + return req.getServletContext().getResource(resourcePath); + } + + private boolean isValidResourcePath(String resourceBasePath, String resourcePath) { + Path requestedPath = Paths.get(resourceBasePath).resolve(resourcePath).normalize(); + return requestedPath.startsWith(resourceBasePath); + } + + @Override + // GOOD: Request loading constructed from `ServletContext` with path normalization and comparison + public void doPut(HttpServletRequest req, HttpServletResponse resp) throws IOException { + String path = req.getParameter("path"); + + URL url = getResourceUrl(path, req); + + // create a urlconnection object + URLConnection urlConnection = url.openConnection(); + } + + @Override + // GOOD: Request loading constructed from `ClassLoader` with path normalization and comparison + protected void doHead(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + String path = req.getParameter("path"); + + String resourceBasePath = req.getServletContext().getRealPath(BASE_PATH); + Path requestedPath = Paths.get(resourceBasePath).resolve(path).normalize(); + + if (requestedPath.startsWith(resourceBasePath)) { + InputStream in = getClass().getClassLoader().getResourceAsStream(path); + } + } + + // BAD: Request loading constructed from `ClassLoader` with path comparison but without `..` navigation character check + protected void doHead2(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + String path = req.getParameter("path"); + + if (path.startsWith(BASE_PATH)) { + InputStream in = getClass().getClassLoader().getResourceAsStream(path); + } + } + + // GOOD: Request loading constructed from `ClassLoader` with path comparison and navigation check + protected void doHead3(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + String path = req.getParameter("path"); + + if (path.startsWith(BASE_PATH) && !path.contains("..")) { + InputStream in = getClass().getClassLoader().getResourceAsStream(path); + } + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.qlref b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.qlref new file mode 100644 index 000000000000..008414b215c3 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-552/UnsafeResourceLoad.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-552/UnsafeResourceLoad.ql diff --git a/java/ql/test/stubs/servlet-api-2.4/javax/servlet/http/HttpServletRequest.java b/java/ql/test/stubs/servlet-api-2.4/javax/servlet/http/HttpServletRequest.java index bc15d102eca2..8dd87864e7e1 100644 --- a/java/ql/test/stubs/servlet-api-2.4/javax/servlet/http/HttpServletRequest.java +++ b/java/ql/test/stubs/servlet-api-2.4/javax/servlet/http/HttpServletRequest.java @@ -25,6 +25,7 @@ import java.util.Enumeration; import javax.servlet.ServletRequest; +import javax.servlet.ServletContext; public interface HttpServletRequest extends ServletRequest { public String getAuthType(); @@ -52,4 +53,6 @@ public interface HttpServletRequest extends ServletRequest { public boolean isRequestedSessionIdFromCookie(); public boolean isRequestedSessionIdFromURL(); public boolean isRequestedSessionIdFromUrl(); + // Since Servlet 3.0 + public ServletContext getServletContext(); }