Java: rename 'UnsafeUrlForward' to 'UrlForward'

java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll → java/ql/lib/semmle/code/java/security/UrlForward.qll

@@ -1,23 +1,23 @@
-/** Provides classes related to unsafe URL forwarding in Java. */
+/** Provides classes to reason about URL forward attacks. */
 
 import java
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.dataflow.StringPrefixes
 
-/** A sink for unsafe URL forward vulnerabilities. */
-abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
+/** A URL forward sink. */
+abstract class UrlForwardSink extends DataFlow::Node { }
 
-/** A default sink representing methods susceptible to unsafe URL forwarding. */
-private class DefaultUnsafeUrlForwardSink extends UnsafeUrlForwardSink {
-  DefaultUnsafeUrlForwardSink() { sinkNode(this, "url-forward") }
+/** A default sink representing methods susceptible to URL forwarding attacks. */
+private class DefaultUrlForwardSink extends UrlForwardSink {
+  DefaultUrlForwardSink() { sinkNode(this, "url-forward") }
 }
 
 /**
  * An expression appended (perhaps indirectly) to `"forward:"`, and which
  * is reachable from a Spring entry point.
  */
-private class SpringUrlForwardSink extends UnsafeUrlForwardSink {
+private class SpringUrlForwardSink extends UrlForwardSink {
   SpringUrlForwardSink() {
     // TODO: check if can use MaD "Annotated" for `SpringRequestMappingMethod` or if too complicated for MaD (probably too complicated).
     any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable()) and
@@ -32,10 +32,10 @@ private class ForwardPrefix extends InterestingPrefix {
   override int getOffset() { result = 0 }
 }
 
-/** A sanitizer for unsafe URL forward vulnerabilities. */
-abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { }
+/** A URL forward sanitizer. */
+abstract class UrlForwardSanitizer extends DataFlow::Node { }
 
-private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
+private class PrimitiveSanitizer extends UrlForwardSanitizer {
   PrimitiveSanitizer() {
     this.getType() instanceof PrimitiveType or
     this.getType() instanceof BoxedType or
@@ -44,7 +44,7 @@ private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
 }
 
 // TODO: double-check this sanitizer (and should I switch all "sanitizer" naming to "barrier" instead?)
-private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
+private class FollowsSanitizingPrefix extends UrlForwardSanitizer {
   FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
 }
 

java/ql/lib/semmle/code/java/security/UnsafeUrlForwardQuery.qll → java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll

@@ -1,18 +1,18 @@
-/** Provides configurations to be used in queries related to unsafe URL forwarding. */
+/** Provides a taint-tracking configuration for reasoning about URL forwarding. */
 
 import java
-import semmle.code.java.security.UnsafeUrlForward
+import semmle.code.java.security.UrlForward
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.security.PathSanitizer
 
 /**
- * A taint-tracking configuration for untrusted user input in a URL forward or include.
+ * A taint-tracking configuration for reasoning about URL forwarding.
  */
-module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
+module UrlForwardFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
     source instanceof ThreatModelFlowSource and
-    // TODO: move below logic to class in UnsafeUrlForward.qll? And check exactly why these were excluded.
+    // TODO: move below logic to class in UrlForward.qll? And check exactly why these were excluded.
     not exists(MethodCall ma, Method m | ma.getMethod() = m |
       (
         m instanceof HttpServletRequestGetRequestUriMethod or
@@ -23,10 +23,10 @@ module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
     )
   }
 
-  predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeUrlForwardSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof UrlForwardSink }
 
   predicate isBarrier(DataFlow::Node node) {
-    node instanceof UnsafeUrlForwardSanitizer or
+    node instanceof UrlForwardSanitizer or
     node instanceof PathInjectionSanitizer
   }
 
@@ -35,6 +35,6 @@ module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
 }
 
 /**
- * Taint-tracking flow for untrusted user input in a URL forward or include.
+ * Taint-tracking flow for URL forwarding.
  */
-module UnsafeUrlForwardFlow = TaintTracking::Global<UnsafeUrlForwardFlowConfig>;
+module UrlForwardFlow = TaintTracking::Global<UrlForwardFlowConfig>;

java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.java → java/ql/src/Security/CWE/CWE-552/UrlForward.java

@@ -7,7 +7,7 @@
 import org.springframework.web.servlet.ModelAndView;
 
 @Controller
-public class UnsafeUrlForward {
+public class UrlForward {
 
 	@GetMapping("/bad1")
 	public ModelAndView bad1(String url) {

java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.qhelp → java/ql/src/Security/CWE/CWE-552/UrlForward.qhelp

@@ -27,7 +27,7 @@ without validating the input, which may cause file leakage. In the <code>good1</
 ordinary forwarding requests are shown, which will not cause file leakage.
 </p>
 
-<sample src="UnsafeUrlForward.java" />
+<sample src="UrlForward.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

java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.ql → java/ql/src/Security/CWE/CWE-552/UrlForward.ql

@@ -14,10 +14,10 @@
  */
 
 import java
-import semmle.code.java.security.UnsafeUrlForwardQuery
-import UnsafeUrlForwardFlow::PathGraph
+import semmle.code.java.security.UrlForwardQuery
+import UrlForwardFlow::PathGraph
 
-from UnsafeUrlForwardFlow::PathNode source, UnsafeUrlForwardFlow::PathNode sink
-where UnsafeUrlForwardFlow::flowPath(source, sink)
+from UrlForwardFlow::PathNode source, UrlForwardFlow::PathNode sink
+where UrlForwardFlow::flowPath(source, sink)
 select sink.getNode(), source, sink, "Untrusted URL forward depends on a $@.", source.getNode(),
   "user-provided value"

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

@@ -20,7 +20,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
 		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); // $ hasUnsafeUrlForward
+			request.getRequestDispatcher(path).forward(request, response); // $ hasUrlForward
 		} else {
 			chain.doFilter(request, response);
 		}

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

@@ -29,7 +29,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
 			rd.forward(request, response);
 		} else {
 			ServletContext sc = cfg.getServletContext();
-			RequestDispatcher rd = sc.getRequestDispatcher(returnURL); // $ hasUnsafeUrlForward
+			RequestDispatcher rd = sc.getRequestDispatcher(returnURL); // $ hasUrlForward
 			rd.forward(request, response);
 		}
 	}
@@ -45,7 +45,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
 			RequestDispatcher rd = request.getRequestDispatcher("/Login.jsp");
 			rd.forward(request, response);
 		} else {
-			RequestDispatcher rd = request.getRequestDispatcher(returnURL); // $ hasUnsafeUrlForward
+			RequestDispatcher rd = request.getRequestDispatcher(returnURL); // $ hasUrlForward
 			rd.forward(request, response);
 		}
 	}
