Query to detect unsafe request dispatcher usage

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

@@ -347,3 +347,27 @@ predicate isRequestGetParamMethod(MethodAccess ma) {
   ma.getMethod() instanceof ServletRequestGetParameterMapMethod or
   ma.getMethod() instanceof HttpServletRequestGetQueryStringMethod
 }
+
+/** The Java EE RequestDispatcher. */
+library class RequestDispatcher extends RefType {
+  RequestDispatcher() {
+    this.hasQualifiedName(["javax.servlet", "jakarta.servlet"], "RequestDispatcher") or
+    this.hasQualifiedName("javax.portlet", "PortletRequestDispatcher")
+  }
+}
+
+/** The `getRequestDispatcher` method. */
+library class GetRequestDispatcherMethod extends Method {
+  GetRequestDispatcherMethod() {
+    this.getReturnType() instanceof RequestDispatcher and
+    this.getName() = "getRequestDispatcher"
+  }
+}
+
+/** The request dispatch method. */
+library class RequestDispatchMethod extends Method {
+  RequestDispatchMethod() {
+    this.getDeclaringType() instanceof RequestDispatcher and
+    this.hasName(["forward", "include"])
+  }
+}

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

@@ -0,0 +1,30 @@
+public class UnsafeRequestPath implements Filter {
+	private static final String BASE_PATH = "/pages";
+
+	@Override
+	public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+		throws IOException, ServletException {
+
+		{
+			// BAD: Request dispatcher from servlet path without check 
+			String path = ((HttpServletRequest) request).getServletPath();
+			// A sample payload "/%57EB-INF/web.xml" can bypass this `startsWith` check
+			if (path != null && !path.startsWith("/WEB-INF")) {
+				request.getRequestDispatcher(path).forward(request, response);
+			} else {
+				chain.doFilter(request, response);
+			}
+		}
+
+		{
+			// GOOD: Request dispatcher from servlet path with path traversal check 
+			String path = ((HttpServletRequest) request).getServletPath();
+		
+			if (path.startsWith(BASE_PATH) && !path.contains("..")) {
+				request.getRequestDispatcher(path).forward(request, response);
+			} else {
+				chain.doFilter(request, response);
+			}
+		}
+	}
+}

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

@@ -0,0 +1,82 @@
+public class UnsafeServletRequestDispatch extends HttpServlet {
+	private static final String BASE_PATH = "/pages";
+
+	@Override
+	protected void doGet(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
+		{
+			// GOOD: whitelisted URI
+			if (action.equals("Login")) {
+				ServletContext sc = cfg.getServletContext();
+				RequestDispatcher rd = sc.getRequestDispatcher("/Login.jsp");
+				rd.forward(request, response);
+			} 
+		}
+
+		{
+			// BAD: Request dispatcher constructed from `ServletContext` without input validation
+			String returnURL = request.getParameter("returnURL");
+			ServletConfig cfg = getServletConfig();
+
+			ServletContext sc = cfg.getServletContext();
+			RequestDispatcher rd = sc.getRequestDispatcher(returnURL);
+			rd.forward(request, response);
+		}
+
+		{
+			// BAD: Request dispatcher without path traversal check 
+			String path = request.getParameter("path");
+
+			// A sample payload "/pages/welcome.jsp/../WEB-INF/web.xml" can bypass the `startsWith` check 
+			// The payload "/pages/welcome.jsp/../../%57EB-INF/web.xml" can bypass the check as well since RequestDispatcher will decode `%57` as `W`  
+			if (path.startsWith(BASE_PATH)) {
+				request.getServletContext().getRequestDispatcher(path).include(request, response);
+			}
+		}
+	}
+
+	{
+		// GOOD: Request dispatcher with path traversal check 
+		String path = request.getParameter("path");
+
+		if (path.startsWith(BASE_PATH) && !path.contains("..")) {
+			request.getServletContext().getRequestDispatcher(path).include(request, response);
+		}
+	}
+
+	{
+		// GOOD: Request dispatcher with path normalization 
+		String path = request.getParameter("path");
+		Path requestedPath = Paths.get(BASE_PATH).resolve(path).normalize();
+
+		// /pages/welcome.jsp/../../WEB-INF/web.xml becomes /WEB-INF/web.xml
+		// /pages/welcome.jsp/../../%57EB-INF/web.xml becomes /%57EB-INF/web.xml
+		if (requestedPath.startsWith(BASE_PATH)) {
+			request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response);
+		}
+	}
+
+	{
+		// BAD: Request dispatcher with improper negation check and without url decoding 
+		String path = request.getParameter("path");
+		Path requestedPath = Paths.get(BASE_PATH).resolve(path).normalize();
+
+		if (!requestedPath.startsWith("/WEB-INF") && !requestedPath.startsWith("/META-INF")) {
+			request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response);
+		}
+	}
+
+	{
+		// GOOD: Request dispatcher with path traversal check and url decoding
+		String path = request.getParameter("path");
+		boolean hasEncoding = path.contains("%");
+		while (hasEncoding) {
+			path = URLDecoder.decode(path, "UTF-8");
+			hasEncoding = path.contains("%");
+		}
+
+		if (!path.startsWith("/WEB-INF/") && !path.contains("..")) {
+			request.getServletContext().getRequestDispatcher(path).include(request, response);
+		}
+	}
+}

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

