Java: Add Spring MVC URL redirect sinks

java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md

@@ -0,0 +1,6 @@
+---
+category: minorAnalysis
+---
+* The `java/unvalidated-url-redirection` query now detects untrusted URLs used in Spring MVC
+  `RedirectView` objects and `redirect:` view names, including view names constructed in helper
+  methods called by request handlers.

java/ql/lib/semmle/code/java/security/UrlRedirect.qll

@@ -8,7 +8,10 @@ import semmle.code.java.frameworks.Servlets
 import semmle.code.java.frameworks.ApacheHttp
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSinks
+private import semmle.code.java.dataflow.StringPrefixes
+private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.frameworks.JaxWS
+private import semmle.code.java.frameworks.spring.SpringController
 private import semmle.code.java.security.RequestForgery
 
 /** A URL redirection sink. */
@@ -50,5 +53,131 @@ private class ApacheUrlRedirectSink extends UrlRedirectSink {
   }
 }
 
+/** An expression appended to a Spring `"redirect:"` view-name returned by a request handler. */
+private class SpringUrlRedirectPrefixSink extends UrlRedirectSink {
+  SpringUrlRedirectPrefixSink() {
+    appendedToRedirectPrefix(this) and
+    (
+      isSpringMvcReturnedString(this.asExpr())
+      or
+      isSpringModelAndViewName(this.asExpr())
+    )
+  }
+}
+
+/** A call to a helper that returns a Spring `"redirect:"` view name. */
+private class SpringUrlRedirectHelperSink extends UrlRedirectSink {
+  SpringUrlRedirectHelperSink() {
+    exists(MethodCall call |
+      this.asExpr() = call and
+      isSpringMvcViewResult(call) and
+      returnsRedirectViewName(call.getCallee().getSourceDeclaration())
+    )
+  }
+}
+
+pragma[nomagic]
+private predicate appendedToRedirectPrefix(DataFlow::ExprNode exprNode) {
+  exists(SpringRedirectPrefix prefix | exprNode.asExpr() = prefix.getAnAppendedExpression())
+}
+
+private class SpringRedirectPrefix extends InterestingPrefix {
+  SpringRedirectPrefix() { this.getStringValue() = "redirect:" }
+
+  override int getOffset() { result = 0 }
+}
+
+private predicate contributesToReturn(Expr value) {
+  exists(ReturnStmt ret |
+    ret.getEnclosingCallable() = value.getEnclosingCallable() and
+    DataFlow::localFlow(DataFlow::exprNode(value), DataFlow::exprNode(ret.getExpr()))
+  )
+}
+
+/** Holds if `value` contributes string content to the return value of its callable. */
+private predicate contributesToReturnedString(Expr value) {
+  exists(ReturnStmt ret |
+    ret.getEnclosingCallable() = value.getEnclosingCallable() and
+    TaintTracking::localTaint(DataFlow::exprNode(value), DataFlow::exprNode(ret.getExpr()))
+  )
+}
+
+/** Holds if `value` contributes to a view returned from a Spring MVC request handler. */
+pragma[nomagic]
+private predicate isSpringMvcViewResult(Expr value) {
+  contributesToReturn(value) and
+  value.getEnclosingCallable() instanceof SpringRequestMappingMethod and
+  not value.getEnclosingCallable().(SpringRequestMappingMethod).isResponseBody()
+}
+
+/** Holds if `value` contributes string content to a view name returned by a request handler. */
+private predicate isSpringMvcReturnedString(Expr value) {
+  contributesToReturnedString(value) and
+  value.getEnclosingCallable() instanceof SpringRequestMappingMethod and
+  not value.getEnclosingCallable().(SpringRequestMappingMethod).isResponseBody()
+}
+
+/** Holds if `value` contributes to the view name of a returned `ModelAndView`. */
+private predicate isSpringModelAndViewName(Expr value) {
+  exists(ClassInstanceExpr newModelAndView |
+    newModelAndView
+        .getConstructedType()
+        .hasQualifiedName("org.springframework.web.servlet", "ModelAndView") and
+    TaintTracking::localTaint(DataFlow::exprNode(value),
+      DataFlow::exprNode(newModelAndView.getArgument(0))) and
+    isSpringMvcViewResult(newModelAndView)
+  )
+}
+
+/** Holds if `callable` returns a view name constructed with the Spring `"redirect:"` prefix. */
+pragma[nomagic]
+private predicate returnsRedirectViewName(Callable callable) {
+  exists(DataFlow::ExprNode appended, ReturnStmt ret |
+    appendedToRedirectPrefix(appended) and
+    appended.asExpr().getEnclosingCallable() = callable and
+    ret.getEnclosingCallable() = callable and
+    TaintTracking::localTaint(appended, DataFlow::exprNode(ret.getExpr()))
+  )
+  or
+  exists(MethodCall call |
+    call.getEnclosingCallable() = callable and
+    contributesToReturn(call) and
+    returnsRedirectViewName(call.getCallee().getSourceDeclaration())
+  )
+}
+
+/** A URL passed to a Spring `RedirectView` constructor. */
+private class SpringRedirectViewSink extends UrlRedirectSink {
+  SpringRedirectViewSink() {
+    exists(ClassInstanceExpr newRedirectView |
+      newRedirectView
+          .getConstructedType()
+          .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and
+      isSpringMvcViewResult(newRedirectView) and
+      this.asExpr() = newRedirectView.getArgument(0)
+    )
+  }
+}
+
+/** A URL passed to `setUrl` on a Spring `RedirectView` that is returned by a request handler. */
+private class SpringRedirectViewSetUrlSink extends UrlRedirectSink {
+  SpringRedirectViewSetUrlSink() {
+    exists(MethodCall setUrl |
+      setUrl
+          .getMethod()
+          .getSourceDeclaration()
+          .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView", "setUrl") and
+      setUrl
+          .getQualifier()
+          .getType()
+          .(RefType)
+          .getASupertype*()
+          .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and
+      isSpringMvcViewResult(setUrl.getQualifier()) and
+      this.asExpr() = setUrl.getArgument(0)
+    )
+  }
+}
+
 private class DefaultUrlRedirectSanitizer extends UrlRedirectSanitizer instanceof RequestForgerySanitizer
 { }