@@ -73,7 +73,7 @@ protected void doHead2(HttpServletRequest request, HttpServletResponse response)
 		// 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); // $ hasUnsafeUrlForward
+			request.getServletContext().getRequestDispatcher(path).include(request, response); // $ hasUrlForward
 		}
 	}
 
@@ -110,7 +110,7 @@ protected void doHead5(HttpServletRequest request, HttpServletResponse response)
 		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); // $ MISSING: hasUnsafeUrlForward
+			request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response); // $ MISSING: hasUrlForward
 		}
 	}
 

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

@@ -7,35 +7,35 @@
 import org.springframework.web.servlet.ModelAndView;
 
 @Controller
-public class UnsafeUrlForward {
+public class UrlForwardTest {
 
 	@GetMapping("/bad1")
 	public ModelAndView bad1(String url) {
-		return new ModelAndView(url); // $ hasUnsafeUrlForward
+		return new ModelAndView(url); // $ hasUrlForward
 	}
 
 	@GetMapping("/bad2")
 	public ModelAndView bad2(String url) {
 		ModelAndView modelAndView = new ModelAndView();
-		modelAndView.setViewName(url); // $ hasUnsafeUrlForward
+		modelAndView.setViewName(url); // $ hasUrlForward
 		return modelAndView;
 	}
 
 	@GetMapping("/bad3")
 	public String bad3(String url) {
-		return "forward:" + url + "/swagger-ui/index.html"; // $ hasUnsafeUrlForward
+		return "forward:" + url + "/swagger-ui/index.html"; // $ hasUrlForward
 	}
 
 	@GetMapping("/bad4")
 	public ModelAndView bad4(String url) {
-		ModelAndView modelAndView = new ModelAndView("forward:" + url); // $ hasUnsafeUrlForward
+		ModelAndView modelAndView = new ModelAndView("forward:" + url); // $ hasUrlForward
 		return modelAndView;
 	}
 
 	@GetMapping("/bad5")
 	public void bad5(String url, HttpServletRequest request, HttpServletResponse response) {
 		try {
-			request.getRequestDispatcher(url).include(request, response); // $ hasUnsafeUrlForward
+			request.getRequestDispatcher(url).include(request, response); // $ hasUrlForward
 		} catch (ServletException e) {
 			e.printStackTrace();
 		} catch (IOException e) {
@@ -46,7 +46,7 @@ public void bad5(String url, HttpServletRequest request, HttpServletResponse res
 	@GetMapping("/bad6")
 	public void bad6(String url, HttpServletRequest request, HttpServletResponse response) {
 		try {
-			request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").include(request, response); // $ hasUnsafeUrlForward
+			request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").include(request, response); // $ hasUrlForward
 		} catch (ServletException e) {
 			e.printStackTrace();
 		} catch (IOException e) {
@@ -57,7 +57,7 @@ public void bad6(String url, HttpServletRequest request, HttpServletResponse res
 	@GetMapping("/bad7")
 	public void bad7(String url, HttpServletRequest request, HttpServletResponse response) {
 		try {
-			request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").forward(request, response); // $ hasUnsafeUrlForward
+			request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").forward(request, response); // $ hasUrlForward
 		} catch (ServletException e) {
 			e.printStackTrace();
 		} catch (IOException e) {

java/ql/test/query-tests/security/CWE-552/UrlForwardTest.ql

@@ -0,0 +1,18 @@
+import java
+import TestUtilities.InlineExpectationsTest
+import semmle.code.java.security.UrlForwardQuery
+
+module UrlForwardTest implements TestSig {
+  string getARelevantTag() { result = "hasUrlForward" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasUrlForward" and
+    exists(UrlForwardFlow::PathNode sink | UrlForwardFlow::flowPath(_, sink) |
+      location = sink.getNode().getLocation() and
+      element = sink.getNode().toString() and
+      value = ""
+    )
+  }
+}
+
+import MakeTest<UrlForwardTest>