Skip to content

Commit 09bc21d

Browse files
Jami CogswellJami Cogswell
Jami Cogswell
authored and
Jami Cogswell
committed
Java: rename 'UnsafeUrlForward' to 'UrlForward'
1 parent 6e7c054 commit 09bc21d

File tree

11 files changed

+57
-57
lines changed

11 files changed

+57
-57
lines changed

‎java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll renamed to ‎java/ql/lib/semmle/code/java/security/UrlForward.qll

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
1-
/** Provides classes related to unsafe URL forwarding in Java. */
1+
/** Provides classes to reason about URL forward attacks. */
22

33
import java
44
private import semmle.code.java.dataflow.ExternalFlow
55
private import semmle.code.java.dataflow.FlowSources
66
private import semmle.code.java.dataflow.StringPrefixes
77

8-
/** A sink for unsafe URL forward vulnerabilities. */
9-
abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
8+
/** A URL forward sink. */
9+
abstract class UrlForwardSink extends DataFlow::Node { }
1010

11-
/** A default sink representing methods susceptible to unsafe URL forwarding. */
12-
private class DefaultUnsafeUrlForwardSink extends UnsafeUrlForwardSink {
13-
DefaultUnsafeUrlForwardSink() { sinkNode(this, "url-forward") }
11+
/** A default sink representing methods susceptible to URL forwarding attacks. */
12+
private class DefaultUrlForwardSink extends UrlForwardSink {
13+
DefaultUrlForwardSink() { sinkNode(this, "url-forward") }
1414
}
1515

1616
/**
1717
* An expression appended (perhaps indirectly) to `"forward:"`, and which
1818
* is reachable from a Spring entry point.
1919
*/
20-
private class SpringUrlForwardSink extends UnsafeUrlForwardSink {
20+
private class SpringUrlForwardSink extends UrlForwardSink {
2121
SpringUrlForwardSink() {
2222
// TODO: check if can use MaD "Annotated" for `SpringRequestMappingMethod` or if too complicated for MaD (probably too complicated).
2323
any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable()) and
@@ -32,10 +32,10 @@ private class ForwardPrefix extends InterestingPrefix {
3232
override int getOffset() { result = 0 }
3333
}
3434

35-
/** A sanitizer for unsafe URL forward vulnerabilities. */
36-
abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { }
35+
/** A URL forward sanitizer. */
36+
abstract class UrlForwardSanitizer extends DataFlow::Node { }
3737

38-
private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
38+
private class PrimitiveSanitizer extends UrlForwardSanitizer {
3939
PrimitiveSanitizer() {
4040
this.getType() instanceof PrimitiveType or
4141
this.getType() instanceof BoxedType or
@@ -44,7 +44,7 @@ private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
4444
}
4545

4646
// TODO: double-check this sanitizer (and should I switch all "sanitizer" naming to "barrier" instead?)
47-
private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
47+
private class FollowsSanitizingPrefix extends UrlForwardSanitizer {
4848
FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
4949
}
5050

‎java/ql/lib/semmle/code/java/security/UnsafeUrlForwardQuery.qll renamed to ‎java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
/** Provides configurations to be used in queries related to unsafe URL forwarding. */
1+
/** Provides a taint-tracking configuration for reasoning about URL forwarding. */
22

33
import java
4-
import semmle.code.java.security.UnsafeUrlForward
4+
import semmle.code.java.security.UrlForward
55
import semmle.code.java.dataflow.FlowSources
66
import semmle.code.java.dataflow.TaintTracking
77
import semmle.code.java.security.PathSanitizer
88

99
/**
10-
* A taint-tracking configuration for untrusted user input in a URL forward or include.
10+
* A taint-tracking configuration for reasoning about URL forwarding.
1111
*/
12-
module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
12+
module UrlForwardFlowConfig implements DataFlow::ConfigSig {
1313
predicate isSource(DataFlow::Node source) {
1414
source instanceof ThreatModelFlowSource and
15-
// TODO: move below logic to class in UnsafeUrlForward.qll? And check exactly why these were excluded.
15+
// TODO: move below logic to class in UrlForward.qll? And check exactly why these were excluded.
1616
not exists(MethodCall ma, Method m | ma.getMethod() = m |
1717
(
1818
m instanceof HttpServletRequestGetRequestUriMethod or
@@ -23,10 +23,10 @@ module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
2323
)
2424
}
2525

26-
predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeUrlForwardSink }
26+
predicate isSink(DataFlow::Node sink) { sink instanceof UrlForwardSink }
2727

2828
predicate isBarrier(DataFlow::Node node) {
29-
node instanceof UnsafeUrlForwardSanitizer or
29+
node instanceof UrlForwardSanitizer or
3030
node instanceof PathInjectionSanitizer
3131
}
3232

@@ -35,6 +35,6 @@ module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
3535
}
3636

3737
/**
38-
* Taint-tracking flow for untrusted user input in a URL forward or include.
38+
* Taint-tracking flow for URL forwarding.
3939
*/
40-
module UnsafeUrlForwardFlow = TaintTracking::Global<UnsafeUrlForwardFlowConfig>;
40+
module UrlForwardFlow = TaintTracking::Global<UrlForwardFlowConfig>;

‎java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.java renamed to ‎java/ql/src/Security/CWE/CWE-552/UrlForward.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import org.springframework.web.servlet.ModelAndView;
88

99
@Controller
10-
public class UnsafeUrlForward {
10+
public class UrlForward {
1111

1212
@GetMapping("/bad1")
1313
public ModelAndView bad1(String url) {

‎java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.qhelp renamed to ‎java/ql/src/Security/CWE/CWE-552/UrlForward.qhelp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ without validating the input, which may cause file leakage. In the <code>good1</
2727
ordinary forwarding requests are shown, which will not cause file leakage.
2828
</p>
2929

30-
<sample src="UnsafeUrlForward.java" />
30+
<sample src="UrlForward.java" />
3131

3232
<p>The following examples show an HTTP request parameter or request path being used directly in a
3333
request dispatcher of Java EE without validating the input, which allows sensitive file exposure

‎java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.ql renamed to ‎java/ql/src/Security/CWE/CWE-552/UrlForward.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
*/
1515

1616
import java
17-
import semmle.code.java.security.UnsafeUrlForwardQuery
18-
import UnsafeUrlForwardFlow::PathGraph
17+
import semmle.code.java.security.UrlForwardQuery
18+
import UrlForwardFlow::PathGraph
1919

20-
from UnsafeUrlForwardFlow::PathNode source, UnsafeUrlForwardFlow::PathNode sink
21-
where UnsafeUrlForwardFlow::flowPath(source, sink)
20+
from UrlForwardFlow::PathNode source, UrlForwardFlow::PathNode sink
21+
where UrlForwardFlow::flowPath(source, sink)
2222
select sink.getNode(), source, sink, "Untrusted URL forward depends on a $@.", source.getNode(),
2323
"user-provided value"

‎java/ql/test/query-tests/security/CWE-552/UnsafeRequestPath.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
2020
String path = ((HttpServletRequest) request).getServletPath();
2121
// A sample payload "/%57EB-INF/web.xml" can bypass this `startsWith` check
2222
if (path != null && !path.startsWith("/WEB-INF")) {
23-
request.getRequestDispatcher(path).forward(request, response); // $ hasUnsafeUrlForward
23+
request.getRequestDispatcher(path).forward(request, response); // $ hasUrlForward
2424
} else {
2525
chain.doFilter(request, response);
2626
}

‎java/ql/test/query-tests/security/CWE-552/UnsafeServletRequestDispatch.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
2929
rd.forward(request, response);
3030
} else {
3131
ServletContext sc = cfg.getServletContext();
32-
RequestDispatcher rd = sc.getRequestDispatcher(returnURL); // $ hasUnsafeUrlForward
32+
RequestDispatcher rd = sc.getRequestDispatcher(returnURL); // $ hasUrlForward
3333
rd.forward(request, response);
3434
}
3535
}
@@ -45,7 +45,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
4545
RequestDispatcher rd = request.getRequestDispatcher("/Login.jsp");
4646
rd.forward(request, response);
4747
} else {
48-
RequestDispatcher rd = request.getRequestDispatcher(returnURL); // $ hasUnsafeUrlForward
48+
RequestDispatcher rd = request.getRequestDispatcher(returnURL); // $ hasUrlForward
4949
rd.forward(request, response);
5050
}
5151
}
@@ -73,7 +73,7 @@ protected void doHead2(HttpServletRequest request, HttpServletResponse response)
7373
// A sample payload "/pages/welcome.jsp/../WEB-INF/web.xml" can bypass the `startsWith` check
7474
// The payload "/pages/welcome.jsp/../../%57EB-INF/web.xml" can bypass the check as well since RequestDispatcher will decode `%57` as `W`
7575
if (path.startsWith(BASE_PATH)) {
76-
request.getServletContext().getRequestDispatcher(path).include(request, response); // $ hasUnsafeUrlForward
76+
request.getServletContext().getRequestDispatcher(path).include(request, response); // $ hasUrlForward
7777
}
7878
}
7979

@@ -110,7 +110,7 @@ protected void doHead5(HttpServletRequest request, HttpServletResponse response)
110110
Path requestedPath = Paths.get(BASE_PATH).resolve(path).normalize();
111111

112112
if (!requestedPath.startsWith("/WEB-INF") && !requestedPath.startsWith("/META-INF")) {
113-
request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response); // $ MISSING: hasUnsafeUrlForward
113+
request.getServletContext().getRequestDispatcher(requestedPath.toString()).forward(request, response); // $ MISSING: hasUrlForward
114114
}
115115
}
116116