@@ -5,27 +5,45 @@
 
 
 <overview>
-<p>Constructing a server-side redirect path with user input could allow an attacker to download application binaries 
+<p>Constructing a server-side redirect path with user input could allow an attacker to download application binaries
 (including application classes or jar files) or view arbitrary files within protected directories.</p>
 
 </overview>
 <recommendation>
 
-<p>In order to prevent untrusted URL forwarding, it is recommended to avoid concatenating user input directly into the forwarding URL.</p>
+<p>Unsanitized user provided data must not be used to construct the path for URL forwarding. In order to prevent
+untrusted URL forwarding, it is recommended to avoid concatenating user input directly into the forwarding URL.
+</p>
 
 </recommendation>
 <example>
 
 <p>The following examples show the bad case and the good case respectively.
 The <code>bad</code> methods show an HTTP request parameter being used directly in a URL forward
-without validating the input, which may cause file leakage. In <code>good1</code> method, 
+without validating the input, which may cause file leakage. In <code>good1</code> method,
 ordinary forwarding requests are shown, which will not cause file leakage.
 </p>
 
 <sample src="UnsafeUrlForward.java" />
 
+<p>The following examples show an HTTP request parameter or request path being used directly in a
+request dispatcher of Java EE without validating the input, which allows sensitive file exposure
+attacks. It also shows how to remedy the problem by validating the user input.
+</p>
+
+<sample src="UnsafeServletRequestDispatch.java" />
+<sample src="UnsafeRequestPath.java" />
+
 </example>
 <references>
-<li>File Disclosure: <a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_spring">Unsafe Url Forward</a>.</li>
+<li>File Disclosure:
+  <a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_spring">Unsafe Url Forward</a>.
+</li>
+<li>Jakarta Javadoc:
+  <a href="https://jakarta.ee/specifications/webprofile/9/apidocs/jakarta/servlet/servletrequest#getRequestDispatcher-java.lang.String-">Security vulnerability with unsafe usage of RequestDispatcher</a>.
+</li>
+<li>Micro Focus:
+  <a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_j2ee">File Disclosure: J2EE</a>
+</li>
 </references>
 </qhelp>

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

@@ -1,11 +1,11 @@
 /**
- * @name Unsafe url forward from remote source
- * @description URL forward based on unvalidated user-input
+ * @name Unsafe url forward or dispatch from remote source
+ * @description URL forward or dispatch based on unvalidated user-input
  *              may cause file information disclosure.
  * @kind path-problem
  * @problem.severity error
  * @precision high
- * @id java/unsafe-url-forward
+ * @id java/unsafe-url-forward-dispatch
  * @tags security
  *       external/cwe-552
  */
