diff --git a/java/ql/lib/ext/jakarta.servlet.model.yml b/java/ql/lib/ext/jakarta.servlet.model.yml new file mode 100644 index 00000000000..fc1274cadaf --- /dev/null +++ b/java/ql/lib/ext/jakarta.servlet.model.yml @@ -0,0 +1,8 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: sinkModel + data: + # TODO: potentially switch to using Argument[this] of `RequestDispatcher.forward|include` as sink instead of the below. + - ["jakarta.servlet", "ServletContext", True, "getRequestDispatcher", "(String)", "", "Argument[0]", "url-forward", "manual"] + - ["jakarta.servlet", "ServletRequest", True, "getRequestDispatcher", "(String)", "", "Argument[0]", "url-forward", "manual"] diff --git a/java/ql/lib/ext/javax.portlet.model.yml b/java/ql/lib/ext/javax.portlet.model.yml new file mode 100644 index 00000000000..e39484abcb7 --- /dev/null +++ b/java/ql/lib/ext/javax.portlet.model.yml @@ -0,0 +1,7 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: sinkModel + data: + # TODO: potentially switch to using Argument[this] of `PortletRequestDispatcher.forward|include` as sink instead of the below. + - ["javax.portlet", "PortletContext", True, "getRequestDispatcher", "(String)", "", "Argument[0]", "url-forward", "manual"] diff --git a/java/ql/lib/ext/javax.servlet.model.yml b/java/ql/lib/ext/javax.servlet.model.yml index 581863c74f7..7c405ac0de9 100644 --- a/java/ql/lib/ext/javax.servlet.model.yml +++ b/java/ql/lib/ext/javax.servlet.model.yml @@ -14,6 +14,9 @@ extensions: extensible: sinkModel data: - ["javax.servlet", "ServletContext", True, "getResourceAsStream", "(String)", "", "Argument[0]", "path-injection", "ai-manual"] + # TODO: potentially switch to using Argument[this] of `RequestDispatcher.forward|include` as sink instead of the below. + - ["javax.servlet", "ServletContext", True, "getRequestDispatcher", "(String)", "", "Argument[0]", "url-forward", "manual"] + - ["javax.servlet", "ServletRequest", True, "getRequestDispatcher", "(String)", "", "Argument[0]", "url-forward", "manual"] - addsTo: pack: codeql/java-all extensible: summaryModel diff --git a/java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll b/java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll index 628397d07ef..e7780ee971b 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll @@ -10,14 +10,9 @@ abstract class UnsafeUrlForwardSink extends DataFlow::Node { } /** A sanitizer for unsafe URL forward vulnerabilities. */ abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { } -/** An argument to `getRequestDispatcher`. */ -private class RequestDispatcherSink extends UnsafeUrlForwardSink { - RequestDispatcherSink() { - exists(MethodCall ma | - ma.getMethod() instanceof GetRequestDispatcherMethod and - ma.getArgument(0) = this.asExpr() - ) - } +/** A default sink representing methods susceptible to unsafe URL forwarding. */ +private class DefaultUnsafeUrlForwardSink extends UnsafeUrlForwardSink { + DefaultUnsafeUrlForwardSink() { sinkNode(this, "url-forward") } } // TODO: look into `StaplerResponse.forward`, etc., and think about re-adding the MaD "request-forgery" sinks as a result