Add spring url redirection detect

java/ql/src/experimental/Security/CWE/CWE-601/SpringUrlRedirect.java

@@ -0,0 +1,46 @@
+import javax.servlet.http.HttpServletResponse;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.servlet.ModelAndView;
+import org.springframework.web.servlet.view.RedirectView;
+
+@Controller
+public class SpringUrlRedirect {
+
+    private final static String VALID_REDIRECT = "http://127.0.0.1";
+
+    @GetMapping("url1")
+    public RedirectView bad1(String redirectUrl, HttpServletResponse response) throws Exception {
+        RedirectView rv = new RedirectView();
+        rv.setUrl(redirectUrl);
+        return rv;
+    }
+
+    @GetMapping("url2")
+    public String bad2(String redirectUrl) {
+        String url = "redirect:" + redirectUrl;
+        return url;
+    }
+
+    @GetMapping("url3")
+    public RedirectView bad3(String redirectUrl) {
+        RedirectView rv = new RedirectView(redirectUrl);
+        return rv;
+    }
+
+    @GetMapping("url4")
+    public ModelAndView bad4(String redirectUrl) {
+        return new ModelAndView("redirect:" + redirectUrl);
+    }
+
+    @GetMapping("url5")
+    public RedirectView good1(String redirectUrl) {
+        RedirectView rv = new RedirectView();
+        if (redirectUrl.startsWith(VALID_REDIRECT)){
+            rv.setUrl(redirectUrl);
+        }else {
+            rv.setUrl(VALID_REDIRECT);
+        }
+        return rv;
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-601/SpringUrlRedirect.qhelp

@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>Directly incorporating user input into a URL redirect request without validating the input
+can facilitate phishing attacks. In these attacks, unsuspecting users can be redirected to a
+malicious site that looks very similar to the real site they intend to visit, but which is
+controlled by the attacker.</p>
+
+</overview>
+<recommendation>
+
+<p>To guard against untrusted URL redirection, it is advisable to avoid putting user input
+directly into a redirect URL. Instead, maintain a list of authorized
+redirects on the server; then choose from that list based on the user input provided.</p>
+
+</recommendation>
+<example>
+
+<p>The following examples show the bad case and the good case respectively.
+In <code>bad1</code> method and <code>bad2</code> method and <code>bad3</code> method and 
+<code>bad4</code> method, shows an HTTP request parameter being used directly in a URL redirect 
+without validating the input, which facilitates phishing attacks. In <code>good1</code> method, 
+shows how to solve this problem by verifying whether the user input is a known fixed string beginning.
+</p>
+
+<sample src="SpringUrlRedirect.java" />
+
+</example>
+<references>
+<li>A Guide To Spring Redirects: <a href="https://www.baeldung.com/spring-redirect-and-forward">Spring Redirects</a>.</li>
+<li>Url redirection - attack and defense: <a href="https://www.virtuesecurity.com/kb/url-redirection-attack-and-defense/">Url Redirection</a>.</li>
+</references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-601/SpringUrlRedirect.ql

@@ -0,0 +1,42 @@
+/**
+ * @name Spring url redirection from remote source
+ * @description Spring url redirection based on unvalidated user-input
+ *              may cause redirection to malicious web sites.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/spring-unvalidated-url-redirection
+ * @tags security
+ *       external/cwe-601
+ */
+
+import java
+import SpringUrlRedirect
+import semmle.code.java.dataflow.FlowSources
+import DataFlow::PathGraph
+
+class SpringUrlRedirectFlowConfig extends TaintTracking::Configuration {
+  SpringUrlRedirectFlowConfig() { this = "SpringUrlRedirectFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof SpringUrlRedirectSink }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StartsWithSanitizer
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    // Exclude the case where the left side of the concatenated string is not `redirect:`.
+    // E.g: `String url = "/path?token=" + request.getParameter("token");`
+    exists(AddExpr ae |
+      ae.getRightOperand() = node.asExpr() and
+      not ae instanceof RedirectBuilderExpr
+    )
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, SpringUrlRedirectFlowConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Potentially untrusted URL redirection due to $@.",
+  source.getNode(), "user-provided value"

java/ql/src/experimental/Security/CWE/CWE-601/SpringUrlRedirect.qll

@@ -0,0 +1,91 @@
+import java
+import DataFlow
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.DataFlow2
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.frameworks.spring.SpringController
+
+class StartsWithSanitizer extends DataFlow::BarrierGuard {
+  StartsWithSanitizer() {
+    this.(MethodAccess).getMethod().hasName("startsWith") and
+    this.(MethodAccess).getMethod().getDeclaringType() instanceof TypeString and
+    this.(MethodAccess).getMethod().getNumberOfParameters() = 1
+  }
+
+  override predicate checks(Expr e, boolean branch) {
+    e = this.(MethodAccess).getQualifier() and branch = true
+  }
+}
+
+/**
+ * A concatenate expression using the string `redirect:` on the left.
+ *
+ * E.g: `"redirect:" + redirectUrl`
+ */
+class RedirectBuilderExpr extends AddExpr {
+  RedirectBuilderExpr() {
+    this.getLeftOperand().(CompileTimeConstantExpr).getStringValue() = "redirect:"
+  }
+}
+
+/** A URL redirection sink from spring controller method. */
+class SpringUrlRedirectSink extends DataFlow::Node {
+  SpringUrlRedirectSink() {
+    exists(RedirectBuilderExpr rbe | rbe.getRightOperand() = this.asExpr())
+    or
+    exists(MethodAccess ma |
+      ma.getMethod().hasName("setUrl") and
+      ma.getMethod()
+          .getDeclaringType()
+          .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView") and
+      ma.getArgument(0) = this.asExpr() and
+      exists(RedirectViewFlowConfig rvfc | rvfc.hasFlowToExpr(ma.getQualifier()))
+    )
+    or
+    exists(ClassInstanceExpr cie |
+      cie.getConstructedType()
+          .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and
+      cie.getArgument(0) = this.asExpr()
+    )
+    or
+    exists(ClassInstanceExpr cie |
+      cie.getConstructedType().hasQualifiedName("org.springframework.web.servlet", "ModelAndView") and
+      cie.getArgument(0) = this.asExpr() and
+      exists(RedirectBuilderFlowConfig rstrbfc | rstrbfc.hasFlowToExpr(cie.getArgument(0)))
+    )
+  }
+}
+
+/** A data flow configuration tracing flow from remote sources to redirect builder expression. */
+private class RedirectBuilderFlowConfig extends DataFlow2::Configuration {
+  RedirectBuilderFlowConfig() { this = "RedirectBuilderFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(RedirectBuilderExpr rbe | rbe.getRightOperand() = sink.asExpr())
+  }
+}
+
+/** A data flow configuration tracing flow from RedirectView object to calling setUrl method. */
+private class RedirectViewFlowConfig extends DataFlow2::Configuration {
+  RedirectViewFlowConfig() { this = "RedirectViewFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) {
+    exists(ClassInstanceExpr cie |
+      cie.getConstructedType()
+          .hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and
+      cie = src.asExpr()
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      ma.getMethod().hasName("setUrl") and
+      ma.getMethod()
+          .getDeclaringType()
+          .hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView") and
+      ma.getQualifier() = sink.asExpr()
+    )
+  }
+}

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

@@ -0,0 +1,19 @@
+edges
+| SpringUrlRedirect.java:13:30:13:47 | redirectUrl : String | SpringUrlRedirect.java:15:19:15:29 | redirectUrl |
+| SpringUrlRedirect.java:20:24:20:41 | redirectUrl : String | SpringUrlRedirect.java:21:36:21:46 | redirectUrl |
+| SpringUrlRedirect.java:26:30:26:47 | redirectUrl : String | SpringUrlRedirect.java:27:44:27:54 | redirectUrl |
+| SpringUrlRedirect.java:32:30:32:47 | redirectUrl : String | SpringUrlRedirect.java:33:47:33:57 | redirectUrl |
+nodes
+| SpringUrlRedirect.java:13:30:13:47 | redirectUrl : String | semmle.label | redirectUrl : String |
+| SpringUrlRedirect.java:15:19:15:29 | redirectUrl | semmle.label | redirectUrl |
+| SpringUrlRedirect.java:20:24:20:41 | redirectUrl : String | semmle.label | redirectUrl : String |
+| SpringUrlRedirect.java:21:36:21:46 | redirectUrl | semmle.label | redirectUrl |
+| SpringUrlRedirect.java:26:30:26:47 | redirectUrl : String | semmle.label | redirectUrl : String |
+| SpringUrlRedirect.java:27:44:27:54 | redirectUrl | semmle.label | redirectUrl |
+| SpringUrlRedirect.java:32:30:32:47 | redirectUrl : String | semmle.label | redirectUrl : String |
+| SpringUrlRedirect.java:33:47:33:57 | redirectUrl | semmle.label | redirectUrl |
+#select
+| SpringUrlRedirect.java:15:19:15:29 | redirectUrl | SpringUrlRedirect.java:13:30:13:47 | redirectUrl : String | SpringUrlRedirect.java:15:19:15:29 | redirectUrl | Potentially untrusted URL redirection due to $@. | SpringUrlRedirect.java:13:30:13:47 | redirectUrl | user-provided value |
+| SpringUrlRedirect.java:21:36:21:46 | redirectUrl | SpringUrlRedirect.java:20:24:20:41 | redirectUrl : String | SpringUrlRedirect.java:21:36:21:46 | redirectUrl | Potentially untrusted URL redirection due to $@. | SpringUrlRedirect.java:20:24:20:41 | redirectUrl | user-provided value |
+| SpringUrlRedirect.java:27:44:27:54 | redirectUrl | SpringUrlRedirect.java:26:30:26:47 | redirectUrl : String | SpringUrlRedirect.java:27:44:27:54 | redirectUrl | Potentially untrusted URL redirection due to $@. | SpringUrlRedirect.java:26:30:26:47 | redirectUrl | user-provided value |
+| SpringUrlRedirect.java:33:47:33:57 | redirectUrl | SpringUrlRedirect.java:32:30:32:47 | redirectUrl : String | SpringUrlRedirect.java:33:47:33:57 | redirectUrl | Potentially untrusted URL redirection due to $@. | SpringUrlRedirect.java:32:30:32:47 | redirectUrl | user-provided value |

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

@@ -0,0 +1,52 @@
+import javax.servlet.http.HttpServletResponse;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.servlet.ModelAndView;
+import org.springframework.web.servlet.view.RedirectView;
+
+@Controller
+public class SpringUrlRedirect {
+
+    private final static String VALID_REDIRECT = "http://127.0.0.1";
+
+    @GetMapping("url1")
+    public RedirectView bad1(String redirectUrl, HttpServletResponse response) throws Exception {
+        RedirectView rv = new RedirectView();
+        rv.setUrl(redirectUrl);
+        return rv;
+    }
+
+    @GetMapping("url2")
+    public String bad2(String redirectUrl) {
+        String url = "redirect:" + redirectUrl;
+        return url;
+    }
+
+    @GetMapping("url3")
+    public RedirectView bad3(String redirectUrl) {
+        RedirectView rv = new RedirectView(redirectUrl);
+        return rv;
+    }
+
+    @GetMapping("url4")
+    public ModelAndView bad4(String redirectUrl) {
+        return new ModelAndView("redirect:" + redirectUrl);
+    }
+
+    @GetMapping("url5")
+    public RedirectView good1(String redirectUrl) {
+        RedirectView rv = new RedirectView();
+        if (redirectUrl.startsWith(VALID_REDIRECT)){
+            rv.setUrl(redirectUrl);
+        }else {
+            rv.setUrl(VALID_REDIRECT);
+        }
+        return rv;
+    }
+
+    @GetMapping("url6")
+    public ModelAndView good2(String token) {
+        String url = "/edit?token=" + token;
+        return new ModelAndView("redirect:" + url);
+    }
+}

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

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-601/SpringUrlRedirect.ql

java/ql/test/experimental/query-tests/security/CWE-601/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.2.3/

java/ql/test/stubs/springframework-5.2.3/org/springframework/web/servlet/ModelAndView.java

@@ -0,0 +1,107 @@
+package org.springframework.web.servlet;
+
+import java.util.Map;
+import org.springframework.http.HttpStatus;
+import org.springframework.lang.Nullable;
+
+public class ModelAndView {
+    @Nullable
+    private Object view;
+    @Nullable
+    private HttpStatus status;
+    private boolean cleared = false;
+
+    public ModelAndView() {
+    }
+
+    public ModelAndView(String viewName) {
+        this.view = viewName;
+    }
+
+    public ModelAndView(View view) {
+        this.view = view;
+    }
+
+    public ModelAndView(String viewName, @Nullable Map<String, ?> model) { }
+
+    public ModelAndView(View view, @Nullable Map<String, ?> model) { }
+
+    public ModelAndView(String viewName, HttpStatus status) { }
+
+    public ModelAndView(@Nullable String viewName, @Nullable Map<String, ?> model, @Nullable HttpStatus status) { }
+
+    public ModelAndView(String viewName, String modelName, Object modelObject) { }
+
+    public ModelAndView(View view, String modelName, Object modelObject) { }
+
+    public void setViewName(@Nullable String viewName) {
+        this.view = viewName;
+    }
+
+    @Nullable
+    public String getViewName() {
+        return "";
+    }
+
+    public void setView(@Nullable View view) { }
+
+    @Nullable
+    public View getView() {
+        return null;
+    }
+
+    public boolean hasView() {
+        return true;
+    }
+
+    public boolean isReference() {
+        return true;
+    }
+
+    @Nullable
+    protected Map<String, Object> getModelInternal() {
+        return null;
+    }
+
+    public Map<String, Object> getModel() {
+        return null;
+    }
+
+    public void setStatus(@Nullable HttpStatus status) { }
+
+    @Nullable
+    public HttpStatus getStatus() {
+        return this.status;
+    }
+
+    public ModelAndView addObject(String attributeName, @Nullable Object attributeValue) {
+        return null;
+    }
+
+    public ModelAndView addObject(Object attributeValue) {
+        return null;
+    }
+
+    public ModelAndView addAllObjects(@Nullable Map<String, ?> modelMap) {
+        return null;
+    }
+
+    public void clear() { }
+
+    public boolean isEmpty() {
+        return true;
+    }
+
+    public boolean wasCleared() {
+        return true;
+    }
+
+    public String toString() {
+        return "";
+    }
+
+    private String formatView() {
+        return "";
+    }
+}
+

java/ql/test/stubs/springframework-5.2.3/org/springframework/web/servlet/View.java

@@ -0,0 +1,20 @@
+package org.springframework.web.servlet;
+
+import java.util.Map;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.springframework.lang.Nullable;
+
+public interface View {
+    String RESPONSE_STATUS_ATTRIBUTE = View.class.getName() + ".responseStatus";
+    String PATH_VARIABLES = View.class.getName() + ".pathVariables";
+    String SELECTED_CONTENT_TYPE = View.class.getName() + ".selectedContentType";
+
+    @Nullable
+    default String getContentType() {
+        return null;
+    }
+
+    void render(@Nullable Map<String, ?> var1, HttpServletRequest var2, HttpServletResponse var3) throws Exception;
+}
+

java/ql/test/stubs/springframework-5.2.3/org/springframework/web/servlet/view/AbstractUrlBasedView.java

@@ -0,0 +1,39 @@
+package org.springframework.web.servlet.view;
+
+import java.util.Locale;
+import org.springframework.lang.Nullable;
+
+public abstract class AbstractUrlBasedView {
+    @Nullable
+    private String url;
+
+    protected AbstractUrlBasedView() { }
+
+    protected AbstractUrlBasedView(String url) {
+        this.url = url;
+    }
+
+    public void setUrl(@Nullable String url) {
+        this.url = url;
+    }
+
+    @Nullable
+    public String getUrl() {
+        return "";
+    }
+
+    public void afterPropertiesSet() throws Exception { }
+
+    protected boolean isUrlRequired() {
+        return true;
+    }
+
+    public boolean checkResource(Locale locale) throws Exception {
+        return true;
+    }
+
+    public String toString() {
+        return "";
+    }
+}
+

java/ql/test/stubs/springframework-5.2.3/org/springframework/web/servlet/view/RedirectView.java

@@ -0,0 +1,129 @@
+package org.springframework.web.servlet.view;
+
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Array;
+import java.net.URLEncoder;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.springframework.http.HttpStatus;
+import org.springframework.lang.Nullable;
+
+public class RedirectView extends AbstractUrlBasedView {
+    private static final Pattern URI_TEMPLATE_VARIABLE_PATTERN = Pattern.compile("\\{([^/]+?)\\}");
+    private boolean contextRelative = false;
+    private boolean http10Compatible = true;
+    private boolean exposeModelAttributes = true;
+    @Nullable
+    private String encodingScheme;
+    @Nullable
+    private HttpStatus statusCode;
+    private boolean expandUriTemplateVariables = true;
+    private boolean propagateQueryParams = false;
+    @Nullable
+    private String[] hosts;
+
+    public RedirectView() { }
+
+    public RedirectView(String url) { }
+
+    public RedirectView(String url, boolean contextRelative) { }
+
+    public RedirectView(String url, boolean contextRelative, boolean http10Compatible) { }
+
+    public RedirectView(String url, boolean contextRelative, boolean http10Compatible, boolean exposeModelAttributes) { }
+
+    public void setContextRelative(boolean contextRelative) { }
+
+    public void setHttp10Compatible(boolean http10Compatible) { }
+
+    public void setExposeModelAttributes(boolean exposeModelAttributes) { }
+
+    public void setEncodingScheme(String encodingScheme) { }
+
+    public void setStatusCode(HttpStatus statusCode) { }
+
+    public void setExpandUriTemplateVariables(boolean expandUriTemplateVariables) { }
+
+    public void setPropagateQueryParams(boolean propagateQueryParams) { }
+
+    public boolean isPropagateQueryProperties() {
+        return true;
+    }
+
+    public void setHosts(@Nullable String... hosts) { }
+
+    @Nullable
+    public String[] getHosts() {
+        return this.hosts;
+    }
+
+    public boolean isRedirectView() {
+        return true;
+    }
+
+    protected boolean isContextRequired() {
+        return false;
+    }
+
+    protected void renderMergedOutputModel(Map<String, Object> model, HttpServletRequest request, HttpServletResponse response) throws IOException { }
+
+    protected final String createTargetUrl(Map<String, Object> model, HttpServletRequest request) throws UnsupportedEncodingException {
+        return "";
+    }
+
+    private String getContextPath(HttpServletRequest request) {
+        return "";
+    }
+
+    protected StringBuilder replaceUriTemplateVariables(String targetUrl, Map<String, Object> model, Map<String, String> currentUriVariables, String encodingScheme) throws UnsupportedEncodingException {
+        return null;
+    }
+
+    private Map<String, String> getCurrentRequestUriVariables(HttpServletRequest request) {
+        return null;
+    }
+
+    protected void appendCurrentQueryParams(StringBuilder targetUrl, HttpServletRequest request) { }
+
+    protected void appendQueryProperties(StringBuilder targetUrl, Map<String, Object> model, String encodingScheme) throws UnsupportedEncodingException { }
+
+    protected Map<String, Object> queryProperties(Map<String, Object> model) {
+        return null;
+    }
+
+    protected boolean isEligibleProperty(String key, @Nullable Object value) {
+        return true;
+    }
+
+    protected boolean isEligibleValue(@Nullable Object value) {
+        return true;
+    }
+
+    protected String urlEncode(String input, String encodingScheme) throws UnsupportedEncodingException {
+        return "";
+    }
+
+    protected String updateTargetUrl(String targetUrl, Map<String, Object> model, HttpServletRequest request, HttpServletResponse response) {
+        return "";
+    }
+
+    protected void sendRedirect(HttpServletRequest request, HttpServletResponse response, String targetUrl, boolean http10Compatible) throws IOException { }
+
+    protected boolean isRemoteHost(String targetUrl) {
+        return true;
+    }
+
+    protected HttpStatus getHttp11StatusCode(HttpServletRequest request, HttpServletResponse response, String targetUrl) {
+        return this.statusCode;
+    }
+}
+