mirror of
https://github.com/github/codeql.git
synced 2026-04-23 07:45:17 +02:00
add some request-forgery sanitizers, inspired from C#
This commit is contained in:
@@ -8,6 +8,7 @@ import semmle.code.java.frameworks.JaxWS
|
||||
import semmle.code.java.frameworks.javase.Http
|
||||
import semmle.code.java.dataflow.DataFlow
|
||||
import semmle.code.java.frameworks.Properties
|
||||
private import semmle.code.java.controlflow.Guards
|
||||
private import semmle.code.java.dataflow.StringPrefixes
|
||||
private import semmle.code.java.dataflow.ExternalFlow
|
||||
private import semmle.code.java.security.Sanitizers
|
||||
@@ -83,3 +84,79 @@ private class HostnameSanitizingPrefix extends InterestingPrefix {
|
||||
private class HostnameSantizer extends RequestForgerySanitizer {
|
||||
HostnameSantizer() { this.asExpr() = any(HostnameSanitizingPrefix hsp).getAnAppendedExpression() }
|
||||
}
|
||||
|
||||
/**
|
||||
* An argument to a call to `List.contains()` that is a sanitizer for URL redirects.
|
||||
*/
|
||||
private predicate isContainsUrlSanitizer(Guard guard, Expr e, boolean branch) {
|
||||
guard =
|
||||
any(MethodCall method |
|
||||
method.getMethod().getName() = "contains" and
|
||||
e = method.getArgument(0) and
|
||||
branch = true
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* An URL argument to a call to `.contains()` that is a sanitizer for URL redirects.
|
||||
*
|
||||
* This `contains` method is usually called on a list, but the sanitizer matches any call to a method
|
||||
* called `contains`, so other methods with the same name will also be considered sanitizers.
|
||||
*/
|
||||
class ContainsUrlSanitizer extends RequestForgerySanitizer {
|
||||
ContainsUrlSanitizer() {
|
||||
this = DataFlow::BarrierGuard<isContainsUrlSanitizer/3>::getABarrierNode()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A check that the URL is relative, and therefore safe for URL redirects.
|
||||
*/
|
||||
private predicate isRelativeUrlSanitizer(Guard guard, Expr e, boolean branch) {
|
||||
guard =
|
||||
any(MethodCall call |
|
||||
exists(Method method |
|
||||
call.getMethod() = method and
|
||||
method.getName() = "isAbsolute" and
|
||||
method.getDeclaringType().hasQualifiedName("java.net", "URI")
|
||||
) and
|
||||
e = call.getQualifier() and
|
||||
branch = false
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A check that the URL is relative, and therefore safe for URL redirects.
|
||||
*/
|
||||
class RelativeUrlSanitizer extends RequestForgerySanitizer {
|
||||
RelativeUrlSanitizer() {
|
||||
this = DataFlow::BarrierGuard<isRelativeUrlSanitizer/3>::getABarrierNode()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A comparison on the host of a url, that is a sanitizer for URL redirects.
|
||||
* E.g. `"example.org".equals(url.getHost())"`
|
||||
*/
|
||||
private predicate isHostComparisonSanitizer(Guard guard, Expr e, boolean branch) {
|
||||
guard =
|
||||
any(MethodCall equalsCall |
|
||||
equalsCall.getMethod().getName() = "equals" and
|
||||
branch = true and
|
||||
exists(MethodCall hostCall |
|
||||
hostCall = [equalsCall.getQualifier(), equalsCall.getArgument(0)] and
|
||||
hostCall.getMethod().getName() = "getHost" and
|
||||
hostCall.getMethod().getDeclaringType().hasQualifiedName("java.net", "URI") and
|
||||
e = hostCall.getQualifier()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A comparison on the `Host` property of a url, that is a sanitizer for URL redirects.
|
||||
*/
|
||||
class HostComparisonSanitizer extends RequestForgerySanitizer {
|
||||
HostComparisonSanitizer() {
|
||||
this = DataFlow::BarrierGuard<isHostComparisonSanitizer/3>::getABarrierNode()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ edges
|
||||
| mad/Test.java:9:16:9:41 | getParameter(...) : String | mad/Test.java:14:31:14:38 | source(...) : String | provenance | |
|
||||
| mad/Test.java:14:31:14:38 | source(...) : String | mad/Test.java:14:22:14:38 | (...)... | provenance | |
|
||||
nodes
|
||||
| UrlRedirect2.java:27:25:27:54 | getParameter(...) | semmle.label | getParameter(...) |
|
||||
| UrlRedirect.java:23:25:23:54 | getParameter(...) | semmle.label | getParameter(...) |
|
||||
| UrlRedirect.java:32:25:32:67 | weakCleanup(...) | semmle.label | weakCleanup(...) |
|
||||
| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | semmle.label | getParameter(...) : String |
|
||||
@@ -20,6 +21,7 @@ nodes
|
||||
subpaths
|
||||
| UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:45:28:45:39 | input : String | UrlRedirect.java:46:10:46:40 | replaceAll(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) |
|
||||
#select
|
||||
| UrlRedirect2.java:27:25:27:54 | getParameter(...) | UrlRedirect2.java:27:25:27:54 | getParameter(...) | UrlRedirect2.java:27:25:27:54 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect2.java:27:25:27:54 | getParameter(...) | user-provided value |
|
||||
| UrlRedirect.java:23:25:23:54 | getParameter(...) | UrlRedirect.java:23:25:23:54 | getParameter(...) | UrlRedirect.java:23:25:23:54 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:23:25:23:54 | getParameter(...) | user-provided value |
|
||||
| UrlRedirect.java:32:25:32:67 | weakCleanup(...) | UrlRedirect.java:32:37:32:66 | getParameter(...) : String | UrlRedirect.java:32:25:32:67 | weakCleanup(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:32:37:32:66 | getParameter(...) | user-provided value |
|
||||
| UrlRedirect.java:39:34:39:63 | getParameter(...) | UrlRedirect.java:39:34:39:63 | getParameter(...) | UrlRedirect.java:39:34:39:63 | getParameter(...) | Untrusted URL redirection depends on a $@. | UrlRedirect.java:39:34:39:63 | getParameter(...) | user-provided value |
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
// Test case for
|
||||
// CWE-601: URL Redirection to Untrusted Site ('Open Redirect')
|
||||
// http://cwe.mitre.org/data/definitions/601.html
|
||||
|
||||
package test.cwe601.cwe.examples;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.net.URI;
|
||||
import java.net.URISyntaxException;
|
||||
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
public class UrlRedirect2 extends HttpServlet {
|
||||
private static final List<String> VALID_REDIRECTS = Arrays.asList(
|
||||
"http://cwe.mitre.org/data/definitions/601.html",
|
||||
"http://cwe.mitre.org/data/definitions/79.html"
|
||||
);
|
||||
|
||||
protected void doGet(HttpServletRequest request, HttpServletResponse response)
|
||||
throws ServletException, IOException {
|
||||
// BAD: a request parameter is incorporated without validation into a URL redirect
|
||||
response.sendRedirect(request.getParameter("target"));
|
||||
|
||||
// GOOD: the request parameter is validated against a known list of strings
|
||||
String target = request.getParameter("target");
|
||||
if (VALID_REDIRECTS.contains(target)) {
|
||||
response.sendRedirect(target);
|
||||
}
|
||||
|
||||
try {
|
||||
String urlString = request.getParameter("page");
|
||||
URI url = new URI(urlString);
|
||||
|
||||
if (!url.isAbsolute()) {
|
||||
// GOOD: The redirect is to a relative URL
|
||||
response.sendRedirect(url.toString());
|
||||
}
|
||||
|
||||
if ("example.org".equals(url.getHost())) {
|
||||
// GOOD: The redirect is to a known host
|
||||
response.sendRedirect(url.toString());
|
||||
}
|
||||
} catch (URISyntaxException e) {
|
||||
// handle exception
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user