java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java

@@ -0,0 +1,161 @@
+package test.cwe601.cwe.examples;
+
+import javax.servlet.http.HttpServletRequest;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.servlet.ModelAndView;
+import org.springframework.web.servlet.view.RedirectView;
+
+class BaseController {
+    protected String redirect(String path) {
+        return "redirect:" + path;
+    }
+
+    protected String ordinaryView(String path) {
+        return "view:" + path;
+    }
+
+    protected String discardedRedirect(String path) {
+        return "redirect:" + path;
+    }
+}
+
+class LabeledRedirectView extends RedirectView {
+    LabeledRedirectView(String label, String url) {
+        super(url);
+    }
+
+    void setUrl(Object label) {
+    }
+}
+
+@Controller
+public class SpringUrlRedirect extends BaseController {
+    @GetMapping("/case1")
+    public String directViewName(HttpServletRequest request) {
+        String next = request.getHeader("referer"); // $ Source
+        return "redirect:" + next; // $ Alert
+    }
+
+    @GetMapping("/case2")
+    public ModelAndView modelAndView(HttpServletRequest request) {
+        String next = request.getHeader("referer"); // $ Source
+        return new ModelAndView("redirect:" + next); // $ Alert
+    }
+
+    @GetMapping("/case3")
+    public RedirectView redirectView(HttpServletRequest request) {
+        String next = request.getHeader("referer"); // $ Source
+        return new RedirectView(next); // $ Alert
+    }
+
+    @GetMapping("/case4")
+    public String helperViewName(HttpServletRequest request) {
+        String next = request.getHeader("referer"); // $ Source
+        return redirect(next); // $ Alert
+    }
+
+    @GetMapping("/case5")
+    public RedirectView configuredRedirectView(HttpServletRequest request) {
+        String next = request.getParameter("next"); // $ Source
+        RedirectView view = new RedirectView();
+        view.setUrl(next); // $ Alert
+        return view;
+    }
+
+    @GetMapping("/case6")
+    public RedirectView overloadedRedirectView(HttpServletRequest request) {
+        String next = request.getParameter("next"); // $ Source
+        return new RedirectView(next, true); // $ Alert
+    }
+
+    @GetMapping("/safe-constant")
+    public String constantViewName() {
+        return "redirect:/account";
+    }
+
+    @GetMapping("/safe-path")
+    public String fixedPath(HttpServletRequest request) {
+        String tab = request.getParameter("tab");
+        return "redirect:/account?tab=" + tab;
+    }
+
+    @GetMapping("/safe-model-and-view")
+    public ModelAndView ordinaryModelAndView(HttpServletRequest request) {
+        String view = request.getParameter("view");
+        return new ModelAndView(ordinaryView(view));
+    }
+
+    @GetMapping("/safe-model-value")
+    public ModelAndView redirectInModel(HttpServletRequest request) {
+        String value = "redirect:" + request.getParameter("value");
+        return new ModelAndView("home", "value", value);
+    }
+
+    @GetMapping("/safe-discarded")
+    public String discardedRedirectValue(HttpServletRequest request) {
+        discardedRedirect(request.getParameter("next"));
+        return "home";
+    }
+
+    @GetMapping("/safe-redirect-view")
+    public RedirectView constantRedirectView() {
+        return new RedirectView("https://example.com/account");
+    }
+
+    @GetMapping("/safe-subclass-label")
+    public RedirectView customRedirectView(HttpServletRequest request) {
+        String label = request.getParameter("label");
+        return new LabeledRedirectView(label, "/account");
+    }
+
+    @GetMapping("/safe-set-url-overload")
+    public RedirectView customSetUrl(HttpServletRequest request) {
+        LabeledRedirectView view = new LabeledRedirectView("label", "/account");
+        Object label = request.getParameter("label");
+        view.setUrl(label);
+        return view;
+    }
+}
+
+@Controller
+class ResponseBodyController {
+    @ResponseBody
+    @GetMapping("/body")
+    public String responseBody(HttpServletRequest request) {
+        return "redirect:" + request.getParameter("value");
+    }
+}
+
+@RestController
+class JsonController {
+    @GetMapping("/json")
+    public String responseBody(HttpServletRequest request) {
+        return "redirect:" + request.getParameter("value");
+    }
+}
+
+class SharedRedirectHelper {
+    protected String redirectShared(String path) {
+        return "redirect:" + path;
+    }
+}
+
+@Controller
+class MvcViewUser extends SharedRedirectHelper {
+    @GetMapping("/constant-view")
+    public String view() {
+        return redirectShared("/account");
+    }
+}
+
+@Controller
+class ResponseBodyUser extends SharedRedirectHelper {
+    @ResponseBody
+    @GetMapping("/body-user")
+    public String body(HttpServletRequest request) {
+        return redirectShared(request.getParameter("value"));
+    }
+}

