hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 08:45:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Java: update UrlPathBarrier to include FollowsBarrierPrefix

Browse Source
This commit is contained in:
Jami Cogswell
2024-03-12 12:21:22 -04:00
parent c5a59d6c51
commit e99cea340b
2 changed files with 44 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
java/ql/lib/semmle/code/java/security/UrlForward.qll
View File

@@ -41,8 +41,7 @@ abstract class UrlForwardBarrier extends DataFlow::Node { }
private class PrimitiveBarrier extends UrlForwardBarrier instanceof SimpleTypeSanitizer { }
/** A barrier for URLs appended to a prefix. */
private class FollowsBarrierPrefix extends UrlForwardBarrier {
private class FollowsBarrierPrefix extends DataFlow::Node {
FollowsBarrierPrefix() { this.asExpr() = any(BarrierPrefix fp).getAnAppendedExpression() }
}
@@ -55,14 +54,16 @@ private class BarrierPrefix extends InterestingPrefix {
override int getOffset() { result = 0 }
}
/** A barrier that protects against path injection vulnerabilities while accounting for URL encoding. */
/**
* A barrier that protects against path injection vulnerabilities while accounting
* for URL encoding and concatenated prefixes.
*/
private class UrlPathBarrier extends UrlForwardBarrier instanceof PathInjectionSanitizer {
UrlPathBarrier() {
this instanceof ExactPathMatchSanitizer
or
this instanceof NoUrlEncodingBarrier
or
this instanceof FullyDecodesUrlBarrier
this instanceof ExactPathMatchSanitizer or
this instanceof NoUrlEncodingBarrier or
this instanceof FullyDecodesUrlBarrier or
this instanceof FollowsBarrierPrefix
}
}

36
java/ql/test/query-tests/security/CWE-552/UrlForwardTest.java
View File

@@ -85,7 +85,41 @@ public class UrlForwardTest extends HttpServlet implements Filter {
@GetMapping("/good1")
public void good1(String url, HttpServletRequest request, HttpServletResponse response) {
try {
request.getRequestDispatcher("/index.jsp?token=" + url).forward(request, response);
request.getRequestDispatcher("/index.jsp?token=" + url).forward(request, response); // $ SPURIOUS: hasUrlForward
} catch (ServletException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
}
// BAD: appended to a prefix without path sanitization
@GetMapping("/bad8")
public void bad8(String urlPath, HttpServletRequest request, HttpServletResponse response) {
try {
String url = "/pages" + urlPath;
request.getRequestDispatcher(url).forward(request, response); // $ hasUrlForward
} catch (ServletException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
}
// GOOD: appended to a prefix with path sanitization
@GetMapping("/good2")
public void good2(String urlPath, HttpServletRequest request, HttpServletResponse response) {
try {
while (urlPath.contains("%")) {
urlPath = URLDecoder.decode(urlPath, "UTF-8");
}
if (!urlPath.contains("..") && !urlPath.startsWith("/WEB-INF")) {
// Note: path injection sanitizer does not account for string concatenation instead of a `startswith` check
String url = "/pages" + urlPath;
request.getRequestDispatcher(url).forward(request, response);
}
} catch (ServletException e) {
e.printStackTrace();
} catch (IOException e) {