From 5f560e04659caa3c62612302f5a5886f75d2bf02 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Wed, 8 Jul 2020 17:17:24 +0200 Subject: [PATCH] Extract `HeaderSplittingSink` and `WhitelistedSource` - Extract `HeaderSplittingSink` and `WhitelistedSource` into an importable library. - Rename the existing `HeaderSplittingSink` implementation to `ServletHeaderSplittingSink`. --- java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql | 4 ++-- .../Security/CWE/CWE-113/ResponseSplittingLocal.ql | 2 +- ...onseSplitting.qll => ServletResponseSplitting.qll} | 9 +++++---- .../semmle/code/java/security/ResponseSplitting.qll | 11 +++++++++++ 4 files changed, 19 insertions(+), 7 deletions(-) rename java/ql/src/Security/CWE/CWE-113/{ResponseSplitting.qll => ServletResponseSplitting.qll} (80%) create mode 100644 java/ql/src/semmle/code/java/security/ResponseSplitting.qll diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql index c160895f472..9b26aff05be 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql @@ -11,7 +11,7 @@ */ import java -import ResponseSplitting +import ServletResponseSplitting import DataFlow::PathGraph class ResponseSplittingConfig extends TaintTracking::Configuration { @@ -19,7 +19,7 @@ class ResponseSplittingConfig extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and - not source instanceof WhitelistedSource + not source instanceof TrustedSource } override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink } diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql index 3de3a2229ca..dfeeddbd3f4 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 ServletResponseSplitting import DataFlow::PathGraph class ResponseSplittingLocalConfig extends TaintTracking::Configuration { diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll b/java/ql/src/Security/CWE/CWE-113/ServletResponseSplitting.qll similarity index 80% rename from java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll rename to java/ql/src/Security/CWE/CWE-113/ServletResponseSplitting.qll index f17ac91fa97..39c8ff6266e 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll +++ b/java/ql/src/Security/CWE/CWE-113/ServletResponseSplitting.qll @@ -1,12 +1,13 @@ import java import semmle.code.java.frameworks.Servlets import semmle.code.java.dataflow.FlowSources +import semmle.code.java.security.ResponseSplitting /** * Header-splitting sinks. Expressions that end up in an HTTP header. */ -class HeaderSplittingSink extends DataFlow::ExprNode { - HeaderSplittingSink() { +class ServletHeaderSplittingSink extends HeaderSplittingSink { + ServletHeaderSplittingSink() { exists(ResponseAddCookieMethod m, MethodAccess ma | ma.getMethod() = m and this.getExpr() = ma.getArgument(0) @@ -30,8 +31,8 @@ class HeaderSplittingSink extends DataFlow::ExprNode { } } -class WhitelistedSource extends DataFlow::ExprNode { - WhitelistedSource() { +class TrustedServletSource extends TrustedSource { + TrustedServletSource() { this.asExpr().(MethodAccess).getMethod() instanceof HttpServletRequestGetHeaderMethod or this.asExpr().(MethodAccess).getMethod() instanceof CookieGetNameMethod } 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..583833df01e --- /dev/null +++ b/java/ql/src/semmle/code/java/security/ResponseSplitting.qll @@ -0,0 +1,11 @@ +import semmle.code.java.dataflow.DataFlow + +/** + * Header-splitting sinks. Expressions that end up in an HTTP header. + */ +abstract class HeaderSplittingSink extends DataFlow::ExprNode { } + +/** + * Sources that cannot be used to perform a header splitting attack. + */ +abstract class TrustedSource extends DataFlow::ExprNode { }