java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected

@@ -1,22 +1,53 @@
 #select
+| SpringUrlRedirect.java:39:30:39:33 | next | SpringUrlRedirect.java:38:23:38:50 | getHeader(...) : String | SpringUrlRedirect.java:39:30:39:33 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:38:23:38:50 | getHeader(...) | user-provided value |
+| SpringUrlRedirect.java:45:47:45:50 | next | SpringUrlRedirect.java:44:23:44:50 | getHeader(...) : String | SpringUrlRedirect.java:45:47:45:50 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:44:23:44:50 | getHeader(...) | user-provided value |
+| SpringUrlRedirect.java:51:33:51:36 | next | SpringUrlRedirect.java:50:23:50:50 | getHeader(...) : String | SpringUrlRedirect.java:51:33:51:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:50:23:50:50 | getHeader(...) | user-provided value |
+| SpringUrlRedirect.java:57:16:57:29 | redirect(...) | SpringUrlRedirect.java:56:23:56:50 | getHeader(...) : String | SpringUrlRedirect.java:57:16:57:29 | redirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:56:23:56:50 | getHeader(...) | user-provided value |
+| SpringUrlRedirect.java:64:21:64:24 | next | SpringUrlRedirect.java:62:23:62:50 | getParameter(...) : String | SpringUrlRedirect.java:64:21:64:24 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:62:23:62:50 | getParameter(...) | user-provided value |
+| SpringUrlRedirect.java:71:33:71:36 | next | SpringUrlRedirect.java:70:23:70:50 | getParameter(...) : String | SpringUrlRedirect.java:71:33:71:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:70:23:70:50 | getParameter(...) | user-provided value |
 | UrlRedirect2.java:27:25:27:54 | getParameter(...) | UrlRedirect2.java:27:25:27:54 | getParameter(...) | UrlRedirect2.java:27:25:27:54 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect2.java:27:25:27:54 | getParameter(...) | user-provided value |
 | UrlRedirect.java:23:25:23:54 | getParameter(...) | UrlRedirect.java:23:25:23:54 | getParameter(...) | UrlRedirect.java:23:25:23:54 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:23:25:23:54 | getParameter(...) | user-provided value |
 | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:32:37:32:66 | getParameter(...) | user-provided value |
 | UrlRedirect.java:39:34:39:63 | getParameter(...) | UrlRedirect.java:39:34:39:63 | getParameter(...) | UrlRedirect.java:39:34:39:63 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:39:34:39:63 | getParameter(...) | user-provided value |
 | UrlRedirect.java:42:43:42:72 | getParameter(...) | UrlRedirect.java:42:43:42:72 | getParameter(...) | UrlRedirect.java:42:43:42:72 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:42:43:42:72 | getParameter(...) | user-provided value |
 | mad/Test.java:14:22:14:38 | (...)... | mad/Test.java:9:16:9:41 | getParameter(...) : String | mad/Test.java:14:22:14:38 | (...)... | Untrusted URL redirection depends on a $@. | mad/Test.java:9:16:9:41 | getParameter(...) | user-provided value |
 edges
-| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | provenance | Src:MaD:2 MaD:3 |
-| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:45:28:45:39 | input : String | provenance | Src:MaD:2  |
+| SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | provenance |  |
+| SpringUrlRedirect.java:38:23:38:50 | getHeader(...) : String | SpringUrlRedirect.java:39:30:39:33 | next | provenance | Src:MaD:2  |
+| SpringUrlRedirect.java:44:23:44:50 | getHeader(...) : String | SpringUrlRedirect.java:45:47:45:50 | next | provenance | Src:MaD:2  |
+| SpringUrlRedirect.java:50:23:50:50 | getHeader(...) : String | SpringUrlRedirect.java:51:33:51:36 | next | provenance | Src:MaD:2  |
+| SpringUrlRedirect.java:56:23:56:50 | getHeader(...) : String | SpringUrlRedirect.java:57:25:57:28 | next : String | provenance | Src:MaD:2  |
+| SpringUrlRedirect.java:57:25:57:28 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance |  |
+| SpringUrlRedirect.java:57:25:57:28 | next : String | SpringUrlRedirect.java:57:16:57:29 | redirect(...) | provenance |  |
+| SpringUrlRedirect.java:62:23:62:50 | getParameter(...) : String | SpringUrlRedirect.java:64:21:64:24 | next | provenance | Src:MaD:3  |
+| SpringUrlRedirect.java:70:23:70:50 | getParameter(...) : String | SpringUrlRedirect.java:71:33:71:36 | next | provenance | Src:MaD:3  |
+| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | provenance | Src:MaD:3 MaD:4 |
+| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:45:28:45:39 | input : String | provenance | Src:MaD:3  |
 | UrlRedirect.java:45:28:45:39 | input : String | UrlRedirect.java:46:10:46:14 | input : String | provenance |  |
