Skip to content

Commit effa2b1

Browse files
haby0haby0
committed
Add spring url redirection detect
1 parent 059a5f3 commit effa2b1

File tree

12 files changed

+584
-0
lines changed

12 files changed

+584
-0
lines changed

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import javax.servlet.http.HttpServletResponse;
2+
import org.springframework.stereotype.Controller;
3+
import org.springframework.web.bind.annotation.GetMapping;
4+
import org.springframework.web.servlet.ModelAndView;
5+
import org.springframework.web.servlet.view.RedirectView;
6+
7+
@Controller
8+
public class SpringUrlRedirect {
9+
10+
private final static String VALID_REDIRECT = "http://127.0.0.1";
11+
12+
@GetMapping("url1")
13+
public RedirectView bad1(String redirectUrl, HttpServletResponse response) throws Exception {
14+
RedirectView rv = new RedirectView();
15+
rv.setUrl(redirectUrl);
16+
return rv;
17+
}
18+
19+
@GetMapping("url2")
20+
public String bad2(String redirectUrl) {
21+
String url = "redirect:" + redirectUrl;
22+
return url;
23+
}
24+
25+
@GetMapping("url3")
26+
public RedirectView bad3(String redirectUrl) {
27+
RedirectView rv = new RedirectView(redirectUrl);
28+
return rv;
29+
}
30+
31+
@GetMapping("url4")
32+
public ModelAndView bad4(String redirectUrl) {
33+
return new ModelAndView("redirect:" + redirectUrl);
34+
}
35+
36+
@GetMapping("url5")
37+
public RedirectView good1(String redirectUrl) {
38+
RedirectView rv = new RedirectView();
39+
if (redirectUrl.startsWith(VALID_REDIRECT)){
40+
rv.setUrl(redirectUrl);
41+
}else {
42+
rv.setUrl(VALID_REDIRECT);
43+
}
44+
return rv;
45+
}
46+
}

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
7+
<overview>
8+
<p>Directly incorporating user input into a URL redirect request without validating the input
9+
can facilitate phishing attacks. In these attacks, unsuspecting users can be redirected to a
10+
malicious site that looks very similar to the real site they intend to visit, but which is
11+
controlled by the attacker.</p>
12+
13+
</overview>
14+
<recommendation>
15+
16+
<p>To guard against untrusted URL redirection, it is advisable to avoid putting user input
17+
directly into a redirect URL. Instead, maintain a list of authorized
18+
redirects on the server; then choose from that list based on the user input provided.</p>
19+
20+
</recommendation>
21+
<example>
22+
23+
<p>The following examples show the bad case and the good case respectively.
24+
In <code>bad1</code> method and <code>bad2</code> method and <code>bad3</code> method and
25+
<code>bad4</code> method, shows an HTTP request parameter being used directly in a URL redirect
26+
without validating the input, which facilitates phishing attacks. In <code>good1</code> method,
27+
shows how to solve this problem by verifying whether the user input is a known fixed string beginning.
28+
</p>
29+
30+
<sample src="SpringUrlRedirect.java" />
31+
32+
</example>
33+
<references>
34+
<li>A Guide To Spring Redirects: <a href="https://www.baeldung.com/spring-redirect-and-forward">Spring Redirects</a>.</li>
35+
<li>Url redirection - attack and defense: <a href="https://www.virtuesecurity.com/kb/url-redirection-attack-and-defense/">Url Redirection</a>.</li>
36+
</references>
37+
</qhelp>

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* @name Spring url redirection from remote source
3+
* @description Spring url redirection based on unvalidated user-input
4+
* may cause redirection to malicious web sites.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/spring-unvalidated-url-redirection
9+
* @tags security
10+
* external/cwe-601
11+
*/
12+
13+
import java
14+
import SpringUrlRedirect
15+
import semmle.code.java.dataflow.FlowSources
16+
import DataFlow::PathGraph
17+
18+
class SpringUrlRedirectFlowConfig extends TaintTracking::Configuration {
19+
SpringUrlRedirectFlowConfig() { this = "SpringUrlRedirectFlowConfig" }
20+
21+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
22+
23+
override predicate isSink(DataFlow::Node sink) { sink instanceof SpringUrlRedirectSink }
24+
25+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
26+
guard instanceof StartsWithSanitizer
27+
}
28+
29+
override predicate isSanitizer(DataFlow::Node node) {
30+
// Exclude the case where the left side of the concatenated string is not `redirect:`.
31+
// E.g: `String url = "/path?token=" + request.getParameter("token");`
32+
exists(AddExpr ae |
33+
ae.getRightOperand() = node.asExpr() and
34+
not ae instanceof RedirectBuilderExpr
35+
)
36+
}
37+
}
38+
39+
from DataFlow::PathNode source, DataFlow::PathNode sink, SpringUrlRedirectFlowConfig conf
40+
where conf.hasFlowPath(source, sink)
41+
select sink.getNode(), source, sink, "Potentially untrusted URL redirection due to $@.",
42+
source.getNode(), "user-provided value"

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import java
2+
import DataFlow
3+
import semmle.code.java.dataflow.FlowSources
4+
import semmle.code.java.dataflow.DataFlow2
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.frameworks.spring.SpringController
7+
8+
class StartsWithSanitizer extends DataFlow::BarrierGuard {
9+
StartsWithSanitizer() {
10+
this.(MethodAccess).getMethod().hasName("startsWith") and
11+
this.(MethodAccess).getMethod().getDeclaringType() instanceof TypeString and
12+
this.(MethodAccess).getMethod().getNumberOfParameters() = 1
13+
}
14+
15+
override predicate checks(Expr e, boolean branch) {
16+
e = this.(MethodAccess).getQualifier() and branch = true
17+
}
18+
}
19+
20+
/**
21+
* A concatenate expression using the string `redirect:` on the left.
22+
*
23+
* E.g: `"redirect:" + redirectUrl`
24+
*/
25+
class RedirectBuilderExpr extends AddExpr {
26+
RedirectBuilderExpr() {
27+
this.getLeftOperand().(CompileTimeConstantExpr).getStringValue() = "redirect:"
28+
}
29+
}
30+
31+
/** A URL redirection sink from spring controller method. */
32+
class SpringUrlRedirectSink extends DataFlow::Node {
33+
SpringUrlRedirectSink() {
34+
exists(RedirectBuilderExpr rbe | rbe.getRightOperand() = this.asExpr())
35+
or
36+
exists(MethodAccess ma |
37+
ma.getMethod().hasName("setUrl") and
38+
ma.getMethod()
39+
.getDeclaringType()
40+
.hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView") and
41+
ma.getArgument(0) = this.asExpr() and
42+
exists(RedirectViewFlowConfig rvfc | rvfc.hasFlowToExpr(ma.getQualifier()))
43+
)
44+
or
45+
exists(ClassInstanceExpr cie |
46+
cie.getConstructedType()
47+
.hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and
48+
cie.getArgument(0) = this.asExpr()
49+
)
50+
or
51+
exists(ClassInstanceExpr cie |
52+
cie.getConstructedType().hasQualifiedName("org.springframework.web.servlet", "ModelAndView") and
53+
cie.getArgument(0) = this.asExpr() and
54+
exists(RedirectBuilderFlowConfig rstrbfc | rstrbfc.hasFlowToExpr(cie.getArgument(0)))
55+
)
56+
}
57+
}
58+
59+
/** A data flow configuration tracing flow from remote sources to redirect builder expression. */
60+
private class RedirectBuilderFlowConfig extends DataFlow2::Configuration {
61+
RedirectBuilderFlowConfig() { this = "RedirectBuilderFlowConfig" }
62+
63+
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
64+
65+
override predicate isSink(DataFlow::Node sink) {
66+
exists(RedirectBuilderExpr rbe | rbe.getRightOperand() = sink.asExpr())
67+
}
68+
}
69+
70+
/** A data flow configuration tracing flow from RedirectView object to calling setUrl method. */
71+
private class RedirectViewFlowConfig extends DataFlow2::Configuration {
72+
RedirectViewFlowConfig() { this = "RedirectViewFlowConfig" }
73+
74+
override predicate isSource(DataFlow::Node src) {
75+
exists(ClassInstanceExpr cie |
76+
cie.getConstructedType()
77+
.hasQualifiedName("org.springframework.web.servlet.view", "RedirectView") and
78+
cie = src.asExpr()
79+
)
80+
}
81+
82+
override predicate isSink(DataFlow::Node sink) {
83+
exists(MethodAccess ma |
84+
ma.getMethod().hasName("setUrl") and
85+
ma.getMethod()
86+
.getDeclaringType()
87+
.hasQualifiedName("org.springframework.web.servlet.view", "AbstractUrlBasedView") and
88+
ma.getQualifier() = sink.asExpr()
89+
)
90+
}
91+
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
edges
2+
| SpringUrlRedirect.java:13:30:13:47 | redirectUrl : String | SpringUrlRedirect.java:15:19:15:29 | redirectUrl |
3+
| SpringUrlRedirect.java:20:24:20:41 | redirectUrl : String | SpringUrlRedirect.java:21:36:21:46 | redirectUrl |
4+
| SpringUrlRedirect.java:26:30:26:47 | redirectUrl : String | SpringUrlRedirect.java:27:44:27:54 | redirectUrl |
5+
| SpringUrlRedirect.java:32:30:32:47 | redirectUrl : String | SpringUrlRedirect.java:33:47:33:57 | redirectUrl |
6+
nodes
7+
| SpringUrlRedirect.java:13:30:13:47 | redirectUrl : String | semmle.label | redirectUrl : String |
8+
| SpringUrlRedirect.java:15:19:15:29 | redirectUrl | semmle.label | redirectUrl |
9+
| SpringUrlRedirect.java:20:24:20:41 | redirectUrl : String | semmle.label | redirectUrl : String |
10+
| SpringUrlRedirect.java:21:36:21:46 | redirectUrl | semmle.label | redirectUrl |
11+
| SpringUrlRedirect.java:26:30:26:47 | redirectUrl : String | semmle.label | redirectUrl : String |
12+
| SpringUrlRedirect.java:27:44:27:54 | redirectUrl | semmle.label | redirectUrl |
13+
| SpringUrlRedirect.java:32:30:32:47 | redirectUrl : String | semmle.label | redirectUrl : String |
14+
| SpringUrlRedirect.java:33:47:33:57 | redirectUrl | semmle.label | redirectUrl |
15+
#select
16+
| 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 |
17+
| 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 |
18+
| 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 |
19+
| 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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import javax.servlet.http.HttpServletResponse;
2+
import org.springframework.stereotype.Controller;
3+
import org.springframework.web.bind.annotation.GetMapping;
4+
import org.springframework.web.servlet.ModelAndView;
5+
import org.springframework.web.servlet.view.RedirectView;
6+
7+
@Controller
8+
public class SpringUrlRedirect {
9+
10+
private final static String VALID_REDIRECT = "http://127.0.0.1";
11+
12+
@GetMapping("url1")
13+
public RedirectView bad1(String redirectUrl, HttpServletResponse response) throws Exception {
14+
RedirectView rv = new RedirectView();
15+
rv.setUrl(redirectUrl);
16+
return rv;
17+
}
18+
19+
@GetMapping("url2")
20+
public String bad2(String redirectUrl) {
21+
String url = "redirect:" + redirectUrl;
22+
return url;
23+
}
24+
25+
@GetMapping("url3")
26+
public RedirectView bad3(String redirectUrl) {
27+
RedirectView rv = new RedirectView(redirectUrl);
28+
return rv;
29+
}
30+
31+
@GetMapping("url4")
32+
public ModelAndView bad4(String redirectUrl) {
33+
return new ModelAndView("redirect:" + redirectUrl);
34+
}
35+
36+
@GetMapping("url5")
37+
public RedirectView good1(String redirectUrl) {
38+
RedirectView rv = new RedirectView();
39+
if (redirectUrl.startsWith(VALID_REDIRECT)){
40+
rv.setUrl(redirectUrl);
41+
}else {
42+
rv.setUrl(VALID_REDIRECT);
43+
}
44+
return rv;
45+
}
46+
47+
@GetMapping("url6")
48+
public ModelAndView good2(String token) {
49+
String url = "/edit?token=" + token;
50+
return new ModelAndView("redirect:" + url);
51+
}
52+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-601/SpringUrlRedirect.ql

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.2.3/

0 commit comments

Comments
 (0)