@@ -14,20 +14,137 @@ import java
 import UnsafeUrlForward
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.frameworks.Servlets
+import semmle.code.java.controlflow.Guards
 import DataFlow::PathGraph
 
-private class StartsWithSanitizer extends DataFlow::BarrierGuard {
-  StartsWithSanitizer() {
-    this.(MethodAccess).getMethod().hasName("startsWith") and
-    this.(MethodAccess).getMethod().getDeclaringType() instanceof TypeString and
-    this.(MethodAccess).getMethod().getNumberOfParameters() = 1
-  }
+/**
+ * Holds if `ma` is a method call of matching with a path string, probably a whitelisted one.
+ */
+predicate isStringPathMatch(MethodAccess ma) {
+  ma.getMethod().getDeclaringType() instanceof TypeString and
+  ma.getMethod().getName() = ["startsWith", "matches", "regionMatches"]
+}
 
-  override predicate checks(Expr e, boolean branch) {
-    e = this.(MethodAccess).getQualifier() and branch = true
+/**
+ * Holds if `ma` is a method call of `java.nio.file.Path` which matches with another
+ * path, probably a whitelisted one.
+ */
+predicate isFilePathMatch(MethodAccess ma) {
+  ma.getMethod().getDeclaringType() instanceof TypePath and
+  ma.getMethod().getName() = "startsWith"
+}
+
+/**
+ * Holds if `ma` is a method call that checks an input doesn't match using the `!`
+ * logical negation expression.
+ */
+predicate checkNoPathMatch(MethodAccess ma) {
+  exists(LogNotExpr lne |
+    (isStringPathMatch(ma) or isFilePathMatch(ma)) and
+    lne.getExpr() = ma
+  )
+}
+
+/**
+ * Holds if `ma` is a method call to check special characters `..` used in path traversal.
+ */
+predicate isPathTraversalCheck(MethodAccess ma) {
+  ma.getMethod().getDeclaringType() instanceof TypeString and
+  ma.getMethod().hasName(["contains", "indexOf"]) and
+  ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = ".."
+}
+
+/**
+ * Holds if `ma` is a method call to decode a url string or check url encoding.
+ */
+predicate isPathDecoding(MethodAccess ma) {
+  // Search the special character `%` used in url encoding
+  ma.getMethod().getDeclaringType() instanceof TypeString and
+  ma.getMethod().hasName(["contains", "indexOf"]) and
+  ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "%"
+  or
+  // Call to `URLDecoder` assuming the implementation handles double encoding correctly
+  ma.getMethod().getDeclaringType().hasQualifiedName("java.net", "URLDecoder") and
+  ma.getMethod().hasName("decode")
+}
+
+private class PathMatchSanitizer extends DataFlow::Node {
+  PathMatchSanitizer() {
+    exists(MethodAccess ma |
+      (
+        isStringPathMatch(ma) and
+        exists(MethodAccess ma2 |
+          isPathTraversalCheck(ma2) and
+          ma.getQualifier().(VarAccess).getVariable().getAnAccess() = ma2.getQualifier()
+        )
+        or
+        isFilePathMatch(ma)
+      ) and
+      (
+        not checkNoPathMatch(ma)
+        or
+        // non-match check needs decoding e.g. !path.startsWith("/WEB-INF/") won't detect /%57EB-INF/web.xml, which will be decoded and served by RequestDispatcher
+        checkNoPathMatch(ma) and
+        exists(MethodAccess ma2 |
+          isPathDecoding(ma2) and
+          ma.getQualifier().(VarAccess).getVariable().getAnAccess() = ma2.getQualifier()
+        )
+      ) and
+      this.asExpr() = ma.getQualifier()
+    )
   }
 }
 
+/**
+ * Holds if `ma` is a method call to check string content, which means an input string is not
+ * blindly trusted and helps to reduce FPs.
+ */
+predicate checkStringContent(MethodAccess ma, Expr expr) {
+  ma.getMethod().getDeclaringType() instanceof TypeString and
+  ma.getMethod()
+      .hasName([
+          "charAt", "contains", "equals", "equalsIgnoreCase", "getBytes", "getChars", "indexOf",
+          "lastIndexOf", "length", "matches", "regionMatches", "replace", "replaceAll",
+          "replaceFirst", "substring"
+        ]) and
+  expr = ma.getQualifier()
+  or
+  (
+    ma.getMethod().getDeclaringType() instanceof TypeStringBuffer or
+    ma.getMethod().getDeclaringType() instanceof TypeStringBuilder
+  ) and
+  expr = ma.getAnArgument()
+}
+
+private class StringOperationSanitizer extends DataFlow::Node {
+  StringOperationSanitizer() { exists(MethodAccess ma | checkStringContent(ma, this.asExpr())) }
+}
+
+/**
+ * Holds if `expr` is an expression returned from null or empty string check.
+ */
+predicate isNullOrEmptyCheck(Expr expr) {
+  exists(ConditionBlock cb, ReturnStmt rt |
+    cb.controls(rt.getBasicBlock(), true) and
+    (
+      cb.getCondition().(EQExpr).getAnOperand() instanceof NullLiteral // if (path == null)
+      or
+      // if (path.equals(""))
+      exists(MethodAccess ma |
+        cb.getCondition() = ma and
+        ma.getMethod().getDeclaringType() instanceof TypeString and
+        ma.getMethod().hasName("equals") and
+        ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = ""
+      )
+    ) and
+    expr.getParent+() = rt
+  )
+}
+
+private class NullOrEmptyCheckSanitizer extends DataFlow::Node {
+  NullOrEmptyCheckSanitizer() { isNullOrEmptyCheck(this.asExpr()) }
+}
+
 class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
   UnsafeUrlForwardFlowConfig() { this = "UnsafeUrlForwardFlowConfig" }
 
@@ -45,11 +162,12 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeUrlForwardSink }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof StartsWithSanitizer
+  override predicate isSanitizer(DataFlow::Node node) {
+    node instanceof UnsafeUrlForwardSanitizer or
+    node instanceof PathMatchSanitizer or
+    node instanceof StringOperationSanitizer or
+    node instanceof NullOrEmptyCheckSanitizer
   }
-
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof UnsafeUrlForwardSanitizer }
 }
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeUrlForwardFlowConfig conf

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

