Merge pull request github#12541 from egregius313/egregius313/refactor-queries-to-new-dataflow-api

Java: Refactor more queries to the new DataFlow module API

Author: egregius313

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

@@ -6,9 +6,11 @@ private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.security.XxeQuery
 
 /**
+ * DEPRECATED: Use `XxeLocalFlow` instead.
+ *
  * 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 +23,23 @@ class XxeLocalConfig extends TaintTracking::Configuration {
     any(XxeAdditionalTaintStep s).step(n1, n2)
   }
 }
+
+/**
+ * A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion.
+ */
+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)
+  }
+}
+
+/**
+ * Detect taint flow of unvalidated local user input that is used in XML external entity expansion.
+ */
+module XxeLocalFlow = TaintTracking::Make<XxeLocalConfig>;

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

@@ -6,9 +6,11 @@ private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.security.XxeQuery
 
 /**
+ * DEPRECATED: Use `XxeFlow` instead.
+ *
  * 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 +23,23 @@ class XxeConfig extends TaintTracking::Configuration {
     any(XxeAdditionalTaintStep s).step(n1, n2)
   }
 }
+
+/**
+ * A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion.
+ */
+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)
+  }
+}
+
+/**
+ * Detect taint flow of unvalidated remote user input that is used in XML external entity expansion.
+ */
+module XxeFlow = TaintTracking::Make<XxeConfig>;

java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql

@@ -18,32 +18,33 @@ import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.security.PathCreation
 import semmle.code.java.security.PathSanitizer
-import DataFlow::PathGraph
 import TaintedPathCommon
 
-class TaintedPathLocalConfig extends TaintTracking::Configuration {
-  TaintedPathLocalConfig() { this = "TaintedPathLocalConfig" }
+module TaintedPathLocalConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     sink.asExpr() = any(PathCreation p).getAnInput()
     or
     sinkNode(sink, "create-file")
   }
 
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
+  predicate isBarrier(DataFlow::Node sanitizer) {
     sanitizer.getType() instanceof BoxedType or
     sanitizer.getType() instanceof PrimitiveType or
     sanitizer.getType() instanceof NumberType or
     sanitizer instanceof PathInjectionSanitizer
   }
 
-  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
     any(TaintedPathAdditionalTaintStep s).step(n1, n2)
   }
 }
 
+module TaintedPathLocalFlow = TaintTracking::Make<TaintedPathLocalConfig>;
+
+import TaintedPathLocalFlow::PathGraph
+
 /**
  * Gets the data-flow node at which to report a path ending at `sink`.
  *
@@ -52,13 +53,13 @@ class TaintedPathLocalConfig extends TaintTracking::Configuration {
  * continue to report there; otherwise we report directly at `sink`.
  */
 DataFlow::Node getReportingNode(DataFlow::Node sink) {
-  any(TaintedPathLocalConfig c).hasFlowTo(sink) and
+  TaintedPathLocalFlow::hasFlowTo(sink) and
   if exists(PathCreation pc | pc.getAnInput() = sink.asExpr())
   then result.asExpr() = any(PathCreation pc | pc.getAnInput() = sink.asExpr())
   else result = sink
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, TaintedPathLocalConfig conf
-where conf.hasFlowPath(source, sink)
+from TaintedPathLocalFlow::PathNode source, TaintedPathLocalFlow::PathNode sink
+where TaintedPathLocalFlow::hasFlowPath(source, sink)
 select getReportingNode(sink.getNode()), source, sink, "This path depends on a $@.",
   source.getNode(), "user-provided value"

java/ql/src/Security/CWE/CWE-022/ZipSlip.ql

@@ -17,8 +17,6 @@ import semmle.code.java.controlflow.Guards
 import semmle.code.java.dataflow.SSA
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.security.PathSanitizer
-import DataFlow
-import PathGraph
 private import semmle.code.java.dataflow.ExternalFlow
 
 /**
@@ -36,27 +34,29 @@ class ArchiveEntryNameMethod extends Method {
   }
 }
 
-class ZipSlipConfiguration extends TaintTracking::Configuration {
-  ZipSlipConfiguration() { this = "ZipSlip" }
-
-  override predicate isSource(Node source) {
+module ZipSlipConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     source.asExpr().(MethodAccess).getMethod() instanceof ArchiveEntryNameMethod
   }
 
-  override predicate isSink(Node sink) { sink instanceof FileCreationSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink }
 
-  override predicate isSanitizer(Node node) { node instanceof PathInjectionSanitizer }
+  predicate isBarrier(DataFlow::Node node) { node instanceof PathInjectionSanitizer }
 }
 
+module ZipSlipFlow = TaintTracking::Make<ZipSlipConfig>;
+
+import ZipSlipFlow::PathGraph
+
 /**
  * A sink that represents a file creation, such as a file write, copy or move operation.
  */
 private class FileCreationSink extends DataFlow::Node {
   FileCreationSink() { sinkNode(this, "create-file") }
 }
 
-from PathNode source, PathNode sink
-where any(ZipSlipConfiguration c).hasFlowPath(source, sink)
+from ZipSlipFlow::PathNode source, ZipSlipFlow::PathNode sink
+where ZipSlipFlow::hasFlowPath(source, sink)
 select source.getNode(), source, sink,
   "Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(),
   "file system operation"

java/ql/src/Security/CWE/CWE-090/LdapInjection.ql

@@ -14,9 +14,9 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import LdapInjectionLib
-import DataFlow::PathGraph
+import LdapInjectionFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, LdapInjectionFlowConfig conf
-where conf.hasFlowPath(source, sink)
+from LdapInjectionFlow::PathNode source, LdapInjectionFlow::PathNode sink
+where LdapInjectionFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "This LDAP query depends on a $@.", source.getNode(),
   "user-provided value"

java/ql/src/Security/CWE/CWE-090/LdapInjectionLib.qll

@@ -1,21 +1,20 @@
 import java
 import semmle.code.java.dataflow.FlowSources
-import DataFlow
 import semmle.code.java.security.LdapInjection
 
 /**
  * A taint-tracking configuration for unvalidated user input that is used to construct LDAP queries.
  */
-class LdapInjectionFlowConfig extends TaintTracking::Configuration {
-  LdapInjectionFlowConfig() { this = "LdapInjectionFlowConfig" }
+module LdapInjectionFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+  predicate isSink(DataFlow::Node sink) { sink instanceof LdapInjectionSink }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof LdapInjectionSink }
+  predicate isBarrier(DataFlow::Node node) { node instanceof LdapInjectionSanitizer }
 
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof LdapInjectionSanitizer }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
     any(LdapInjectionAdditionalTaintStep a).step(pred, succ)
   }
 }
