Skip to content

Commit 1ae9d68

Browse files
committed
Move and convert URL redirect sinks
Adds for them as well
1 parent f2ff2aa commit 1ae9d68

File tree

7 files changed

+48
-16
lines changed

7 files changed

+48
-16
lines changed

java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,19 @@
1313
import java
1414
import semmle.code.java.dataflow.FlowSources
1515
import semmle.code.java.security.UrlRedirect
16+
import semmle.code.java.dataflow.ExternalFlow
1617
import DataFlow::PathGraph
1718

1819
class UrlRedirectConfig extends TaintTracking::Configuration {
1920
UrlRedirectConfig() { this = "UrlRedirectConfig" }
2021

2122
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2223

23-
override predicate isSink(DataFlow::Node sink) { sink instanceof UrlRedirectSink }
24+
override predicate isSink(DataFlow::Node sink) {
25+
sink instanceof UrlRedirectSink
26+
or
27+
sinkNode(sink, "url-redirect")
28+
}
2429
}
2530

2631
from DataFlow::PathNode source, DataFlow::PathNode sink, UrlRedirectConfig conf

java/ql/src/semmle/code/java/frameworks/JaxWS.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,20 @@ class JaxRSConsumesAnnotation extends JaxRSAnnotation {
308308
JaxRSConsumesAnnotation() { this.getType().hasQualifiedName(getAJaxRsPackage(), "Consumes") }
309309
}
310310

311+
/** A URL redirection sink from JAX-RS */
312+
private class JaxRsUrlRedirectSink extends SinkModelCsv {
313+
override predicate row(string row) {
314+
row =
315+
[
316+
//`namespace; type; subtypes; name; signature; ext; input; kind`
317+
"javax.ws.rs.core;Response;true;seeOther;;;Argument[0];url-redirect",
318+
"javax.ws.rs.core;Response;true;temporaryRedirect;;;Argument[0];url-redirect",
319+
"jakarta.ws.rs.core;Response;true;seeOther;;;Argument[0];url-redirect",
320+
"jakarta.ws.rs.core;Response;true;temporaryRedirect;;;Argument[0];url-redirect"
321+
]
322+
}
323+
}
324+
311325
/**
312326
* Model Response:
313327
*

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,3 @@ private class ApacheUrlRedirectSink extends UrlRedirectSink {
3636
)
3737
}
3838
}
39-
40-
/** A URL redirection sink from JAX-RS */
41-
private class JaxRsUrlRedirectSink extends UrlRedirectSink {
42-
JaxRsUrlRedirectSink() {
43-
exists(MethodAccess ma |
44-
ma.getMethod()
45-
.getDeclaringType()
46-
.getAnAncestor()
47-
.hasQualifiedName(getAJaxRsPackage("core"), "Response") and
48-
ma.getMethod().getName() in ["seeOther", "temporaryRedirect"] and
49-
this.asExpr() = ma.getArgument(0)
50-
)
51-
}
52-
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
edges
2+
| UrlRedirect.java:10:32:10:61 | getParameter(...) : String | UrlRedirect.java:10:24:10:62 | new URI(...) |
3+
| UrlRedirect.java:13:41:13:70 | getParameter(...) : String | UrlRedirect.java:13:33:13:71 | new URI(...) |
4+
nodes
5+
| UrlRedirect.java:10:24:10:62 | new URI(...) | semmle.label | new URI(...) |
6+
| UrlRedirect.java:10:32:10:61 | getParameter(...) : String | semmle.label | getParameter(...) : String |
7+
| UrlRedirect.java:13:33:13:71 | new URI(...) | semmle.label | new URI(...) |
8+
| UrlRedirect.java:13:41:13:70 | getParameter(...) : String | semmle.label | getParameter(...) : String |
9+
#select
10+
| UrlRedirect.java:10:24:10:62 | new URI(...) | UrlRedirect.java:10:32:10:61 | getParameter(...) : String | UrlRedirect.java:10:24:10:62 | new URI(...) | Potentially untrusted URL redirection due to $@. | UrlRedirect.java:10:32:10:61 | getParameter(...) | user-provided value |
11+
| UrlRedirect.java:13:33:13:71 | new URI(...) | UrlRedirect.java:13:41:13:70 | getParameter(...) : String | UrlRedirect.java:13:33:13:71 | new URI(...) | Potentially untrusted URL redirection due to $@. | UrlRedirect.java:13:41:13:70 | getParameter(...) | user-provided value |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-601/UrlRedirect.ql
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import java.io.IOException;
2+
import java.net.URI;
3+
import javax.servlet.http.HttpServlet;
4+
import javax.servlet.http.HttpServletRequest;
5+
import javax.ws.rs.core.Response;
6+
7+
public class UrlRedirectJax extends HttpServlet {
8+
protected void doGetJax(HttpServletRequest request, Response jaxResponse) throws Exception {
9+
// BAD
10+
jaxResponse.seeOther(new URI(request.getParameter("target")));
11+
12+
// BAD
13+
jaxResponse.temporaryRedirect(new URI(request.getParameter("target")));
14+
}
15+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/javax-ws-rs-api-2.1.1:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/jsr181-api:${testdir}/../../../stubs/jaxws-api-2.0
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/javax-ws-rs-api-2.1.1:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/jsr181-api:${testdir}/../../../stubs/jaxws-api-2.0:${testdir}/../../../stubs/servlet-api-2.4

0 commit comments

Comments
 (0)