@@ -32,11 +32,11 @@ private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
 
 abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
 
-/** An argument to `ServletRequest.getRequestDispatcher`. */
+/** An argument to `getRequestDispatcher`. */
 private class RequestDispatcherSink extends UnsafeUrlForwardSink {
   RequestDispatcherSink() {
     exists(MethodAccess ma |
-      ma.getMethod() instanceof ServletRequestGetRequestDispatcherMethod and
+      ma.getMethod() instanceof GetRequestDispatcherMethod and
       ma.getArgument(0) = this.asExpr()
     )
   }
@@ -70,3 +70,28 @@ private class SpringUrlForwardSink extends UnsafeUrlForwardSink {
     this.asExpr() = any(ForwardPrefix fp).getAnAppendedExpression()
   }
 }
+
+/** Source model of remote flow source from `getServletPath`. */
+private class ServletGetPathSource extends SourceModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "javax.servlet.http;HttpServletRequest;true;getServletPath;;;ReturnValue;remote",
+        "jakarta.servlet.http;HttpServletRequest;true;getServletPath;;;ReturnValue;remote"
+      ]
+  }
+}
+
+/** Taint model related to `java.nio.file.Path`. */
+private class FilePathFlowStep extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "java.nio.file;Paths;true;get;;;Argument[-1];ReturnValue;taint",
+        "java.nio.file;Path;true;resolve;;;Argument[0];ReturnValue;taint",
+        "java.nio.file;Path;true;normalize;;;Argument[-1];ReturnValue;taint",
+        "java.nio.file;Path;true;startsWith;;;Argument[-1];ReturnValue;taint",
+        "java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint"
+      ]
+  }
+}

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

