Refactor XXE files

Author: egregius313

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

@@ -14,18 +14,19 @@
 
 import java
 import XXELib
+import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import DataFlow::PathGraph
+import XxeFlow::PathGraph
 
-class XxeConfig extends TaintTracking::Configuration {
-  XxeConfig() { this = "XxeConfig" }
+module XxeConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
-where conf.hasFlowPath(source, sink)
+module XxeFlow = TaintTracking::Global<XxeConfig>;
+
+from XxeFlow::PathNode source, XxeFlow::PathNode sink
+where XxeFlow::flowPath(source, sink)
 select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
   "user input"

java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll

@@ -52,9 +52,7 @@ class ValidatorValidate extends XmlParserCall {
 
   override Expr getSink() { result = this.getArgument(0) }
 
-  override predicate isSafe() {
-    exists(SafeValidatorFlowConfig svfc | svfc.hasFlowToExpr(this.getQualifier()))
-  }
+  override predicate isSafe() { SafeValidatorFlow::flowToExpr(this.getQualifier()) }
 }
 
 /** A `ParserConfig` specific to `Validator`. */
@@ -82,21 +80,21 @@ class SafeValidator extends VarAccess {
   }
 }
 
-private class SafeValidatorFlowConfig extends DataFlow3::Configuration {
-  SafeValidatorFlowConfig() { this = "SafeValidatorFlowConfig" }
-
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeValidator }
+private module SafeValidatorFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeValidator }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |
       sink.asExpr() = ma.getQualifier() and
       ma.getMethod().getDeclaringType() instanceof Validator
     )
   }
 
-  override int fieldFlowBranchLimit() { result = 0 }
+  int fieldFlowBranchLimit() { result = 0 }
 }
 
+private module SafeValidatorFlow = DataFlow::Global<SafeValidatorFlowConfig>;
+
 /**
  * The classes `org.apache.commons.digester3.Digester`, `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`.
  */
@@ -121,9 +119,7 @@ class DigesterParse extends XmlParserCall {
 
   override Expr getSink() { result = this.getArgument(0) }
 
-  override predicate isSafe() {
-    exists(SafeDigesterFlowConfig sdfc | sdfc.hasFlowToExpr(this.getQualifier()))
-  }
+  override predicate isSafe() { SafeDigesterFlow::flowToExpr(this.getQualifier()) }
 }
 
 /** A `ParserConfig` that is specific to `Digester`. */
@@ -170,20 +166,20 @@ class SafeDigester extends VarAccess {
   }
 }
 
-private class SafeDigesterFlowConfig extends DataFlow4::Configuration {
-  SafeDigesterFlowConfig() { this = "SafeDigesterFlowConfig" }
-
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDigester }
+private module SafeDigesterFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDigester }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |
       sink.asExpr() = ma.getQualifier() and ma.getMethod().getDeclaringType() instanceof Digester
     )
   }
 
-  override int fieldFlowBranchLimit() { result = 0 }
+  int fieldFlowBranchLimit() { result = 0 }
 }
 
+private module SafeDigesterFlow = DataFlow::Global<SafeDigesterFlowConfig>;
+
 /** The class `java.beans.XMLDecoder`. */
 class XmlDecoder extends RefType {
   XmlDecoder() { this.hasQualifiedName("java.beans", "XMLDecoder") }

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

@@ -16,18 +16,19 @@
 
 import java
 import XXELib
+import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import DataFlow::PathGraph
+import XxeLocalFlow::PathGraph
 
-class XxeLocalConfig extends TaintTracking::Configuration {
-  XxeLocalConfig() { this = "XxeLocalConfig" }
+module XxeLocalConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
 
-  override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, XxeLocalConfig conf
-where conf.hasFlowPath(source, sink)
+module XxeLocalFlow = TaintTracking::Global<XxeLocalConfig>;
+
+from XxeLocalFlow::PathNode source, XxeLocalFlow::PathNode sink
+where XxeLocalFlow::flowPath(source, sink)
 select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
   "user input"