Further refactoring

Avoid having two taint tracking configurations in the same file

Author: atorralba

java/ql/lib/semmle/code/java/security/Xxe.qll

@@ -0,0 +1,24 @@
+/** Provides classes to reason about XML eXternal Entity (XXE) vulnerabilities. */
+
+import java
+import semmle.code.java.security.XmlParsers
+
+/** A node where insecure XML parsing takes place. */
+abstract class XxeSink extends DataFlow::Node { }
+
+/** A node that acts as a sanitizer in configurations realted to XXE vulnerabilities. */
+abstract class XxeSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to flows related to
+ * XXE vulnerabilities.
+ */
+class XxeAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for flows related to XXE vulnerabilities.
+   */
+  abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
+}

java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll

@@ -0,0 +1,23 @@
+/** Provides taint tracking configurations to be used in local XXE queries. */
+
+import java
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.security.XxeQuery
+
+/**
+ * A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion.
+ */
+class XxeLocalConfig extends TaintTracking::Configuration {
+  XxeLocalConfig() { this = "XxeLocalConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    any(XxeAdditionalTaintStep s).step(n1, n2)
+  }
+}

java/ql/lib/semmle/code/java/security/XxeQuery.qll

@@ -1,38 +1,18 @@
-/** Provides taint tracking configurations to be used in XXE queries. */
+/** Provides default definitions to be used in XXE queries. */
 
 import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.TaintTracking2
-import semmle.code.java.security.XmlParsers
+private import semmle.code.java.dataflow.TaintTracking2
+import semmle.code.java.security.Xxe
 
 /**
- * A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion.
+ * The default implementation of a XXE sink.
+ * The argument of a parse call on an insecurely configured XML parser.
  */
-class XxeConfig extends TaintTracking::Configuration {
-  XxeConfig() { this = "XxeConfig" }
-
-  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
-}
-
-/**
- * A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion.
- */
-class XxeLocalConfig extends TaintTracking::Configuration {
-  XxeLocalConfig() { this = "XxeLocalConfig" }
-
-  override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
-}
-
-private class UnsafeXxeSink extends DataFlow::ExprNode {
-  UnsafeXxeSink() {
+private class DefaultXxeSink extends XxeSink {
+  DefaultXxeSink() {
     not exists(SafeSaxSourceFlowConfig safeSource | safeSource.hasFlowTo(this)) and
     exists(XmlParserCall parse |
-      parse.getSink() = this.getExpr() and
+      parse.getSink() = this.asExpr() and
       not parse.isSafe()
     )
   }
@@ -42,7 +22,7 @@ private class UnsafeXxeSink extends DataFlow::ExprNode {
  * A taint-tracking configuration for safe XML readers used to parse XML documents.
  */
 private class SafeSaxSourceFlowConfig extends TaintTracking2::Configuration {
-  SafeSaxSourceFlowConfig() { this = "XmlParsers::SafeSAXSourceFlowConfig" }
+  SafeSaxSourceFlowConfig() { this = "SafeSaxSourceFlowConfig" }
 
   override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource }
 

java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll

@@ -0,0 +1,23 @@
+/** Provides taint tracking configurations to be used in remote XXE queries. */
+
+import java
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.security.XxeQuery
+
+/**
+ * A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion.
+ */
+class XxeConfig extends TaintTracking::Configuration {
+  XxeConfig() { this = "XxeConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    any(XxeAdditionalTaintStep s).step(n1, n2)
+  }
+}

java/ql/src/Security/CWE/CWE-611/XXE.ql

@@ -14,7 +14,8 @@
  */
 
 import java
-import semmle.code.java.security.XxeQuery
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.security.XxeRemoteQuery
 import DataFlow::PathGraph
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf

java/ql/src/Security/CWE/CWE-611/XXELocal.ql

@@ -14,7 +14,8 @@
  */
 
 import java
-import semmle.code.java.security.XxeQuery
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.security.XxeLocalQuery
 import DataFlow::PathGraph
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, XxeLocalConfig conf