From 3a4deee656a34c21bd278b4b6529149cfdf82439 Mon Sep 17 00:00:00 2001 From: Keshav Malik Date: Mon, 17 Aug 2026 15:57:42 +0530 Subject: [PATCH 1/3] Java: Add Spring MVC URL redirect sinks --- .../2026-08-17-spring-url-redirect-sinks.md | 6 + .../semmle/code/java/security/UrlRedirect.qll | 84 ++++++++++++ .../semmle/tests/SpringUrlRedirect.java | 125 ++++++++++++++++++ .../CWE-601/semmle/tests/UrlRedirect.expected | 45 ++++++- .../security/CWE-601/semmle/tests/options | 2 +- 5 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md create mode 100644 java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java diff --git a/java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md b/java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md new file mode 100644 index 000000000000..7e9200ff16b7 --- /dev/null +++ b/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. diff --git a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll index be6addfa2529..b6fe7c5b02a0 100644 --- a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll +++ b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll @@ -8,7 +8,9 @@ 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.frameworks.JaxWS +private import semmle.code.java.frameworks.spring.SpringController private import semmle.code.java.security.RequestForgery /** A URL redirection sink. */ @@ -50,5 +52,87 @@ private class ApacheUrlRedirectSink extends UrlRedirectSink { } } +/** + * An expression appended to a Spring `"redirect:"` view-name prefix from a request handler or a + * helper called by one. + */ +private class SpringUrlRedirectPrefixSink extends UrlRedirectSink { + SpringUrlRedirectPrefixSink() { + isSpringMvcViewResult(this.asExpr()) and + appendedToRedirectPrefix(this) + } +} + +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 + ( + value.getParent*() = ret.getExpr() + or + DataFlow::localFlow(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() + or + contributesToReturn(value) and + exists(MethodCall call | + call.getCallee().getSourceDeclaration() = value.getEnclosingCallable() and + isSpringMvcViewResult(call) + ) +} + +private class SpringRedirectViewType extends RefType { + SpringRedirectViewType() { + this.getASupertype*().hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") + } +} + +/** A URL passed to a Spring `RedirectView` constructor. */ +private class SpringRedirectViewSink extends UrlRedirectSink { + SpringRedirectViewSink() { + exists(ClassInstanceExpr newRedirectView | + newRedirectView.getConstructedType() instanceof SpringRedirectViewType 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().hasName("setUrl") and + setUrl.getMethod().getNumberOfParameters() = 1 and + setUrl + .getMethod() + .getDeclaringType() + .getASupertype*() + .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView") and + setUrl.getQualifier().getType() instanceof SpringRedirectViewType and + isSpringMvcViewResult(setUrl.getQualifier()) and + this.asExpr() = setUrl.getArgument(0) + ) + } +} + private class DefaultUrlRedirectSanitizer extends UrlRedirectSanitizer instanceof RequestForgerySanitizer { } diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java b/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java new file mode 100644 index 000000000000..e997d93e8ad9 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java @@ -0,0 +1,125 @@ +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; // $ Alert + } + + protected String ordinaryView(String path) { + return "view:" + path; + } + + protected String discardedRedirect(String path) { + return "redirect:" + path; + } +} + +class CustomRedirectView extends RedirectView { + CustomRedirectView(String url) { + super(url); + } +} + +@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); + } + + @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("/case7") + public RedirectView customRedirectView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new CustomRedirectView(next); // $ 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-discarded") + public String discardedRedirectValue(HttpServletRequest request) { + discardedRedirect(request.getParameter("next")); + return "home"; + } + + @GetMapping("/safe-validated") + public RedirectView validatedRedirectView(HttpServletRequest request) { + String next = request.getParameter("next"); + if ("https://example.com/account".equals(next)) { + return new RedirectView("https://example.com/account"); + } + return new RedirectView("https://example.com/account"); + } +} + +@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"); + } +} diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected b/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected index fdd602c616bf..7b4039beba70 100644 --- a/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected @@ -1,4 +1,11 @@ #select +| SpringUrlRedirect.java:13:30:13:33 | path | SpringUrlRedirect.java:53:23:53:50 | getHeader(...) : String | SpringUrlRedirect.java:13:30:13:33 | path | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:53:23:53:50 | getHeader(...) | user-provided value | +| SpringUrlRedirect.java:36:30:36:33 | next | SpringUrlRedirect.java:35:23:35:50 | getHeader(...) : String | SpringUrlRedirect.java:36:30:36:33 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:35:23:35:50 | getHeader(...) | user-provided value | +| SpringUrlRedirect.java:42:47:42:50 | next | SpringUrlRedirect.java:41:23:41:50 | getHeader(...) : String | SpringUrlRedirect.java:42:47:42:50 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:41:23:41:50 | getHeader(...) | user-provided value | +| SpringUrlRedirect.java:48:33:48:36 | next | SpringUrlRedirect.java:47:23:47:50 | getHeader(...) : String | SpringUrlRedirect.java:48:33:48:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:47:23:47:50 | getHeader(...) | user-provided value | +| SpringUrlRedirect.java:61:21:61:24 | next | SpringUrlRedirect.java:59:23:59:50 | getParameter(...) : String | SpringUrlRedirect.java:61:21:61:24 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:59:23:59:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:68:33:68:36 | next | SpringUrlRedirect.java:67:23:67:50 | getParameter(...) : String | SpringUrlRedirect.java:68:33:68:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:67:23:67:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:74:39:74:42 | next | SpringUrlRedirect.java:73:23:73:50 | getParameter(...) : String | SpringUrlRedirect.java:74:39:74:42 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:73:23:73: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 | @@ -6,17 +13,43 @@ | 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:30:13:33 | path | provenance | | +| SpringUrlRedirect.java:35:23:35:50 | getHeader(...) : String | SpringUrlRedirect.java:36:30:36:33 | next | provenance | Src:MaD:2 | +| SpringUrlRedirect.java:41:23:41:50 | getHeader(...) : String | SpringUrlRedirect.java:42:47:42:50 | next | provenance | Src:MaD:2 | +| SpringUrlRedirect.java:47:23:47:50 | getHeader(...) : String | SpringUrlRedirect.java:48:33:48:36 | next | provenance | Src:MaD:2 | +| SpringUrlRedirect.java:53:23:53:50 | getHeader(...) : String | SpringUrlRedirect.java:54:25:54:28 | next : String | provenance | Src:MaD:2 | +| SpringUrlRedirect.java:54:25:54:28 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance | | +| SpringUrlRedirect.java:59:23:59:50 | getParameter(...) : String | SpringUrlRedirect.java:61:21:61:24 | next | provenance | Src:MaD:3 | +| SpringUrlRedirect.java:67:23:67:50 | getParameter(...) : String | SpringUrlRedirect.java:68:33:68:36 | next | provenance | Src:MaD:3 | +| SpringUrlRedirect.java:73:23:73:50 | getParameter(...) : String | SpringUrlRedirect.java:74:39:74:42 | 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:30:13:33 | path | semmle.label | path | +| SpringUrlRedirect.java:35:23:35:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | +| SpringUrlRedirect.java:36:30:36:33 | next | semmle.label | next | +| SpringUrlRedirect.java:41:23:41:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | +| SpringUrlRedirect.java:42:47:42:50 | next | semmle.label | next | +| SpringUrlRedirect.java:47:23:47:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | +| SpringUrlRedirect.java:48:33:48:36 | next | semmle.label | next | +| SpringUrlRedirect.java:53:23:53:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | +| SpringUrlRedirect.java:54:25:54:28 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:59:23:59:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:61:21:61:24 | next | semmle.label | next | +| SpringUrlRedirect.java:67:23:67:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:68:33:68:36 | next | semmle.label | next | +| SpringUrlRedirect.java:73:23:73:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:74:39:74:42 | 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(...) | diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/options b/java/ql/test/query-tests/security/CWE-601/semmle/tests/options index 637c329a9143..804ee350df4e 100644 --- a/java/ql/test/query-tests/security/CWE-601/semmle/tests/options +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/servlet-api-2.4:${testdir}/../../../../../stubs/stapler-1.263:${testdir}/../../../../../stubs/javax-servlet-2.5:${testdir}/../../../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../../../stubs/saxon-xqj-9.x:${testdir}/../../../../../stubs/apache-commons-beanutils:${testdir}/../../../../../stubs/dom4j-2.1.1:${testdir}/../../../../../stubs/apache-commons-lang:${testdir}/../../../../../stubs/jaxen-1.2.0 +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/servlet-api-2.4:${testdir}/../../../../../stubs/stapler-1.263:${testdir}/../../../../../stubs/javax-servlet-2.5:${testdir}/../../../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../../../stubs/saxon-xqj-9.x:${testdir}/../../../../../stubs/apache-commons-beanutils:${testdir}/../../../../../stubs/dom4j-2.1.1:${testdir}/../../../../../stubs/apache-commons-lang:${testdir}/../../../../../stubs/jaxen-1.2.0:${testdir}/../../../../../stubs/springframework-5.8.x From f231bad6520c7cad3bef71dc4d76d36445db293f Mon Sep 17 00:00:00 2001 From: Keshav Malik Date: Mon, 17 Aug 2026 17:06:25 +0530 Subject: [PATCH 2/3] Tighten Spring redirect sinks --- .../semmle/code/java/security/UrlRedirect.qll | 97 ++++++++++++++----- .../semmle/tests/SpringUrlRedirect.java | 68 ++++++++++--- .../CWE-601/semmle/tests/UrlRedirect.expected | 61 ++++++------ 3 files changed, 153 insertions(+), 73 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll index b6fe7c5b02a0..13d1911b66b0 100644 --- a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll +++ b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll @@ -9,6 +9,7 @@ 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 @@ -52,14 +53,26 @@ private class ApacheUrlRedirectSink extends UrlRedirectSink { } } -/** - * An expression appended to a Spring `"redirect:"` view-name prefix from a request handler or a - * helper called by one. - */ +/** An expression appended to a Spring `"redirect:"` view-name returned by a request handler. */ private class SpringUrlRedirectPrefixSink extends UrlRedirectSink { SpringUrlRedirectPrefixSink() { - isSpringMvcViewResult(this.asExpr()) and - appendedToRedirectPrefix(this) + 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()) + ) } } @@ -77,11 +90,15 @@ private class SpringRedirectPrefix extends InterestingPrefix { private predicate contributesToReturn(Expr value) { exists(ReturnStmt ret | ret.getEnclosingCallable() = value.getEnclosingCallable() and - ( - value.getParent*() = ret.getExpr() - or - DataFlow::localFlow(DataFlow::exprNode(value), DataFlow::exprNode(ret.getExpr())) - ) + 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())) ) } @@ -91,25 +108,51 @@ private predicate isSpringMvcViewResult(Expr value) { contributesToReturn(value) and value.getEnclosingCallable() instanceof SpringRequestMappingMethod and not value.getEnclosingCallable().(SpringRequestMappingMethod).isResponseBody() - or - contributesToReturn(value) and - exists(MethodCall call | - call.getCallee().getSourceDeclaration() = value.getEnclosingCallable() and - isSpringMvcViewResult(call) +} + +/** 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) ) } -private class SpringRedirectViewType extends RefType { - SpringRedirectViewType() { - this.getASupertype*().hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") - } +/** 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() instanceof SpringRedirectViewType and + newRedirectView + .getConstructedType() + .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and isSpringMvcViewResult(newRedirectView) and this.asExpr() = newRedirectView.getArgument(0) ) @@ -120,14 +163,16 @@ private class SpringRedirectViewSink extends UrlRedirectSink { private class SpringRedirectViewSetUrlSink extends UrlRedirectSink { SpringRedirectViewSetUrlSink() { exists(MethodCall setUrl | - setUrl.getMethod().hasName("setUrl") and - setUrl.getMethod().getNumberOfParameters() = 1 and setUrl .getMethod() - .getDeclaringType() + .getSourceDeclaration() + .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView", "setUrl") and + setUrl + .getQualifier() + .getType() + .(RefType) .getASupertype*() - .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView") and - setUrl.getQualifier().getType() instanceof SpringRedirectViewType and + .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and isSpringMvcViewResult(setUrl.getQualifier()) and this.asExpr() = setUrl.getArgument(0) ) diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java b/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java index e997d93e8ad9..d2dbb7933c78 100644 --- a/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java @@ -10,7 +10,7 @@ class BaseController { protected String redirect(String path) { - return "redirect:" + path; // $ Alert + return "redirect:" + path; } protected String ordinaryView(String path) { @@ -22,10 +22,13 @@ protected String discardedRedirect(String path) { } } -class CustomRedirectView extends RedirectView { - CustomRedirectView(String url) { +class LabeledRedirectView extends RedirectView { + LabeledRedirectView(String label, String url) { super(url); } + + void setUrl(Object label) { + } } @Controller @@ -51,7 +54,7 @@ public RedirectView redirectView(HttpServletRequest request) { @GetMapping("/case4") public String helperViewName(HttpServletRequest request) { String next = request.getHeader("referer"); // $ Source - return redirect(next); + return redirect(next); // $ Alert } @GetMapping("/case5") @@ -68,12 +71,6 @@ public RedirectView overloadedRedirectView(HttpServletRequest request) { return new RedirectView(next, true); // $ Alert } - @GetMapping("/case7") - public RedirectView customRedirectView(HttpServletRequest request) { - String next = request.getParameter("next"); // $ Source - return new CustomRedirectView(next); // $ Alert - } - @GetMapping("/safe-constant") public String constantViewName() { return "redirect:/account"; @@ -91,20 +88,36 @@ public ModelAndView ordinaryModelAndView(HttpServletRequest request) { 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-validated") - public RedirectView validatedRedirectView(HttpServletRequest request) { - String next = request.getParameter("next"); - if ("https://example.com/account".equals(next)) { - return new RedirectView("https://example.com/account"); - } + @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 @@ -123,3 +136,26 @@ 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")); + } +} diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected b/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected index 7b4039beba70..a1d45bdab204 100644 --- a/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected @@ -1,11 +1,10 @@ #select -| SpringUrlRedirect.java:13:30:13:33 | path | SpringUrlRedirect.java:53:23:53:50 | getHeader(...) : String | SpringUrlRedirect.java:13:30:13:33 | path | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:53:23:53:50 | getHeader(...) | user-provided value | -| SpringUrlRedirect.java:36:30:36:33 | next | SpringUrlRedirect.java:35:23:35:50 | getHeader(...) : String | SpringUrlRedirect.java:36:30:36:33 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:35:23:35:50 | getHeader(...) | user-provided value | -| SpringUrlRedirect.java:42:47:42:50 | next | SpringUrlRedirect.java:41:23:41:50 | getHeader(...) : String | SpringUrlRedirect.java:42:47:42:50 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:41:23:41:50 | getHeader(...) | user-provided value | -| SpringUrlRedirect.java:48:33:48:36 | next | SpringUrlRedirect.java:47:23:47:50 | getHeader(...) : String | SpringUrlRedirect.java:48:33:48:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:47:23:47:50 | getHeader(...) | user-provided value | -| SpringUrlRedirect.java:61:21:61:24 | next | SpringUrlRedirect.java:59:23:59:50 | getParameter(...) : String | SpringUrlRedirect.java:61:21:61:24 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:59:23:59:50 | getParameter(...) | user-provided value | -| SpringUrlRedirect.java:68:33:68:36 | next | SpringUrlRedirect.java:67:23:67:50 | getParameter(...) : String | SpringUrlRedirect.java:68:33:68:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:67:23:67:50 | getParameter(...) | user-provided value | -| SpringUrlRedirect.java:74:39:74:42 | next | SpringUrlRedirect.java:73:23:73:50 | getParameter(...) : String | SpringUrlRedirect.java:74:39:74:42 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:73:23:73:50 | getParameter(...) | user-provided value | +| 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 | @@ -13,15 +12,15 @@ | 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 -| SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:30:13:33 | path | provenance | | -| SpringUrlRedirect.java:35:23:35:50 | getHeader(...) : String | SpringUrlRedirect.java:36:30:36:33 | next | provenance | Src:MaD:2 | -| SpringUrlRedirect.java:41:23:41:50 | getHeader(...) : String | SpringUrlRedirect.java:42:47:42:50 | next | provenance | Src:MaD:2 | -| SpringUrlRedirect.java:47:23:47:50 | getHeader(...) : String | SpringUrlRedirect.java:48:33:48:36 | next | provenance | Src:MaD:2 | -| SpringUrlRedirect.java:53:23:53:50 | getHeader(...) : String | SpringUrlRedirect.java:54:25:54:28 | next : String | provenance | Src:MaD:2 | -| SpringUrlRedirect.java:54:25:54:28 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance | | -| SpringUrlRedirect.java:59:23:59:50 | getParameter(...) : String | SpringUrlRedirect.java:61:21:61:24 | next | provenance | Src:MaD:3 | -| SpringUrlRedirect.java:67:23:67:50 | getParameter(...) : String | SpringUrlRedirect.java:68:33:68:36 | next | provenance | Src:MaD:3 | -| SpringUrlRedirect.java:73:23:73:50 | getParameter(...) : String | SpringUrlRedirect.java:74:39:74:42 | next | provenance | Src:MaD:3 | +| 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 | | @@ -35,21 +34,20 @@ models | 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:30:13:33 | path | semmle.label | path | -| SpringUrlRedirect.java:35:23:35:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | -| SpringUrlRedirect.java:36:30:36:33 | next | semmle.label | next | -| SpringUrlRedirect.java:41:23:41:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | -| SpringUrlRedirect.java:42:47:42:50 | next | semmle.label | next | -| SpringUrlRedirect.java:47:23:47:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | -| SpringUrlRedirect.java:48:33:48:36 | next | semmle.label | next | -| SpringUrlRedirect.java:53:23:53:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | -| SpringUrlRedirect.java:54:25:54:28 | next : String | semmle.label | next : String | -| SpringUrlRedirect.java:59:23:59:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| SpringUrlRedirect.java:61:21:61:24 | next | semmle.label | next | -| SpringUrlRedirect.java:67:23:67:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| SpringUrlRedirect.java:68:33:68:36 | next | semmle.label | next | -| SpringUrlRedirect.java:73:23:73:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| SpringUrlRedirect.java:74:39:74:42 | next | semmle.label | next | +| 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(...) | @@ -63,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(...) | From abe0c7879174104f24ec8ac2d358bf58cb088bed Mon Sep 17 00:00:00 2001 From: Keshav Malik Date: Wed, 14 Oct 2026 20:24:07 +0530 Subject: [PATCH 3/3] Fix Spring redirect view result modeling --- .../2026-08-17-spring-url-redirect-sinks.md | 6 +- .../semmle/code/java/security/UrlRedirect.qll | 218 +++++++-- .../semmle/tests/SpringUrlRedirect.java | 456 ++++++++++++++++++ .../CWE-601/semmle/tests/UrlRedirect.expected | 201 +++++++- 4 files changed, 819 insertions(+), 62 deletions(-) diff --git a/java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md b/java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md index 7e9200ff16b7..394bf1ac4b32 100644 --- a/java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md +++ b/java/ql/lib/change-notes/2026-08-17-spring-url-redirect-sinks.md @@ -1,6 +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. +* The `java/unvalidated-url-redirection` query now detects untrusted URLs used in returned Spring MVC + `RedirectView` objects and leading `redirect:` view names, including helper-generated names passed + to returned `ModelAndView` objects. View objects returned by response-body handlers are also covered. diff --git a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll index 13d1911b66b0..fabd0b847d13 100644 --- a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll +++ b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll @@ -9,7 +9,9 @@ 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.SSA private import semmle.code.java.dataflow.TaintTracking +private import semmle.code.java.controlflow.Dominance private import semmle.code.java.frameworks.JaxWS private import semmle.code.java.frameworks.spring.SpringController private import semmle.code.java.security.RequestForgery @@ -56,11 +58,11 @@ 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()) + exists(Expr name, SpringRedirectPrefix prefix | + (isSpringMvcReturnedString(name) or isSpringModelAndViewName(name)) and + prefix = getViewNamePrefix(name) and + appendedToRedirectPrefix(prefix, this) and + TaintTracking::localTaint(this, DataFlow::exprNode(name)) ) } } @@ -68,17 +70,18 @@ private class SpringUrlRedirectPrefixSink extends UrlRedirectSink { /** A call to a helper that returns a Spring `"redirect:"` view name. */ private class SpringUrlRedirectHelperSink extends UrlRedirectSink { SpringUrlRedirectHelperSink() { - exists(MethodCall call | + exists(Expr name, MethodCall call | this.asExpr() = call and - isSpringMvcViewResult(call) and + (isSpringMvcReturnedString(name) or isSpringModelAndViewName(name)) and + call = getViewNamePrefix(name) and returnsRedirectViewName(call.getCallee().getSourceDeclaration()) ) } } pragma[nomagic] -private predicate appendedToRedirectPrefix(DataFlow::ExprNode exprNode) { - exists(SpringRedirectPrefix prefix | exprNode.asExpr() = prefix.getAnAppendedExpression()) +private predicate appendedToRedirectPrefix(SpringRedirectPrefix prefix, DataFlow::ExprNode exprNode) { + exprNode.asExpr() = prefix.getAnAppendedExpression() } private class SpringRedirectPrefix extends InterestingPrefix { @@ -87,61 +90,194 @@ private class SpringRedirectPrefix extends InterestingPrefix { 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())) +/** Relates a local read to its reaching SSA definitions. */ +private predicate localViewDefinition(Expr value, SsaDefinition def) { + value.getUnderlyingExpr().(VarRead).getVariable() instanceof LocalVariableDecl and + def.getARead() = value.getUnderlyingExpr() +} + +/** Gets a terminal local value, retaining unknown or merged definitions as unknown reads. */ +pragma[nomagic] +private Expr getALocalViewValue(Expr value) { + result = value.getUnderlyingExpr() and + ( + not exists(SsaDefinition def | localViewDefinition(value, def)) + or + exists(SsaDefinition def | + localViewDefinition(value, def) and + not exists(def.(SsaExplicitWrite).getValue()) + ) + ) + or + exists(SsaExplicitWrite def | + localViewDefinition(value, def) and + result = getALocalViewValue(def.getValue()) ) } -/** 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())) +/** Gets the unique local value, excluding uncertain origins. */ +private Expr getLocalViewValue(Expr value) { + result = unique(Expr terminal | terminal = getALocalViewValue(value)) +} + +/** Gets the leading expression of a complete view name, following only left-hand concatenation. */ +pragma[nomagic] +private Expr getViewNamePrefix(Expr value) { + result = getLocalViewValue(value) and not result instanceof AddExpr + or + result = getViewNamePrefix(getLocalViewValue(value).(AddExpr).getLeftOperand()) +} + +/** Holds if a call writes the URL or view-name property of a Spring view. */ +private predicate isSpringViewPropertyWrite(MethodCall call, string property) { + property = "url" and + call.getMethod() + .getSourceDeclaration() + .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView", "setUrl") + or + property = "view" and + call.getMethod() + .getSourceDeclaration() + .hasQualifiedName("org.springframework.web.servlet", "ModelAndView", + ["setViewName", "setView"]) +} + +/** Gets the definite local allocation behind `value`, excluding ambiguous or merged values. */ +private ClassInstanceExpr getViewAllocation(Expr value) { result = getLocalViewValue(value) } + +/** Holds if an allocation can be changed outside the local, recognized property writes. */ +private predicate viewEscapes(ClassInstanceExpr allocation, string property) { + property = ["url", "view"] and + ( + exists(Call call | + DataFlow::localFlow(DataFlow::exprNode(allocation), DataFlow::exprNode(call.getAnArgument())) + ) + or + exists(MethodCall call | + DataFlow::localFlow(DataFlow::exprNode(allocation), DataFlow::exprNode(call.getQualifier())) and + not isSpringViewPropertyWrite(call, property) + ) + or + exists(AssignExpr assign | + (assign.getDest() instanceof FieldAccess or assign.getDest() instanceof ArrayAccess) and + DataFlow::localFlow(DataFlow::exprNode(allocation), DataFlow::exprNode(assign.getSource())) + ) + or + exists(ArrayInit init | + DataFlow::localFlow(DataFlow::exprNode(allocation), DataFlow::exprNode(init.getAnInit())) + ) + or + exists(MemberRefExpr reference | + DataFlow::localFlow(DataFlow::exprNode(allocation), + DataFlow::exprNode(reference.getReceiverExpr())) + ) + or + exists(LocalScopeVariable variable, VarAccess access | + access.getVariable() = variable and + DataFlow::localFlow(DataFlow::exprNode(allocation), DataFlow::exprNode(access)) and + variable.getAnAccess().getEnclosingCallable() != variable.getCallable() + ) + ) +} + +/** Holds if a constant property replacement completes before this particular returned view. */ +private predicate viewPropertyReplacedBeforeReturn( + Call write, Expr value, ReturnStmt ret, string property +) { + exists(ClassInstanceExpr allocation, MethodCall replacement | + allocation = getViewAllocation(value) and + allocation = getViewAllocation(ret.getExpr()) and + not viewEscapes(allocation, property) and + ( + property = "url" and + allocation + .getConstructedType() + .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") + or + property = "view" and + allocation + .getConstructedType() + .hasQualifiedName("org.springframework.web.servlet", "ModelAndView") + ) and + isSpringViewPropertyWrite(replacement, property) and + exists(replacement.getArgument(0).(CompileTimeConstantExpr).getStringValue()) and + getViewAllocation(replacement.getQualifier()) = allocation and + not allocation.getParent*() instanceof LoopStmt and + not replacement.getParent*() instanceof LoopStmt and + not ret.getParent*() instanceof LoopStmt and + strictlyDominates(write.getControlFlowNode(), replacement.getControlFlowNode()) and + dominates(replacement.getControlFlowNode(), ret.getControlFlowNode()) and + dominates(replacement.getControlFlowNode().getANormalSuccessor(), ret.getControlFlowNode()) and + // A later or unordered write may replace the constant again. Possible aliases suffice here. + not exists(MethodCall other | + other != replacement and + isSpringViewPropertyWrite(other, property) and + DataFlow::localFlow(DataFlow::exprNode(allocation), DataFlow::exprNode(other.getQualifier())) and + not strictlyDominates(other.getControlFlowNode(), replacement.getControlFlowNode()) + ) ) } /** 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() +private predicate isSpringMvcViewResult(Expr value, Call write, string property) { + property = ["url", "view"] and + exists(ReturnStmt ret | + ret.getEnclosingCallable() = value.getEnclosingCallable() and + ret.getEnclosingCallable() instanceof SpringRequestMappingMethod and + DataFlow::localFlow(DataFlow::exprNode(value), DataFlow::exprNode(ret.getExpr())) and + not viewPropertyReplacedBeforeReturn(write, value, ret, property) + ) } -/** 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 `name` is a string view name returned by a request handler. */ +private predicate isSpringMvcReturnedString(Expr name) { + exists(ReturnStmt ret, SpringRequestMappingMethod handler | + handler = name.getEnclosingCallable() and + ret.getEnclosingCallable() = handler and + not handler.isResponseBody() and + not handler instanceof SpringModelAttributeMethod and + name = ret.getExpr() + ) } -/** Holds if `value` contributes to the view name of a returned `ModelAndView`. */ -private predicate isSpringModelAndViewName(Expr value) { +/** Holds if `name` is the string view name of a returned `ModelAndView`. */ +private predicate isSpringModelAndViewName(Expr name) { 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) + newModelAndView.getConstructor().getParameter(0).getType() instanceof TypeString and + name = newModelAndView.getArgument(0) and + isSpringMvcViewResult(newModelAndView, newModelAndView, "view") ) } -/** Holds if `callable` returns a view name constructed with the Spring `"redirect:"` prefix. */ +/** Holds if a name is constant or is constructed from the exact redirect prefix. */ +private predicate isConstantOrRedirectViewName(Expr name) { + exists(getLocalViewValue(name).(CompileTimeConstantExpr).getStringValue()) + or + getViewNamePrefix(name) instanceof SpringRedirectPrefix +} + +/** Holds if `callable` returns redirect names without mixing in other dynamic view names. */ pragma[nomagic] private predicate returnsRedirectViewName(Callable callable) { - exists(DataFlow::ExprNode appended, ReturnStmt ret | - appendedToRedirectPrefix(appended) and - appended.asExpr().getEnclosingCallable() = callable and + exists(SpringRedirectPrefix prefix, DataFlow::ExprNode appended, ReturnStmt ret | + callable.(Method).getReturnType() instanceof TypeString and + forall(ReturnStmt r | r.getEnclosingCallable() = callable | + isConstantOrRedirectViewName(r.getExpr()) + ) and ret.getEnclosingCallable() = callable and + prefix = getViewNamePrefix(ret.getExpr()) and + appendedToRedirectPrefix(prefix, appended) and TaintTracking::localTaint(appended, DataFlow::exprNode(ret.getExpr())) ) or - exists(MethodCall call | - call.getEnclosingCallable() = callable and - contributesToReturn(call) and + exists(MethodCall call, ReturnStmt ret | + callable.(Method).getReturnType() instanceof TypeString and + ret = unique(ReturnStmt r | r.getEnclosingCallable() = callable) and + call = getViewNamePrefix(ret.getExpr()) and returnsRedirectViewName(call.getCallee().getSourceDeclaration()) ) } @@ -153,7 +289,7 @@ private class SpringRedirectViewSink extends UrlRedirectSink { newRedirectView .getConstructedType() .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and - isSpringMvcViewResult(newRedirectView) and + isSpringMvcViewResult(newRedirectView, newRedirectView, "url") and this.asExpr() = newRedirectView.getArgument(0) ) } @@ -173,7 +309,7 @@ private class SpringRedirectViewSetUrlSink extends UrlRedirectSink { .(RefType) .getASupertype*() .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and - isSpringMvcViewResult(setUrl.getQualifier()) and + isSpringMvcViewResult(setUrl.getQualifier(), setUrl, "url") and this.asExpr() = setUrl.getArgument(0) ) } diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java b/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java index d2dbb7933c78..02eb79bd0b81 100644 --- a/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/SpringUrlRedirect.java @@ -159,3 +159,459 @@ public String body(HttpServletRequest request) { return redirectShared(request.getParameter("value")); } } + +@Controller +class SpringRedirectRegressions extends BaseController { + + protected String embeddedRedirect(String path) { + String base = "home"; + return base + "redirect:" + path; + } + + protected String fixedRelativeRedirect(String tab) { + return "redirect:/account?tab=" + tab; + } + + protected String mixedRedirect(String path, boolean useConstant) { + if (useConstant) { + return "redirect:" + "/account"; + } + return "home:" + path; + } + + @GetMapping("/helper-model-and-view") + public ModelAndView helperModelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(redirect(next)); // $ Alert + } + + @GetMapping("/helper-model-and-view-model") + public ModelAndView helperModelAndViewWithModel(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(redirect(next), "note", "value"); // $ Alert + } + + @GetMapping("/redirect-with-suffix") + public String redirectWithSuffix(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return "redirect:" + next + "?tab=profile"; // $ Alert + } + + @GetMapping("/local-model-and-view-name") + public ModelAndView localModelAndViewName(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + String viewName = "redirect:" + next; // $ Alert + return new ModelAndView(viewName); + } + + @GetMapping("/safe-leading-view-name") + public String leadingViewName(HttpServletRequest request) { + String next = request.getParameter("next"); + String base = "home"; + return base + "redirect:" + next; + } + + @GetMapping("/safe-leading-model-and-view") + public ModelAndView leadingModelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); + String base = "home"; + return new ModelAndView(base + "redirect:" + next); + } + + @GetMapping("/safe-leading-helper-model-and-view") + public ModelAndView leadingHelperModelAndView(HttpServletRequest request) { + return new ModelAndView("home" + redirect(request.getParameter("next"))); + } + + @GetMapping("/safe-embedded-helper-prefix") + public String embeddedHelperPrefix(HttpServletRequest request) { + return embeddedRedirect(request.getParameter("next")); + } + + @GetMapping("/safe-helper-model-value") + public ModelAndView helperInModel(HttpServletRequest request) { + return new ModelAndView("home", "value", redirect(request.getParameter("next"))); + } + + @GetMapping("/safe-discarded-model-and-view") + public ModelAndView discardedModelAndView(HttpServletRequest request) { + new ModelAndView(redirect(request.getParameter("next"))); + return new ModelAndView("home"); + } + + @GetMapping("/safe-logged-helper") + public String loggedHelper(HttpServletRequest request) { + System.out.println(redirect(request.getParameter("next"))); + return "home"; + } + + @GetMapping("/safe-leading-builder") + public String leadingBuilder(HttpServletRequest request) { + StringBuilder builder = new StringBuilder("home"); + builder.append("redirect:"); + builder.append(request.getParameter("next")); + return builder.toString(); + } + + @GetMapping("/safe-leading-format") + public String leadingFormat(HttpServletRequest request) { + return "home" + String.format("redirect:%s", request.getParameter("next")); + } + + @GetMapping("/safe-fixed-relative-helper") + public ModelAndView fixedRelativeHelper(HttpServletRequest request) { + return new ModelAndView(fixedRelativeRedirect(request.getParameter("tab"))); + } + + @GetMapping("/safe-mixed-helper") + public ModelAndView mixedHelper(HttpServletRequest request, boolean useConstant) { + return new ModelAndView(mixedRedirect(request.getParameter("next"), useConstant)); + } + + @GetMapping("/safe-validated-helper") + public ModelAndView validatedHelper(HttpServletRequest request) throws Exception { + java.net.URI uri = new java.net.URI(request.getParameter("next")); + if ("example.com".equals(uri.getHost())) { + return new ModelAndView(redirect(uri.toString())); + } + return new ModelAndView("home"); + } + + @GetMapping("/safe-overwritten-constructor-url") + public RedirectView overwrittenConstructorUrl(HttpServletRequest request) { + RedirectView view = new RedirectView(request.getParameter("next")); + view.setUrl("/safe"); + return view; + } + + @GetMapping("/safe-overwritten-setter-url") + public RedirectView overwrittenSetterUrl(HttpServletRequest request) { + RedirectView view = new RedirectView(); + view.setUrl(request.getParameter("next")); + view.setUrl("/safe"); + return view; + } + + @GetMapping("/safe-overwritten-model-and-view-name") + public ModelAndView overwrittenModelAndViewName(HttpServletRequest request) { + ModelAndView view = new ModelAndView("redirect:" + request.getParameter("next")); + view.setViewName("home"); + return view; + } +} + +@Controller +class ResponseBodyRedirectRegressions extends BaseController { + + @ResponseBody + @GetMapping("/body-redirect-view") + public RedirectView redirectView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new RedirectView(next); // $ Alert + } + + @ResponseBody + @GetMapping("/body-model-and-view") + public ModelAndView modelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView("redirect:" + next); // $ Alert + } + + @ResponseBody + @GetMapping("/body-helper-model-and-view") + public ModelAndView helperModelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(redirect(next)); // $ Alert + } +} + +@RestController +class JsonRedirectRegressions extends BaseController { + + @GetMapping("/json-redirect-view") + public RedirectView redirectView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new RedirectView(next); // $ Alert + } + + @GetMapping("/json-model-and-view") + public ModelAndView modelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView("redirect:" + next); // $ Alert + } + + @GetMapping("/json-helper-model-and-view") + public ModelAndView helperModelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(redirect(next)); // $ Alert + } + + @GetMapping("/json-configured-redirect-view") + public RedirectView configuredRedirectView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(); + view.setUrl(next); // $ Alert + return view; + } + + @GetMapping("/json-object-redirect-view") + public Object objectRedirectView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new RedirectView(next); // $ Alert + } +} + +@Controller +class SpringViewPropertyRegressions extends SpringRedirectRegressions { + @GetMapping("/conditional-url-overwrite") + public RedirectView conditionalUrlOverwrite(HttpServletRequest request, boolean replace) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + if (replace) { + view.setUrl("/safe"); + } + return view; + } + + @GetMapping("/other-view-url-overwrite") + public RedirectView otherViewUrlOverwrite(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + RedirectView other = new RedirectView("/account"); + other.setUrl("/safe"); + return view; + } + + @GetMapping("/safe-alias-url-overwrite") + public RedirectView aliasUrlOverwrite(HttpServletRequest request) { + RedirectView view = new RedirectView(request.getParameter("next")); + RedirectView alias = view; + alias.setUrl("/safe"); + return view; + } + + @GetMapping("/conditional-alias-url-overwrite") + public RedirectView conditionalAliasUrlOverwrite(HttpServletRequest request, boolean replace) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + RedirectView alias = replace ? view : new RedirectView("/account"); + alias.setUrl("/safe"); + return view; + } + + @GetMapping("/returned-before-url-overwrite") + public RedirectView returnedBeforeUrlOverwrite(HttpServletRequest request, boolean keepOriginal) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + if (keepOriginal) { + return view; + } + view.setUrl("/safe"); + return view; + } + + @GetMapping("/later-unsafe-url-overwrite") + public RedirectView laterUnsafeUrlOverwrite(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + view.setUrl("/safe"); + view.setUrl(next); // $ Alert + return view; + } + + @GetMapping("/safe-mixed-helper-string") + public String mixedHelperString(HttpServletRequest request, boolean useConstant) { + return mixedRedirect(request.getParameter("next"), useConstant); + } + + @ResponseBody + @GetMapping("/safe-body-shared-view-helper") + public String responseBodySharedViewHelper(HttpServletRequest request) { + return redirect(request.getParameter("next")); + } +} + +@Controller +class SpringModelAndViewPropertyRegressions extends BaseController { + @GetMapping("/later-unsafe-model-and-view-overwrite") + public ModelAndView laterUnsafeModelAndViewOverwrite(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + ModelAndView view = new ModelAndView("redirect:" + next); // $ Alert + view.setViewName("home"); + view.setViewName("redirect:" + next); + return view; + } +} + +@Controller +class SpringAdditionalPrefixRegressions extends BaseController { + protected String conditionalMixedRedirect(String path, boolean useConstant) { + return useConstant ? redirect("/safe") : "home:" + path; + } + + protected String branchingRedirect(String path, boolean useConstant) { + if (useConstant) { + return "redirect:" + "/account"; + } + return "redirect:" + path; + } + + private static void restoreUrl(RedirectView view, String url) { + view.setUrl(url); + } + + @GetMapping("/safe-conditional-mixed-helper") + public ModelAndView conditionalMixedHelper(HttpServletRequest request, boolean useConstant) { + return new ModelAndView(conditionalMixedRedirect(request.getParameter("next"), useConstant)); + } + + @GetMapping("/helper-restores-unsafe-url") + public RedirectView helperRestoresUnsafeUrl(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + view.setUrl("/safe"); + restoreUrl(view, next); + return view; + } + + @GetMapping("/safe-outer-relative-prefix") + public String outerRelativePrefix(HttpServletRequest request) { + String next = request.getParameter("next"); + return "redirect:/safe/" + ("redirect:" + next); + } + + @GetMapping("/safe-outer-relative-model-and-view") + public ModelAndView outerRelativeModelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); + return new ModelAndView("redirect:/safe/" + ("redirect:" + next)); + } + + @GetMapping("/branching-view-name") + public String branchingViewName(HttpServletRequest request, boolean useConstant) { + String next = request.getParameter("next"); // $ Source + return branchingRedirect(next, useConstant); // $ Alert + } + + @GetMapping("/branching-model-and-view-name") + public ModelAndView branchingModelAndViewName(HttpServletRequest request, boolean useConstant) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(branchingRedirect(next, useConstant)); // $ Alert + } +} + +@Controller +class SpringRedirectHelperRegressions extends BaseController { + protected String internallyValidatedRedirect(String path) { + if (!java.util.Arrays.asList("/account", "/home").contains(path)) { + return "redirect:/safe"; + } + return "redirect:" + path; + } + + protected String internallySourcedRedirect(HttpServletRequest request) { + String next = request.getHeader("referer"); // $ Source + return "redirect:" + next; + } + + protected String secondRedirect(String... paths) { + return "redirect:" + paths[1]; + } + + protected String fixedBranchingRedirect(String tab, boolean useConstant) { + if (useConstant) { + return "redirect:/account"; + } + return "redirect:/account?tab=" + tab; + } + + private static void restoreArrayUrl(RedirectView[] views, String url) { + views[0].setUrl(url); + } + + @GetMapping("/safe-internally-validated-helper") + public ModelAndView internallyValidatedHelper(HttpServletRequest request) { + return new ModelAndView(internallyValidatedRedirect(request.getParameter("next"))); + } + + @GetMapping("/internally-sourced-helper") + public ModelAndView internallySourcedHelper(HttpServletRequest request) { + return new ModelAndView(internallySourcedRedirect(request)); // $ Alert + } + + @GetMapping("/varargs-model-and-view-name") + public ModelAndView varargsModelAndViewName(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(secondRedirect("/safe", next)); // $ Alert + } + + @GetMapping("/safe-fixed-branching-helper") + public ModelAndView fixedBranchingHelper(HttpServletRequest request, boolean useConstant) { + return new ModelAndView(fixedBranchingRedirect(request.getParameter("tab"), useConstant)); + } + + @GetMapping("/ambiguous-finally-url-overwrite") + public RedirectView ambiguousFinallyUrlOverwrite( + HttpServletRequest request, boolean flag, RedirectView supplied) { + String next = request.getParameter("next"); // $ Source + RedirectView result = new RedirectView(next); // $ Alert + RedirectView view = result; + try { + if (flag) { + return result; + } + view = supplied; + } finally { + view.setUrl("/safe"); + } + return result; + } + + @GetMapping("/array-helper-restores-unsafe-url") + public RedirectView arrayHelperRestoresUnsafeUrl(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + RedirectView view = new RedirectView(next); // $ Alert + RedirectView[] views = { view }; + view.setUrl("/safe"); + restoreArrayUrl(views, next); + return view; + } +} + +@Controller +class SpringRedirectFallbackRegressions extends BaseController { + protected String branchingFallbackRedirect(String path, boolean useRedirect) { + if (useRedirect) { + return "redirect:" + path; + } + return "home"; + } + + @GetMapping("/branching-helper-with-ordinary-fallback") + public ModelAndView branchingFallbackModelAndView(HttpServletRequest request, boolean useRedirect) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView(branchingFallbackRedirect(next, useRedirect)); // $ Alert + } +} + +@Controller +class SpringModelAttributeRedirectRegressions extends BaseController { + @org.springframework.web.bind.annotation.ModelAttribute("value") + @GetMapping("/safe-model-attribute-value") + public String modelAttributeValue(HttpServletRequest request) { + return "redirect:" + request.getParameter("next"); + } + + @org.springframework.web.bind.annotation.ModelAttribute("value") + @GetMapping("/model-attribute-redirect-view") + public RedirectView modelAttributeRedirectView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new RedirectView(next); // $ Alert + } + + @org.springframework.web.bind.annotation.ModelAttribute("value") + @GetMapping("/model-attribute-model-and-view") + public ModelAndView modelAttributeModelAndView(HttpServletRequest request) { + String next = request.getParameter("next"); // $ Source + return new ModelAndView("redirect:" + next); // $ Alert + } +} diff --git a/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected b/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected index 8d80afe47286..cc0b485afa34 100644 --- a/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected +++ b/java/ql/test/query-tests/security/CWE-601/semmle/tests/UrlRedirect.expected @@ -9,6 +9,35 @@ | 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 | +| SpringUrlRedirect.java:185:33:185:46 | redirect(...) | SpringUrlRedirect.java:184:23:184:50 | getParameter(...) : String | SpringUrlRedirect.java:185:33:185:46 | redirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:184:23:184:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:191:33:191:46 | redirect(...) | SpringUrlRedirect.java:190:23:190:50 | getParameter(...) : String | SpringUrlRedirect.java:191:33:191:46 | redirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:190:23:190:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:197:30:197:33 | next | SpringUrlRedirect.java:196:23:196:50 | getParameter(...) : String | SpringUrlRedirect.java:197:30:197:33 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:196:23:196:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:203:41:203:44 | next | SpringUrlRedirect.java:202:23:202:50 | getParameter(...) : String | SpringUrlRedirect.java:203:41:203:44 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:202:23:202:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:310:33:310:36 | next | SpringUrlRedirect.java:309:23:309:50 | getParameter(...) : String | SpringUrlRedirect.java:310:33:310:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:309:23:309:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:317:47:317:50 | next | SpringUrlRedirect.java:316:23:316:50 | getParameter(...) : String | SpringUrlRedirect.java:317:47:317:50 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:316:23:316:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:324:33:324:46 | redirect(...) | SpringUrlRedirect.java:323:23:323:50 | getParameter(...) : String | SpringUrlRedirect.java:324:33:324:46 | redirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:323:23:323:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:334:33:334:36 | next | SpringUrlRedirect.java:333:23:333:50 | getParameter(...) : String | SpringUrlRedirect.java:334:33:334:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:333:23:333:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:340:47:340:50 | next | SpringUrlRedirect.java:339:23:339:50 | getParameter(...) : String | SpringUrlRedirect.java:340:47:340:50 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:339:23:339:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:346:33:346:46 | redirect(...) | SpringUrlRedirect.java:345:23:345:50 | getParameter(...) : String | SpringUrlRedirect.java:346:33:346:46 | redirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:345:23:345:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:353:21:353:24 | next | SpringUrlRedirect.java:351:23:351:50 | getParameter(...) : String | SpringUrlRedirect.java:353:21:353:24 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:351:23:351:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:360:33:360:36 | next | SpringUrlRedirect.java:359:23:359:50 | getParameter(...) : String | SpringUrlRedirect.java:360:33:360:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:359:23:359:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:369:46:369:49 | next | SpringUrlRedirect.java:368:23:368:50 | getParameter(...) : String | SpringUrlRedirect.java:369:46:369:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:368:23:368:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:379:46:379:49 | next | SpringUrlRedirect.java:378:23:378:50 | getParameter(...) : String | SpringUrlRedirect.java:379:46:379:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:378:23:378:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:396:46:396:49 | next | SpringUrlRedirect.java:395:23:395:50 | getParameter(...) : String | SpringUrlRedirect.java:396:46:396:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:395:23:395:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:405:46:405:49 | next | SpringUrlRedirect.java:404:23:404:50 | getParameter(...) : String | SpringUrlRedirect.java:405:46:405:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:404:23:404:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:416:46:416:49 | next | SpringUrlRedirect.java:415:23:415:50 | getParameter(...) : String | SpringUrlRedirect.java:416:46:416:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:415:23:415:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:418:21:418:24 | next | SpringUrlRedirect.java:415:23:415:50 | getParameter(...) : String | SpringUrlRedirect.java:418:21:418:24 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:415:23:415:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:439:60:439:63 | next | SpringUrlRedirect.java:438:23:438:50 | getParameter(...) : String | SpringUrlRedirect.java:439:60:439:63 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:438:23:438:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:471:46:471:49 | next | SpringUrlRedirect.java:470:23:470:50 | getParameter(...) : String | SpringUrlRedirect.java:471:46:471:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:470:23:470:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:492:16:492:51 | branchingRedirect(...) | SpringUrlRedirect.java:491:23:491:50 | getParameter(...) : String | SpringUrlRedirect.java:492:16:492:51 | branchingRedirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:491:23:491:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:498:33:498:68 | branchingRedirect(...) | SpringUrlRedirect.java:497:23:497:50 | getParameter(...) : String | SpringUrlRedirect.java:498:33:498:68 | branchingRedirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:497:23:497:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:538:33:538:66 | internallySourcedRedirect(...) | SpringUrlRedirect.java:512:23:512:50 | getHeader(...) : String | SpringUrlRedirect.java:538:33:538:66 | internallySourcedRedirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:512:23:512:50 | getHeader(...) | user-provided value | +| SpringUrlRedirect.java:544:33:544:61 | secondRedirect(...) | SpringUrlRedirect.java:543:23:543:50 | getParameter(...) : String | SpringUrlRedirect.java:544:33:544:61 | secondRedirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:543:23:543:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:556:48:556:51 | next | SpringUrlRedirect.java:555:23:555:50 | getParameter(...) : String | SpringUrlRedirect.java:556:48:556:51 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:555:23:555:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:572:46:572:49 | next | SpringUrlRedirect.java:571:23:571:50 | getParameter(...) : String | SpringUrlRedirect.java:572:46:572:49 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:571:23:571:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:592:33:592:76 | branchingFallbackRedirect(...) | SpringUrlRedirect.java:591:23:591:50 | getParameter(...) : String | SpringUrlRedirect.java:592:33:592:76 | branchingFallbackRedirect(...) | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:591:23:591:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:608:33:608:36 | next | SpringUrlRedirect.java:607:23:607:50 | getParameter(...) : String | SpringUrlRedirect.java:608:33:608:36 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:607:23:607:50 | getParameter(...) | user-provided value | +| SpringUrlRedirect.java:615:47:615:50 | next | SpringUrlRedirect.java:614:23:614:50 | getParameter(...) : String | SpringUrlRedirect.java:615:47:615:50 | next | Untrusted URL redirection depends on a $@. | SpringUrlRedirect.java:614:23:614: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 | @@ -17,27 +46,79 @@ | 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 | MicronautUrlRedirect.java:12:39:12:63 | target : String | MicronautUrlRedirect.java:14:49:14:54 | target : String | provenance | | -| MicronautUrlRedirect.java:14:49:14:54 | target : String | MicronautUrlRedirect.java:14:38:14:55 | create(...) | provenance | MaD:9 Sink:MaD:2 | +| MicronautUrlRedirect.java:14:49:14:54 | target : String | MicronautUrlRedirect.java:14:38:14:55 | create(...) | provenance | MaD:10 Sink:MaD:2 | | MicronautUrlRedirect.java:27:45:27:69 | target : String | MicronautUrlRedirect.java:28:58:28:63 | target : String | provenance | | -| MicronautUrlRedirect.java:28:58:28:63 | target : String | MicronautUrlRedirect.java:28:47:28:64 | create(...) | provenance | MaD:9 Sink:MaD:1 | +| MicronautUrlRedirect.java:28:58:28:63 | target : String | MicronautUrlRedirect.java:28:47:28:64 | create(...) | provenance | MaD:10 Sink:MaD:1 | | MicronautUrlRedirect.java:32:44:32:68 | target : String | MicronautUrlRedirect.java:33:49:33:54 | target : String | provenance | | -| MicronautUrlRedirect.java:33:49:33:54 | target : String | MicronautUrlRedirect.java:33:38:33:55 | create(...) | provenance | MaD:9 Sink:MaD:3 | +| MicronautUrlRedirect.java:33:49:33:54 | target : String | MicronautUrlRedirect.java:33:38:33:55 | create(...) | provenance | MaD:10 Sink:MaD:3 | | MicronautUrlRedirect.java:37:45:37:69 | target : String | MicronautUrlRedirect.java:38:58:38:63 | target : String | provenance | | -| MicronautUrlRedirect.java:38:58:38:63 | target : String | MicronautUrlRedirect.java:38:47:38:64 | create(...) | provenance | MaD:9 Sink:MaD:4 | +| MicronautUrlRedirect.java:38:58:38:63 | target : String | MicronautUrlRedirect.java:38:47:38:64 | create(...) | provenance | MaD:10 Sink:MaD:4 | | 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:6 | -| SpringUrlRedirect.java:44:23:44:50 | getHeader(...) : String | SpringUrlRedirect.java:45:47:45:50 | next | provenance | Src:MaD:6 | -| SpringUrlRedirect.java:50:23:50:50 | getHeader(...) : String | SpringUrlRedirect.java:51:33:51:36 | next | provenance | Src:MaD:6 | -| SpringUrlRedirect.java:56:23:56:50 | getHeader(...) : String | SpringUrlRedirect.java:57:25:57:28 | next : String | provenance | Src:MaD:6 | +| SpringUrlRedirect.java:38:23:38:50 | getHeader(...) : String | SpringUrlRedirect.java:39:30:39:33 | next | provenance | Src:MaD:7 | +| SpringUrlRedirect.java:44:23:44:50 | getHeader(...) : String | SpringUrlRedirect.java:45:47:45:50 | next | provenance | Src:MaD:7 | +| SpringUrlRedirect.java:50:23:50:50 | getHeader(...) : String | SpringUrlRedirect.java:51:33:51:36 | next | provenance | Src:MaD:7 | +| SpringUrlRedirect.java:56:23:56:50 | getHeader(...) : String | SpringUrlRedirect.java:57:25:57:28 | next : String | provenance | Src:MaD:7 | | 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:7 | -| SpringUrlRedirect.java:70:23:70:50 | getParameter(...) : String | SpringUrlRedirect.java:71:33:71:36 | next | provenance | Src:MaD:7 | -| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | provenance | Src:MaD:7 MaD:8 | -| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:45:28:45:39 | input : String | provenance | Src:MaD:7 | +| SpringUrlRedirect.java:62:23:62:50 | getParameter(...) : String | SpringUrlRedirect.java:64:21:64:24 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:70:23:70:50 | getParameter(...) : String | SpringUrlRedirect.java:71:33:71:36 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:184:23:184:50 | getParameter(...) : String | SpringUrlRedirect.java:185:42:185:45 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:185:42:185:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance | | +| SpringUrlRedirect.java:185:42:185:45 | next : String | SpringUrlRedirect.java:185:33:185:46 | redirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:190:23:190:50 | getParameter(...) : String | SpringUrlRedirect.java:191:42:191:45 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:191:42:191:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance | | +| SpringUrlRedirect.java:191:42:191:45 | next : String | SpringUrlRedirect.java:191:33:191:46 | redirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:196:23:196:50 | getParameter(...) : String | SpringUrlRedirect.java:197:30:197:33 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:202:23:202:50 | getParameter(...) : String | SpringUrlRedirect.java:203:41:203:44 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:309:23:309:50 | getParameter(...) : String | SpringUrlRedirect.java:310:33:310:36 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:316:23:316:50 | getParameter(...) : String | SpringUrlRedirect.java:317:47:317:50 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:323:23:323:50 | getParameter(...) : String | SpringUrlRedirect.java:324:42:324:45 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:324:42:324:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance | | +| SpringUrlRedirect.java:324:42:324:45 | next : String | SpringUrlRedirect.java:324:33:324:46 | redirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:333:23:333:50 | getParameter(...) : String | SpringUrlRedirect.java:334:33:334:36 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:339:23:339:50 | getParameter(...) : String | SpringUrlRedirect.java:340:47:340:50 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:345:23:345:50 | getParameter(...) : String | SpringUrlRedirect.java:346:42:346:45 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:346:42:346:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | provenance | | +| SpringUrlRedirect.java:346:42:346:45 | next : String | SpringUrlRedirect.java:346:33:346:46 | redirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:351:23:351:50 | getParameter(...) : String | SpringUrlRedirect.java:353:21:353:24 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:359:23:359:50 | getParameter(...) : String | SpringUrlRedirect.java:360:33:360:36 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:368:23:368:50 | getParameter(...) : String | SpringUrlRedirect.java:369:46:369:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:378:23:378:50 | getParameter(...) : String | SpringUrlRedirect.java:379:46:379:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:395:23:395:50 | getParameter(...) : String | SpringUrlRedirect.java:396:46:396:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:404:23:404:50 | getParameter(...) : String | SpringUrlRedirect.java:405:46:405:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:415:23:415:50 | getParameter(...) : String | SpringUrlRedirect.java:416:46:416:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:415:23:415:50 | getParameter(...) : String | SpringUrlRedirect.java:418:21:418:24 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:438:23:438:50 | getParameter(...) : String | SpringUrlRedirect.java:439:60:439:63 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:452:40:452:50 | path : String | SpringUrlRedirect.java:456:16:456:33 | ... + ... : String | provenance | | +| SpringUrlRedirect.java:470:23:470:50 | getParameter(...) : String | SpringUrlRedirect.java:471:46:471:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:491:23:491:50 | getParameter(...) : String | SpringUrlRedirect.java:492:34:492:37 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:492:34:492:37 | next : String | SpringUrlRedirect.java:452:40:452:50 | path : String | provenance | | +| SpringUrlRedirect.java:492:34:492:37 | next : String | SpringUrlRedirect.java:492:16:492:51 | branchingRedirect(...) | provenance | | +| SpringUrlRedirect.java:497:23:497:50 | getParameter(...) : String | SpringUrlRedirect.java:498:51:498:54 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:498:51:498:54 | next : String | SpringUrlRedirect.java:452:40:452:50 | path : String | provenance | | +| SpringUrlRedirect.java:498:51:498:54 | next : String | SpringUrlRedirect.java:498:33:498:68 | branchingRedirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:512:23:512:50 | getHeader(...) : String | SpringUrlRedirect.java:513:16:513:33 | ... + ... : String | provenance | Src:MaD:7 | +| SpringUrlRedirect.java:513:16:513:33 | ... + ... : String | SpringUrlRedirect.java:538:33:538:66 | internallySourcedRedirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:516:37:516:51 | paths : String[] [[]] : String | SpringUrlRedirect.java:517:30:517:34 | paths : String[] [[]] : String | provenance | | +| SpringUrlRedirect.java:517:30:517:34 | paths : String[] [[]] : String | SpringUrlRedirect.java:517:30:517:37 | ...[...] : String | provenance | | +| SpringUrlRedirect.java:517:30:517:37 | ...[...] : String | SpringUrlRedirect.java:517:16:517:37 | ... + ... : String | provenance | | +| SpringUrlRedirect.java:543:23:543:50 | getParameter(...) : String | SpringUrlRedirect.java:544:57:544:60 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:544:33:544:61 | new ..[] { .. } : String[] [[]] : String | SpringUrlRedirect.java:516:37:516:51 | paths : String[] [[]] : String | provenance | | +| SpringUrlRedirect.java:544:33:544:61 | new ..[] { .. } : String[] [[]] : String | SpringUrlRedirect.java:544:33:544:61 | secondRedirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:544:57:544:60 | next : String | SpringUrlRedirect.java:544:33:544:61 | new ..[] { .. } : String[] [[]] : String | provenance | | +| SpringUrlRedirect.java:555:23:555:50 | getParameter(...) : String | SpringUrlRedirect.java:556:48:556:51 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:571:23:571:50 | getParameter(...) : String | SpringUrlRedirect.java:572:46:572:49 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:582:48:582:58 | path : String | SpringUrlRedirect.java:584:20:584:37 | ... + ... : String | provenance | | +| SpringUrlRedirect.java:591:23:591:50 | getParameter(...) : String | SpringUrlRedirect.java:592:59:592:62 | next : String | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:592:59:592:62 | next : String | SpringUrlRedirect.java:582:48:582:58 | path : String | provenance | | +| SpringUrlRedirect.java:592:59:592:62 | next : String | SpringUrlRedirect.java:592:33:592:76 | branchingFallbackRedirect(...) | provenance | Sink:MaD:6 | +| SpringUrlRedirect.java:607:23:607:50 | getParameter(...) : String | SpringUrlRedirect.java:608:33:608:36 | next | provenance | Src:MaD:8 | +| SpringUrlRedirect.java:614:23:614:50 | getParameter(...) : String | SpringUrlRedirect.java:615:47:615:50 | next | provenance | Src:MaD:8 | +| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | provenance | Src:MaD:8 MaD:9 | +| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:45:28:45:39 | input : String | provenance | Src:MaD:8 | | 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:8 | -| mad/Test.java:9:16:9:41 | getParameter(...) : String | mad/Test.java:14:31:14:38 | source(...) : String | provenance | Src:MaD:7 | +| UrlRedirect.java:46:10:46:14 | input : String | UrlRedirect.java:46:10:46:40 | replaceAll(...) : String | provenance | MaD:9 | +| mad/Test.java:9:16:9:41 | getParameter(...) : String | mad/Test.java:14:31:14:38 | source(...) : String | provenance | Src:MaD:8 | | mad/Test.java:14:31:14:38 | source(...) : String | mad/Test.java:14:22:14:38 | (...)... | provenance | Sink:MaD:5 | models | 1 | Sink: io.micronaut.http; HttpResponse; true; permanentRedirect; (URI); ; Argument[0]; url-redirection; manual | @@ -45,10 +126,11 @@ models | 3 | Sink: io.micronaut.http; HttpResponse; true; seeOther; (URI); ; Argument[0]; url-redirection; manual | | 4 | Sink: io.micronaut.http; HttpResponse; true; temporaryRedirect; (URI); ; Argument[0]; url-redirection; manual | | 5 | Sink: org.kohsuke.stapler; HttpResponses; true; redirectTo; (String); ; Argument[0]; url-redirection; ai-manual | -| 6 | Source: javax.servlet.http; HttpServletRequest; false; getHeader; (String); ; ReturnValue; remote; manual | -| 7 | Source: javax.servlet; ServletRequest; false; getParameter; (String); ; ReturnValue; remote; manual | -| 8 | Summary: java.lang; String; false; replaceAll; ; ; Argument[this]; ReturnValue; taint; manual | -| 9 | Summary: java.net; URI; false; create; ; ; Argument[0]; ReturnValue; taint; manual | +| 6 | Sink: org.springframework.web.servlet; ModelAndView; false; ModelAndView; ; ; Argument[0]; url-forward; manual | +| 7 | Source: javax.servlet.http; HttpServletRequest; false; getHeader; (String); ; ReturnValue; remote; manual | +| 8 | Source: javax.servlet; ServletRequest; false; getParameter; (String); ; ReturnValue; remote; manual | +| 9 | Summary: java.lang; String; false; replaceAll; ; ; Argument[this]; ReturnValue; taint; manual | +| 10 | Summary: java.net; URI; false; create; ; ; Argument[0]; ReturnValue; taint; manual | nodes | MicronautUrlRedirect.java:12:39:12:63 | target : String | semmle.label | target : String | | MicronautUrlRedirect.java:14:38:14:55 | create(...) | semmle.label | create(...) | @@ -77,6 +159,81 @@ nodes | 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 | +| SpringUrlRedirect.java:184:23:184:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:185:33:185:46 | redirect(...) | semmle.label | redirect(...) | +| SpringUrlRedirect.java:185:42:185:45 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:190:23:190:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:191:33:191:46 | redirect(...) | semmle.label | redirect(...) | +| SpringUrlRedirect.java:191:42:191:45 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:196:23:196:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:197:30:197:33 | next | semmle.label | next | +| SpringUrlRedirect.java:202:23:202:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:203:41:203:44 | next | semmle.label | next | +| SpringUrlRedirect.java:309:23:309:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:310:33:310:36 | next | semmle.label | next | +| SpringUrlRedirect.java:316:23:316:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:317:47:317:50 | next | semmle.label | next | +| SpringUrlRedirect.java:323:23:323:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:324:33:324:46 | redirect(...) | semmle.label | redirect(...) | +| SpringUrlRedirect.java:324:42:324:45 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:333:23:333:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:334:33:334:36 | next | semmle.label | next | +| SpringUrlRedirect.java:339:23:339:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:340:47:340:50 | next | semmle.label | next | +| SpringUrlRedirect.java:345:23:345:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:346:33:346:46 | redirect(...) | semmle.label | redirect(...) | +| SpringUrlRedirect.java:346:42:346:45 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:351:23:351:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:353:21:353:24 | next | semmle.label | next | +| SpringUrlRedirect.java:359:23:359:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:360:33:360:36 | next | semmle.label | next | +| SpringUrlRedirect.java:368:23:368:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:369:46:369:49 | next | semmle.label | next | +| SpringUrlRedirect.java:378:23:378:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:379:46:379:49 | next | semmle.label | next | +| SpringUrlRedirect.java:395:23:395:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:396:46:396:49 | next | semmle.label | next | +| SpringUrlRedirect.java:404:23:404:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:405:46:405:49 | next | semmle.label | next | +| SpringUrlRedirect.java:415:23:415:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:416:46:416:49 | next | semmle.label | next | +| SpringUrlRedirect.java:418:21:418:24 | next | semmle.label | next | +| SpringUrlRedirect.java:438:23:438:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:439:60:439:63 | next | semmle.label | next | +| SpringUrlRedirect.java:452:40:452:50 | path : String | semmle.label | path : String | +| SpringUrlRedirect.java:456:16:456:33 | ... + ... : String | semmle.label | ... + ... : String | +| SpringUrlRedirect.java:470:23:470:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:471:46:471:49 | next | semmle.label | next | +| SpringUrlRedirect.java:491:23:491:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:492:16:492:51 | branchingRedirect(...) | semmle.label | branchingRedirect(...) | +| SpringUrlRedirect.java:492:34:492:37 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:497:23:497:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:498:33:498:68 | branchingRedirect(...) | semmle.label | branchingRedirect(...) | +| SpringUrlRedirect.java:498:51:498:54 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:512:23:512:50 | getHeader(...) : String | semmle.label | getHeader(...) : String | +| SpringUrlRedirect.java:513:16:513:33 | ... + ... : String | semmle.label | ... + ... : String | +| SpringUrlRedirect.java:516:37:516:51 | paths : String[] [[]] : String | semmle.label | paths : String[] [[]] : String | +| SpringUrlRedirect.java:517:16:517:37 | ... + ... : String | semmle.label | ... + ... : String | +| SpringUrlRedirect.java:517:30:517:34 | paths : String[] [[]] : String | semmle.label | paths : String[] [[]] : String | +| SpringUrlRedirect.java:517:30:517:37 | ...[...] : String | semmle.label | ...[...] : String | +| SpringUrlRedirect.java:538:33:538:66 | internallySourcedRedirect(...) | semmle.label | internallySourcedRedirect(...) | +| SpringUrlRedirect.java:543:23:543:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:544:33:544:61 | new ..[] { .. } : String[] [[]] : String | semmle.label | new ..[] { .. } : String[] [[]] : String | +| SpringUrlRedirect.java:544:33:544:61 | secondRedirect(...) | semmle.label | secondRedirect(...) | +| SpringUrlRedirect.java:544:57:544:60 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:555:23:555:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:556:48:556:51 | next | semmle.label | next | +| SpringUrlRedirect.java:571:23:571:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:572:46:572:49 | next | semmle.label | next | +| SpringUrlRedirect.java:582:48:582:58 | path : String | semmle.label | path : String | +| SpringUrlRedirect.java:584:20:584:37 | ... + ... : String | semmle.label | ... + ... : String | +| SpringUrlRedirect.java:591:23:591:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:592:33:592:76 | branchingFallbackRedirect(...) | semmle.label | branchingFallbackRedirect(...) | +| SpringUrlRedirect.java:592:59:592:62 | next : String | semmle.label | next : String | +| SpringUrlRedirect.java:607:23:607:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:608:33:608:36 | next | semmle.label | next | +| SpringUrlRedirect.java:614:23:614:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| SpringUrlRedirect.java:615:47:615:50 | 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(...) | @@ -91,4 +248,12 @@ nodes | 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(...) | +| SpringUrlRedirect.java:185:42:185:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | SpringUrlRedirect.java:185:33:185:46 | redirect(...) | +| SpringUrlRedirect.java:191:42:191:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | SpringUrlRedirect.java:191:33:191:46 | redirect(...) | +| SpringUrlRedirect.java:324:42:324:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | SpringUrlRedirect.java:324:33:324:46 | redirect(...) | +| SpringUrlRedirect.java:346:42:346:45 | next : String | SpringUrlRedirect.java:12:31:12:41 | path : String | SpringUrlRedirect.java:13:16:13:33 | ... + ... : String | SpringUrlRedirect.java:346:33:346:46 | redirect(...) | +| SpringUrlRedirect.java:492:34:492:37 | next : String | SpringUrlRedirect.java:452:40:452:50 | path : String | SpringUrlRedirect.java:456:16:456:33 | ... + ... : String | SpringUrlRedirect.java:492:16:492:51 | branchingRedirect(...) | +| SpringUrlRedirect.java:498:51:498:54 | next : String | SpringUrlRedirect.java:452:40:452:50 | path : String | SpringUrlRedirect.java:456:16:456:33 | ... + ... : String | SpringUrlRedirect.java:498:33:498:68 | branchingRedirect(...) | +| SpringUrlRedirect.java:544:33:544:61 | new ..[] { .. } : String[] [[]] : String | SpringUrlRedirect.java:516:37:516:51 | paths : String[] [[]] : String | SpringUrlRedirect.java:517:16:517:37 | ... + ... : String | SpringUrlRedirect.java:544:33:544:61 | secondRedirect(...) | +| SpringUrlRedirect.java:592:59:592:62 | next : String | SpringUrlRedirect.java:582:48:582:58 | path : String | SpringUrlRedirect.java:584:20:584:37 | ... + ... : String | SpringUrlRedirect.java:592:33:592:76 | branchingFallbackRedirect(...) | | 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(...) |