+
+module LdapInjectionFlow = TaintTracking::Make<LdapInjectionFlowConfig>;

java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql

@@ -13,7 +13,6 @@
 import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import DataFlow::PathGraph
 private import semmle.code.java.dataflow.ExternalFlow
 
 /**
@@ -56,14 +55,16 @@ class SetMessageInterpolatorCall extends MethodAccess {
  * Taint tracking BeanValidationConfiguration describing the flow of data from user input
  * to the argument of a method that builds constraint error messages.
  */
-class BeanValidationConfig extends TaintTracking::Configuration {
-  BeanValidationConfig() { this = "BeanValidationConfig" }
+module BeanValidationConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof BeanValidationSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof BeanValidationSink }
 }
 
+module BeanValidationFlow = TaintTracking::Make<BeanValidationConfig>;
+
+import BeanValidationFlow::PathGraph
+
 /**
  * A bean validation sink, such as method `buildConstraintViolationWithTemplate`
  * declared on a subtype of `javax.validation.ConstraintValidatorContext`.
@@ -72,13 +73,13 @@ private class BeanValidationSink extends DataFlow::Node {
   BeanValidationSink() { sinkNode(this, "bean-validation") }
 }
 
-from BeanValidationConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+from BeanValidationFlow::PathNode source, BeanValidationFlow::PathNode sink
 where
   (
     not exists(SetMessageInterpolatorCall c)
     or
     exists(SetMessageInterpolatorCall c | not c.isSafe())
   ) and
-  cfg.hasFlowPath(source, sink)
+  BeanValidationFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Custom constraint error message contains an unsanitized $@.",
   source, "user-provided value"

java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql

@@ -13,25 +13,29 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.StringFormat
-import DataFlow::PathGraph
 
-class ExternallyControlledFormatStringConfig extends TaintTracking::Configuration {
-  ExternallyControlledFormatStringConfig() { this = "ExternallyControlledFormatStringConfig" }
+module ExternallyControlledFormatStringConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     sink.asExpr() = any(StringFormat formatCall).getFormatArgument()
   }
 
-  override predicate isSanitizer(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) {
     node.getType() instanceof NumericType or node.getType() instanceof BooleanType
   }
 }
 
+module ExternallyControlledFormatStringFlow =
+  TaintTracking::Make<ExternallyControlledFormatStringConfig>;
+
+import ExternallyControlledFormatStringFlow::PathGraph
+
 from
-  DataFlow::PathNode source, DataFlow::PathNode sink, StringFormat formatCall,
-  ExternallyControlledFormatStringConfig conf
-where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = formatCall.getFormatArgument()
+  ExternallyControlledFormatStringFlow::PathNode source,
+  ExternallyControlledFormatStringFlow::PathNode sink, StringFormat formatCall
+where
+  ExternallyControlledFormatStringFlow::hasFlowPath(source, sink) and
+  sink.getNode().asExpr() = formatCall.getFormatArgument()
 select formatCall.getFormatArgument(), source, sink, "Format string depends on a $@.",
   source.getNode(), "user-provided value"

java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql

@@ -13,23 +13,25 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.StringFormat
-import DataFlow::PathGraph
 
-class ExternallyControlledFormatStringLocalConfig extends TaintTracking::Configuration {
-  ExternallyControlledFormatStringLocalConfig() {
-    this = "ExternallyControlledFormatStringLocalConfig"
-  }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
+module ExternallyControlledFormatStringLocalConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     sink.asExpr() = any(StringFormat formatCall).getFormatArgument()
   }
 }
 
+module ExternallyControlledFormatStringLocalFlow =
+  TaintTracking::Make<ExternallyControlledFormatStringLocalConfig>;
+
+import ExternallyControlledFormatStringLocalFlow::PathGraph
+
 from
-  DataFlow::PathNode source, DataFlow::PathNode sink, StringFormat formatCall,
-  ExternallyControlledFormatStringLocalConfig conf
-where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = formatCall.getFormatArgument()
+  ExternallyControlledFormatStringLocalFlow::PathNode source,
+  ExternallyControlledFormatStringLocalFlow::PathNode sink, StringFormat formatCall
+where
+  ExternallyControlledFormatStringLocalFlow::hasFlowPath(source, sink) and
+  sink.getNode().asExpr() = formatCall.getFormatArgument()
 select formatCall.getFormatArgument(), source, sink, "Format string depends on a $@.",
   source.getNode(), "user-provided value"