-| UrlRedirect.java:46:10:46:14 | input : String | UrlRedirect.java:46:10:46:40 | replaceAll(...) : String | provenance | MaD:3 |
-| mad/Test.java:9:16:9:41 | getParameter(...) : String | mad/Test.java:14:31:14:38 | source(...) : String | provenance | Src:MaD:2  |
+| UrlRedirect.java:46:10:46:14 | input : String | UrlRedirect.java:46:10:46:40 | replaceAll(...) : String | provenance | MaD:4 |
+| mad/Test.java:9:16:9:41 | getParameter(...) : String | mad/Test.java:14:31:14:38 | source(...) : String | provenance | Src:MaD:3  |
 | mad/Test.java:14:31:14:38 | source(...) : String | mad/Test.java:14:22:14:38 | (...)... | provenance | Sink:MaD:1 |
 models
 | 1 | Sink: org.kohsuke.stapler; HttpResponses; true; redirectTo; (String); ; Argument[0]; url-redirection; ai-manual |
-| 2 | Source: javax.servlet; ServletRequest; false; getParameter; (String); ; ReturnValue; remote; manual |
-| 3 | Summary: java.lang; String; false; replaceAll; ; ; Argument[this]; ReturnValue; taint; manual |
+| 2 | Source: javax.servlet.http; HttpServletRequest; false; getHeader; (String); ; ReturnValue; remote; manual |
+| 3 | Source: javax.servlet; ServletRequest; false; getParameter; (String); ; ReturnValue; remote; manual |
+| 4 | Summary: java.lang; String; false; replaceAll; ; ; Argument[this]; ReturnValue; taint; manual |
 nodes
+| SpringUrlRedirect.java:12:31:12:41 | path : String | semmle.label | path : String |
+| SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | semmle.label | ... + ... : String |
+| SpringUrlRedirect.java:38:23:38:50 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| SpringUrlRedirect.java:39:30:39:33 | next | semmle.label | next |
+| SpringUrlRedirect.java:44:23:44:50 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| SpringUrlRedirect.java:45:47:45:50 | next | semmle.label | next |
+| SpringUrlRedirect.java:50:23:50:50 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| SpringUrlRedirect.java:51:33:51:36 | next | semmle.label | next |
+| SpringUrlRedirect.java:56:23:56:50 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| SpringUrlRedirect.java:57:16:57:29 | redirect(...) | semmle.label | redirect(...) |
+| SpringUrlRedirect.java:57:25:57:28 | next : String | semmle.label | next : String |
+| SpringUrlRedirect.java:62:23:62:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| SpringUrlRedirect.java:64:21:64:24 | next | semmle.label | next |
+| SpringUrlRedirect.java:70:23:70:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| SpringUrlRedirect.java:71:33:71:36 | next | semmle.label | next |
 | UrlRedirect2.java:27:25:27:54 | getParameter(...) | semmle.label | getParameter(...) |
 | UrlRedirect.java:23:25:23:54 | getParameter(...) | semmle.label | getParameter(...) |
 | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | semmle.label | weakCleanup(...) |
@@ -30,4 +61,5 @@ nodes
 | mad/Test.java:14:22:14:38 | (...)... | semmle.label | (...)... |
 | mad/Test.java:14:31:14:38 | source(...) : String | semmle.label | source(...) : String |
 subpaths
+| SpringUrlRedirect.java:57:25:57:28 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | SpringUrlRedirect.java:57:16:57:29 | redirect(...) |
 | UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:45:28:45:39 | input : String | UrlRedirect.java:46:10:46:40 | replaceAll(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) |