@@ -0,0 +1,52 @@
+import java.io.IOException;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+// @WebFilter("/*")
+public class UnsafeRequestPath implements Filter {
+	private static final String BASE_PATH = "/pages";
+
+	@Override
+	// BAD: Request dispatcher from servlet path without check 
+	public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+		throws IOException, ServletException {
+		String path = ((HttpServletRequest) request).getServletPath();
+		// A sample payload "/%57EB-INF/web.xml" can bypass this `startsWith` check
+		if (path != null && !path.startsWith("/WEB-INF")) {
+			request.getRequestDispatcher(path).forward(request, response);
+		} else {
+			chain.doFilter(request, response);
+		}
+	}
+
+	// GOOD: Request dispatcher from servlet path with check 
+	public void doFilter2(ServletRequest request, ServletResponse response, FilterChain chain)
+		throws IOException, ServletException {
+		String path = ((HttpServletRequest) request).getServletPath();
+		
+		if (path.startsWith(BASE_PATH) && !path.contains("..")) {
+			request.getRequestDispatcher(path).forward(request, response);
+		} else {
+			chain.doFilter(request, response);
+		}
+	}
+
+	// GOOD: Request dispatcher from servlet path with whitelisted string comparison 
+	public void doFilter3(ServletRequest request, ServletResponse response, FilterChain chain)
+		throws IOException, ServletException {
+		String path = ((HttpServletRequest) request).getServletPath();
+		
+		if (path.equals("/comaction")) {
+			request.getRequestDispatcher(path).forward(request, response);
+		} else {
+			chain.doFilter(request, response);
+		}
+	}
+}

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

