diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql index c160895f472..add36e91963 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql @@ -11,7 +11,8 @@ */ import java -import ResponseSplitting +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.security.ResponseSplitting import DataFlow::PathGraph class ResponseSplittingConfig extends TaintTracking::Configuration { @@ -19,7 +20,7 @@ class ResponseSplittingConfig extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and - not source instanceof WhitelistedSource + not source instanceof SafeHeaderSplittingSource } override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink } diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll deleted file mode 100644 index f17ac91fa97..00000000000 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll +++ /dev/null @@ -1,38 +0,0 @@ -import java -import semmle.code.java.frameworks.Servlets -import semmle.code.java.dataflow.FlowSources - -/** - * Header-splitting sinks. Expressions that end up in an HTTP header. - */ -class HeaderSplittingSink extends DataFlow::ExprNode { - HeaderSplittingSink() { - exists(ResponseAddCookieMethod m, MethodAccess ma | - ma.getMethod() = m and - this.getExpr() = ma.getArgument(0) - ) - or - exists(ResponseAddHeaderMethod m, MethodAccess ma | - ma.getMethod() = m and - this.getExpr() = ma.getAnArgument() - ) - or - exists(ResponseSetHeaderMethod m, MethodAccess ma | - ma.getMethod() = m and - this.getExpr() = ma.getAnArgument() - ) - or - exists(JaxRsResponseBuilder builder, Method m | - m = builder.getAMethod() and m.getName() = "header" - | - this.getExpr() = m.getAReference().getArgument(1) - ) - } -} - -class WhitelistedSource extends DataFlow::ExprNode { - WhitelistedSource() { - this.asExpr().(MethodAccess).getMethod() instanceof HttpServletRequestGetHeaderMethod or - this.asExpr().(MethodAccess).getMethod() instanceof CookieGetNameMethod - } -} diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql index 3de3a2229ca..7a748276aba 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql @@ -12,7 +12,7 @@ import java import semmle.code.java.dataflow.FlowSources -import ResponseSplitting +import semmle.code.java.security.ResponseSplitting import DataFlow::PathGraph class ResponseSplittingLocalConfig extends TaintTracking::Configuration { diff --git a/java/ql/src/semmle/code/java/security/ResponseSplitting.qll b/java/ql/src/semmle/code/java/security/ResponseSplitting.qll new file mode 100644 index 00000000000..d09e6567b15 --- /dev/null +++ b/java/ql/src/semmle/code/java/security/ResponseSplitting.qll @@ -0,0 +1,49 @@ +/** Provides classes to reason about header splitting attacks. */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.frameworks.Servlets +import semmle.code.java.frameworks.JaxWS + +/** A sink that is vulnerable to an HTTP header splitting attack. */ +abstract class HeaderSplittingSink extends DataFlow::Node { } + +/** A source that introduces data considered safe to use by a header splitting source. */ +abstract class SafeHeaderSplittingSource extends DataFlow::Node { + SafeHeaderSplittingSource() { this instanceof RemoteFlowSource } +} + +/** A sink that identifies a Java Servlet or JaxWs method that is vulnerable to an HTTP header splitting attack. */ +private class ServletHeaderSplittingSink extends HeaderSplittingSink { + ServletHeaderSplittingSink() { + exists(ResponseAddCookieMethod m, MethodAccess ma | + ma.getMethod() = m and + this.asExpr() = ma.getArgument(0) + ) + or + exists(ResponseAddHeaderMethod m, MethodAccess ma | + ma.getMethod() = m and + this.asExpr() = ma.getAnArgument() + ) + or + exists(ResponseSetHeaderMethod m, MethodAccess ma | + ma.getMethod() = m and + this.asExpr() = ma.getAnArgument() + ) + or + exists(JaxRsResponseBuilder builder, Method m | + m = builder.getAMethod() and m.getName() = "header" + | + this.asExpr() = m.getAReference().getArgument(1) + ) + } +} + +/** A default source that introduces data considered safe to use by a header splitting source. */ +private class DefaultSafeHeaderSplittingSource extends SafeHeaderSplittingSource { + DefaultSafeHeaderSplittingSource() { + this.asExpr().(MethodAccess).getMethod() instanceof HttpServletRequestGetHeaderMethod or + this.asExpr().(MethodAccess).getMethod() instanceof CookieGetNameMethod + } +}