From 271d50ba996044cd1aafa8d7da9fb718e4bdcc93 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Wed, 15 Mar 2023 14:35:36 -0400
Subject: [PATCH] Refactor Security.CWE.CWE-611 Xxe queries

---
 .../semmle/code/java/security/XxeLocalQuery.qll  | 16 +++++++++++++++-
 .../semmle/code/java/security/XxeRemoteQuery.qll | 16 +++++++++++++++-
 java/ql/src/Security/CWE/CWE-611/XXE.ql          |  6 +++---
 java/ql/src/Security/CWE/CWE-611/XXELocal.ql     |  6 +++---
 4 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll b/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll
index 47d30793f7a..3737afb7797 100644
--- a/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll
@@ -8,7 +8,7 @@ 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 {
+deprecated class XxeLocalConfig extends TaintTracking::Configuration {
   XxeLocalConfig() { this = "XxeLocalConfig" }
 
   override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
@@ -21,3 +21,17 @@ class XxeLocalConfig extends TaintTracking::Configuration {
     any(XxeAdditionalTaintStep s).step(n1, n2)
   }
 }
+
+private module XxeLocalConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink }
+
+  predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer }
+
+  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
+    any(XxeAdditionalTaintStep s).step(n1, n2)
+  }
+}
+
+module XxeLocalFlow = TaintTracking::Make<XxeLocalConfig>;
diff --git a/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll b/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll
index f6ace15eba6..e8a40d5426b 100644
--- a/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll
@@ -8,7 +8,7 @@ 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 {
+deprecated class XxeConfig extends TaintTracking::Configuration {
   XxeConfig() { this = "XxeConfig" }
 
   override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
@@ -21,3 +21,17 @@ class XxeConfig extends TaintTracking::Configuration {
     any(XxeAdditionalTaintStep s).step(n1, n2)
   }
 }
+
+private module XxeConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink }
+
+  predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer }
+
+  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
+    any(XxeAdditionalTaintStep s).step(n1, n2)
+  }
+}
+
+module XxeFlow = TaintTracking::Make<XxeConfig>;
diff --git a/java/ql/src/Security/CWE/CWE-611/XXE.ql b/java/ql/src/Security/CWE/CWE-611/XXE.ql
index af83d7aa35a..708d4f08ee7 100644
--- a/java/ql/src/Security/CWE/CWE-611/XXE.ql
+++ b/java/ql/src/Security/CWE/CWE-611/XXE.ql
@@ -16,10 +16,10 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.security.XxeRemoteQuery
-import DataFlow::PathGraph
+import XxeFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
-where conf.hasFlowPath(source, sink)
+from XxeFlow::PathNode source, XxeFlow::PathNode sink
+where XxeFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
   "XML parsing depends on a $@ without guarding against external entity expansion.",
   source.getNode(), "user-provided value"
diff --git a/java/ql/src/Security/CWE/CWE-611/XXELocal.ql b/java/ql/src/Security/CWE/CWE-611/XXELocal.ql
index 6d142921a1d..0ab4ddcc106 100644
--- a/java/ql/src/Security/CWE/CWE-611/XXELocal.ql
+++ b/java/ql/src/Security/CWE/CWE-611/XXELocal.ql
@@ -16,10 +16,10 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.security.XxeLocalQuery
-import DataFlow::PathGraph
+import XxeLocalFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, XxeLocalConfig conf
-where conf.hasFlowPath(source, sink)
+from XxeLocalFlow::PathNode source, XxeLocalFlow::PathNode sink
+where XxeLocalFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
   "XML parsing depends on a $@ without guarding against external entity expansion.",
   source.getNode(), "user-provided value"