@@ -0,0 +1,128 @@
+import java.io.IOException;
+import java.net.URLDecoder;
+import java.io.File;
+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 UnsafeServletRequestDispatch extends HttpServlet {
+	private static final String BASE_PATH = "/pages";
+
+	@Override
+	// BAD: Request dispatcher constructed from `ServletContext` without input validation
+	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` without input validation 
+	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);
+		}
+	}
+
+	// BAD: Request dispatcher without path traversal check 
+	protected void doHead2(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
+		String path = request.getParameter("path");
+
+		// A sample payload "/pages/welcome.jsp/../WEB-INF/web.xml" can bypass the `startsWith` check 
+		// The payload "/pages/welcome.jsp/../../%57EB-INF/web.xml" can bypass the check as well since RequestDispatcher will decode `%57` as `W`  
+		if (path.startsWith(BASE_PATH)) {
+			request.getServletContext().getRequestDispatcher(path).include(request, response);
+		}
+	}
+
+	// GOOD: Request dispatcher with path traversal check 
+	protected void doHead3(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
+		String path = request.getParameter("path");
+
+		if (path.startsWith(BASE_PATH) && !path.contains("..")) {
+			request.getServletContext().getRequestDispatcher(path).include(request, response);
+		}
+	}
+
+	// GOOD: Request dispatcher with path normalization and comparison
+	protected void doHead4(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
+		String path = request.getParameter("path");
+		Path requestedPath = Paths.get(BASE_PATH).resolve(path).normalize();
+
+		// /pages/welcome.jsp/../../WEB-INF/web.xml becomes /WEB-INF/web.xml
+		// /pages/welcome.jsp/../../%57EB-INF/web.xml becomes /%57EB-INF/web.xml
+		if (requestedPath.startsWith(BASE_PATH)) {
+			request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response);
+		}
+	}
+
+	// BAD: Request dispatcher with improper negation check and without url decoding 
+	protected void doHead5(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
+		String path = request.getParameter("path");
+		Path requestedPath = Paths.get(BASE_PATH).resolve(path).normalize();
+
+		if (!requestedPath.startsWith("/WEB-INF") && !requestedPath.startsWith("/META-INF")) {
+			request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response);
+		}
+	}
+
+	// GOOD: Request dispatcher with path traversal check and url decoding
+	protected void doHead6(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
+		String path = request.getParameter("path");
+		boolean hasEncoding = path.contains("%");
+		while (hasEncoding) {
+			path = URLDecoder.decode(path, "UTF-8");
+			hasEncoding = path.contains("%");
+		}
+
+		if (!path.startsWith("/WEB-INF/") && !path.contains("..")) {
+			request.getServletContext().getRequestDispatcher(path).include(request, response);
+		}
+	}
+}

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

@@ -1,4 +1,13 @@
 edges
+| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | UnsafeRequestPath.java:23:33:23:36 | path |
+| 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 |
+| UnsafeServletRequestDispatch.java:106:17:106:44 | getParameter(...) : String | UnsafeServletRequestDispatch.java:107:53:107:56 | path : String |
+| UnsafeServletRequestDispatch.java:107:24:107:57 | resolve(...) : Path | UnsafeServletRequestDispatch.java:107:24:107:69 | normalize(...) : Path |
+| UnsafeServletRequestDispatch.java:107:24:107:69 | normalize(...) : Path | UnsafeServletRequestDispatch.java:110:53:110:65 | requestedPath : Path |
+| UnsafeServletRequestDispatch.java:107:53:107:56 | path : String | UnsafeServletRequestDispatch.java:107:24:107:57 | resolve(...) : Path |
+| UnsafeServletRequestDispatch.java:110:53:110:65 | requestedPath : Path | UnsafeServletRequestDispatch.java:110:53:110:76 | toString(...) |
 | UnsafeUrlForward.java:13:27:13:36 | url : String | UnsafeUrlForward.java:14:27:14:29 | url |
 | UnsafeUrlForward.java:18:27:18:36 | url : String | UnsafeUrlForward.java:20:28:20:30 | url |
 | UnsafeUrlForward.java:25:21:25:30 | url : String | UnsafeUrlForward.java:26:23:26:25 | url |
@@ -6,7 +15,22 @@ edges
 | UnsafeUrlForward.java:30:27:30:36 | url : String | UnsafeUrlForward.java:31:61:31:63 | url |
 | UnsafeUrlForward.java:36:19:36:28 | url : String | UnsafeUrlForward.java:38:33:38:35 | url |
 | UnsafeUrlForward.java:47:19:47:28 | url : String | UnsafeUrlForward.java:49:33:49:62 | ... + ... |
+| UnsafeUrlForward.java:58:19:58:28 | url : String | UnsafeUrlForward.java:60:33:60:62 | ... + ... |
 nodes
+| UnsafeRequestPath.java:20:17:20:63 | getServletPath(...) : String | semmle.label | getServletPath(...) : String |
+| UnsafeRequestPath.java:23:33:23:36 | path | semmle.label | path |
+| 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 |
+| UnsafeServletRequestDispatch.java:48:56:48:64 | returnURL | semmle.label | returnURL |
+| UnsafeServletRequestDispatch.java:71:17:71:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| UnsafeServletRequestDispatch.java:76:53:76:56 | path | semmle.label | path |
+| UnsafeServletRequestDispatch.java:106:17:106:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| UnsafeServletRequestDispatch.java:107:24:107:57 | resolve(...) : Path | semmle.label | resolve(...) : Path |
+| UnsafeServletRequestDispatch.java:107:24:107:69 | normalize(...) : Path | semmle.label | normalize(...) : Path |
+| UnsafeServletRequestDispatch.java:107:53:107:56 | path : String | semmle.label | path : String |
+| UnsafeServletRequestDispatch.java:110:53:110:65 | requestedPath : Path | semmle.label | requestedPath : Path |
+| UnsafeServletRequestDispatch.java:110:53:110:76 | toString(...) | semmle.label | toString(...) |
 | UnsafeUrlForward.java:13:27:13:36 | url : String | semmle.label | url : String |
 | UnsafeUrlForward.java:14:27:14:29 | url | semmle.label | url |
 | UnsafeUrlForward.java:18:27:18:36 | url : String | semmle.label | url : String |
@@ -20,8 +44,15 @@ nodes
 | UnsafeUrlForward.java:38:33:38:35 | url | semmle.label | url |
 | UnsafeUrlForward.java:47:19:47:28 | url : String | semmle.label | url : String |
 | UnsafeUrlForward.java:49:33:49:62 | ... + ... | semmle.label | ... + ... |
+| UnsafeUrlForward.java:58:19:58:28 | url : String | semmle.label | url : String |
+| UnsafeUrlForward.java:60:33:60:62 | ... + ... | semmle.label | ... + ... |
 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 |
+| 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 |
+| UnsafeServletRequestDispatch.java:110:53:110:76 | toString(...) | UnsafeServletRequestDispatch.java:106:17:106:44 | getParameter(...) : String | UnsafeServletRequestDispatch.java:110:53:110:76 | toString(...) | Potentially untrusted URL forward due to $@. | UnsafeServletRequestDispatch.java:106:17:106:44 | getParameter(...) | user-provided value |
 | UnsafeUrlForward.java:14:27:14:29 | url | UnsafeUrlForward.java:13:27:13:36 | url : String | UnsafeUrlForward.java:14:27:14:29 | url | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:13:27:13:36 | url | user-provided value |
 | UnsafeUrlForward.java:20:28:20:30 | url | UnsafeUrlForward.java:18:27:18:36 | url : String | UnsafeUrlForward.java:20:28:20:30 | url | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:18:27:18:36 | url | user-provided value |
 | UnsafeUrlForward.java:26:23:26:25 | url | UnsafeUrlForward.java:25:21:25:30 | url : String | UnsafeUrlForward.java:26:23:26:25 | url | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:25:21:25:30 | url | user-provided value |
@@ -29,3 +60,4 @@ subpaths
 | UnsafeUrlForward.java:31:61:31:63 | url | UnsafeUrlForward.java:30:27:30:36 | url : String | UnsafeUrlForward.java:31:61:31:63 | url | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:30:27:30:36 | url | user-provided value |
 | UnsafeUrlForward.java:38:33:38:35 | url | UnsafeUrlForward.java:36:19:36:28 | url : String | UnsafeUrlForward.java:38:33:38:35 | url | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:36:19:36:28 | url | user-provided value |
 | UnsafeUrlForward.java:49:33:49:62 | ... + ... | UnsafeUrlForward.java:47:19:47:28 | url : String | UnsafeUrlForward.java:49:33:49:62 | ... + ... | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:47:19:47:28 | url | user-provided value |
+| UnsafeUrlForward.java:60:33:60:62 | ... + ... | UnsafeUrlForward.java:58:19:58:28 | url : String | UnsafeUrlForward.java:60:33:60:62 | ... + ... | Potentially untrusted URL forward due to $@. | UnsafeUrlForward.java:58:19:58:28 | url | user-provided value |

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

@@ -54,6 +54,17 @@ public class UnsafeUrlForward {
 		}
 	}
 
+	@GetMapping("/bad7")
+	public void bad7(String url, HttpServletRequest request, HttpServletResponse response) {
+		try {
+			request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").forward(request, response);
+		} catch (ServletException e) {
+			e.printStackTrace();
+		} catch (IOException e) {
+			e.printStackTrace();
+		}
+	}
+
 	@GetMapping("/good1")
 	public void good1(String url, HttpServletRequest request, HttpServletResponse response) {
 		try {

java/ql/test/stubs/servlet-api-2.4/javax/servlet/http/HttpServletRequest.java

@@ -25,6 +25,7 @@ package javax.servlet.http;
 
 import java.util.Enumeration;
 import javax.servlet.ServletRequest;
+import javax.servlet.ServletContext;
 
 public interface HttpServletRequest extends ServletRequest {
     public String getAuthType();
@@ -52,4 +53,5 @@ public interface HttpServletRequest extends ServletRequest {
     public boolean isRequestedSessionIdFromCookie();
     public boolean isRequestedSessionIdFromURL();
     public boolean isRequestedSessionIdFromUrl();
+    public ServletContext getServletContext();
 }