‎java/ql/test/query-tests/security/CWE-552/UnsafeUrlForwardTest.ql

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎java/ql/test/query-tests/security/CWE-552/UnsafeUrlForwardTest.expected renamed to ‎java/ql/test/query-tests/security/CWE-552/UrlForwardTest.expected

File renamed without changes.

‎java/ql/test/query-tests/security/CWE-552/UnsafeUrlForward.java renamed to ‎java/ql/test/query-tests/security/CWE-552/UrlForwardTest.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,35 +7,35 @@
77
import org.springframework.web.servlet.ModelAndView;
88

99
@Controller
10-
public class UnsafeUrlForward {
10+
public class UrlForwardTest {
1111

1212
@GetMapping("/bad1")
1313
public ModelAndView bad1(String url) {
14-
return new ModelAndView(url); // $ hasUnsafeUrlForward
14+
return new ModelAndView(url); // $ hasUrlForward
1515
}
1616

1717
@GetMapping("/bad2")
1818
public ModelAndView bad2(String url) {
1919
ModelAndView modelAndView = new ModelAndView();
20-
modelAndView.setViewName(url); // $ hasUnsafeUrlForward
20+
modelAndView.setViewName(url); // $ hasUrlForward
2121
return modelAndView;
2222
}
2323

2424
@GetMapping("/bad3")
2525
public String bad3(String url) {
26-
return "forward:" + url + "/swagger-ui/index.html"; // $ hasUnsafeUrlForward
26+
return "forward:" + url + "/swagger-ui/index.html"; // $ hasUrlForward
2727
}
2828

2929
@GetMapping("/bad4")
3030
public ModelAndView bad4(String url) {
31-
ModelAndView modelAndView = new ModelAndView("forward:" + url); // $ hasUnsafeUrlForward
31+
ModelAndView modelAndView = new ModelAndView("forward:" + url); // $ hasUrlForward
3232
return modelAndView;
3333
}
3434

3535
@GetMapping("/bad5")
3636
public void bad5(String url, HttpServletRequest request, HttpServletResponse response) {
3737
try {
38-
request.getRequestDispatcher(url).include(request, response); // $ hasUnsafeUrlForward
38+
request.getRequestDispatcher(url).include(request, response); // $ hasUrlForward
3939
} catch (ServletException e) {
4040
e.printStackTrace();
4141
} catch (IOException e) {
@@ -46,7 +46,7 @@ public void bad5(String url, HttpServletRequest request, HttpServletResponse res
4646
@GetMapping("/bad6")
4747
public void bad6(String url, HttpServletRequest request, HttpServletResponse response) {
4848
try {
49-
request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").include(request, response); // $ hasUnsafeUrlForward
49+
request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").include(request, response); // $ hasUrlForward
5050
} catch (ServletException e) {
5151
e.printStackTrace();
5252
} catch (IOException e) {
@@ -57,7 +57,7 @@ public void bad6(String url, HttpServletRequest request, HttpServletResponse res
5757
@GetMapping("/bad7")
5858
public void bad7(String url, HttpServletRequest request, HttpServletResponse response) {
5959
try {
60-
request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").forward(request, response); // $ hasUnsafeUrlForward
60+
request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").forward(request, response); // $ hasUrlForward
6161
} catch (ServletException e) {
6262
e.printStackTrace();
6363
} catch (IOException e) {

0 commit comments

Comments
 (0)