Merge pull request github#12563 from egregius313/egregius313/refactor-java-libs-to-dataflow-modules

Java: Refactor Java query libraries to use dataflow modules

Author: egregius313

java/ql/lib/semmle/code/java/frameworks/JsonIo.qll

@@ -42,8 +42,12 @@ class JsonIoUseMapsSetter extends MethodAccess {
   }
 }
 
-/** A data flow configuration tracing flow from JsonIo safe settings. */
-class SafeJsonIoConfig extends DataFlow2::Configuration {
+/**
+ * DEPRECATED: Use `SafeJsonIoFlow` instead.
+ *
+ * A data flow configuration tracing flow from JsonIo safe settings.
+ */
+deprecated class SafeJsonIoConfig extends DataFlow2::Configuration {
   SafeJsonIoConfig() { this = "UnsafeDeserialization::SafeJsonIoConfig" }
 
   override predicate isSource(DataFlow::Node src) {
@@ -65,3 +69,30 @@ class SafeJsonIoConfig extends DataFlow2::Configuration {
     )
   }
 }
+
+/**
+ * A data flow configuration tracing flow from JsonIo safe settings.
+ */
+module SafeJsonIoConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) {
+    exists(MethodAccess ma |
+      ma instanceof JsonIoUseMapsSetter and
+      src.asExpr() = ma.getQualifier()
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof JsonIoJsonToJavaMethod and
+      sink.asExpr() = ma.getArgument(1)
+    )
+    or
+    exists(ClassInstanceExpr cie |
+      cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
+      sink.asExpr() = cie.getArgument(1)
+    )
+  }
+}
+
+/** Tracks flow from JsonIo safe settings. */
+module SafeJsonIoFlow = DataFlow::Global<SafeJsonIoConfig>;

java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll

@@ -4,8 +4,6 @@
 
 import java
 import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.DataFlow2
-import semmle.code.java.dataflow.DataFlow3
 
 /**
  * The class `org.yaml.snakeyaml.constructor.SafeConstructor`.
@@ -30,28 +28,28 @@ class Yaml extends RefType {
   Yaml() { this.getAnAncestor().hasQualifiedName("org.yaml.snakeyaml", "Yaml") }
 }
 
-private class SafeYamlConstructionFlowConfig extends DataFlow3::Configuration {
-  SafeYamlConstructionFlowConfig() { this = "SnakeYaml::SafeYamlConstructionFlowConfig" }
+private DataFlow::ExprNode yamlClassInstanceExprArgument(ClassInstanceExpr cie) {
+  cie.getConstructedType() instanceof Yaml and
+  result.getExpr() = cie.getArgument(0)
+}
 
-  override predicate isSource(DataFlow::Node src) {
-    src.asExpr() instanceof SafeSnakeYamlConstruction
-  }
+private module SafeYamlConstructionFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSnakeYamlConstruction }
 
-  override predicate isSink(DataFlow::Node sink) { sink = this.yamlClassInstanceExprArgument(_) }
+  predicate isSink(DataFlow::Node sink) { sink = yamlClassInstanceExprArgument(_) }
 
-  private DataFlow::ExprNode yamlClassInstanceExprArgument(ClassInstanceExpr cie) {
-    cie.getConstructedType() instanceof Yaml and
-    result.getExpr() = cie.getArgument(0)
+  additional ClassInstanceExpr getSafeYaml() {
+    SafeYamlConstructionFlow::flowTo(yamlClassInstanceExprArgument(result))
   }
-
-  ClassInstanceExpr getSafeYaml() { this.hasFlowTo(this.yamlClassInstanceExprArgument(result)) }
 }
 
+private module SafeYamlConstructionFlow = DataFlow::Global<SafeYamlConstructionFlowConfig>;
+
 /**
  * An instance of `Yaml` that does not allow arbitrary constructor to be called.
  */
 private class SafeYaml extends ClassInstanceExpr {
-  SafeYaml() { exists(SafeYamlConstructionFlowConfig conf | conf.getSafeYaml() = this) }
+  SafeYaml() { SafeYamlConstructionFlowConfig::getSafeYaml() = this }
 }
 
 /** A call to a parse method of `Yaml`. */
@@ -65,23 +63,25 @@ private class SnakeYamlParse extends MethodAccess {
   }
 }
 
-private class SafeYamlFlowConfig extends DataFlow2::Configuration {
-  SafeYamlFlowConfig() { this = "SnakeYaml::SafeYamlFlowConfig" }
+private module SafeYamlFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeYaml }
 
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeYaml }
+  predicate isSink(DataFlow::Node sink) { sink = yamlParseQualifier(_) }
 
-  override predicate isSink(DataFlow::Node sink) { sink = this.yamlParseQualifier(_) }
-
-  private DataFlow::ExprNode yamlParseQualifier(SnakeYamlParse syp) {
+  additional DataFlow::ExprNode yamlParseQualifier(SnakeYamlParse syp) {
     result.getExpr() = syp.getQualifier()
   }
 
-  SnakeYamlParse getASafeSnakeYamlParse() { this.hasFlowTo(this.yamlParseQualifier(result)) }
+  additional SnakeYamlParse getASafeSnakeYamlParse() {
+    SafeYamlFlow::flowTo(yamlParseQualifier(result))
+  }
 }
 
+private module SafeYamlFlow = DataFlow::Global<SafeYamlFlowConfig>;
+
 /**
  * A call to a parse method of `Yaml` that allows arbitrary constructor to be called.
  */
 class UnsafeSnakeYamlParse extends SnakeYamlParse {
-  UnsafeSnakeYamlParse() { not exists(SafeYamlFlowConfig sy | sy.getASafeSnakeYamlParse() = this) }
+  UnsafeSnakeYamlParse() { not SafeYamlFlowConfig::getASafeSnakeYamlParse() = this }
 }

java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll

@@ -136,24 +136,22 @@ private class GuavaRegexFlowStep extends RegexAdditionalFlowStep {
   }
 }
 
-private class RegexFlowConf extends DataFlow2::Configuration {
-  RegexFlowConf() { this = "RegexFlowConfig" }
+private module RegexFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node.asExpr() instanceof ExploitableStringLiteral }
 
-  override predicate isSource(DataFlow::Node node) {
-    node.asExpr() instanceof ExploitableStringLiteral
-  }
-
-  override predicate isSink(DataFlow::Node node) { node instanceof RegexFlowSink }
+  predicate isSink(DataFlow::Node node) { node instanceof RegexFlowSink }
 
-  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     any(RegexAdditionalFlowStep s).step(node1, node2)
   }
 
-  override predicate isBarrier(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) {
     node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
   }
 }
 
+private module RegexFlow = DataFlow::Global<RegexFlowConfig>;
+
 /**
  * Holds if `regex` is used as a regex, with the mode `mode` (if known).
  * If regex mode is not known, `mode` will be `"None"`.
@@ -162,7 +160,7 @@ private class RegexFlowConf extends DataFlow2::Configuration {
  * and therefore may be relevant for ReDoS queries are considered.
  */
 predicate usedAsRegex(StringLiteral regex, string mode, boolean match_full_string) {
-  any(RegexFlowConf c).hasFlow(DataFlow2::exprNode(regex), _) and
+  RegexFlow::flow(DataFlow::exprNode(regex), _) and
   mode = "None" and // TODO: proper mode detection
   (if matchesFullString(regex) then match_full_string = true else match_full_string = false)
 }
@@ -172,9 +170,9 @@ predicate usedAsRegex(StringLiteral regex, string mode, boolean match_full_strin
  * as though it was implicitly surrounded by ^ and $.
  */
 private predicate matchesFullString(StringLiteral regex) {
-  exists(RegexFlowConf c, RegexFlowSink sink |
+  exists(RegexFlowSink sink |
     sink.matchesFullString() and
-    c.hasFlow(DataFlow2::exprNode(regex), sink)
+    RegexFlow::flow(DataFlow::exprNode(regex), sink)
   )
 }
 
@@ -185,8 +183,8 @@ private predicate matchesFullString(StringLiteral regex) {
  * and therefore may be relevant for ReDoS queries are considered.
  */
 predicate regexMatchedAgainst(StringLiteral regex, Expr str) {
-  exists(RegexFlowConf c, RegexFlowSink sink |
+  exists(RegexFlowSink sink |
     str = sink.getStringArgument() and
-    c.hasFlow(DataFlow2::exprNode(regex), sink)
+    RegexFlow::flow(DataFlow::exprNode(regex), sink)
   )
 }

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

@@ -151,7 +151,10 @@ deprecated class SensitiveCommunicationConfig extends TaintTracking::Configurati
   }
 }
 
-private module SensitiveCommunicationConfig implements DataFlow::ConfigSig {
+/**
+ * Taint configuration tracking flow from variables containing sensitive information to broadcast Intents.
+ */
+module SensitiveCommunicationConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveInfoExpr }
 
   predicate isSink(DataFlow::Node sink) {

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

@@ -9,7 +9,7 @@ private import semmle.code.java.security.ArbitraryApkInstallation
  * A dataflow configuration for flow from an external source of an APK to the
  * `setData[AndType][AndNormalize]` method of an intent.
  */
-private module ApkInstallationConfig implements DataFlow::ConfigSig {
+module ApkInstallationConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node node) { node instanceof ExternalApkSource }
 
   predicate isSink(DataFlow::Node node) {

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

@@ -29,16 +29,16 @@ class LocalDatabaseOpenMethodAccess extends Storable, Call {
   }
 
   override Expr getAnInput() {
-    exists(LocalDatabaseFlowConfig config, DataFlow::Node database |
+    exists(DataFlow::Node database |
       localDatabaseInput(database, result) and
-      config.hasFlow(DataFlow::exprNode(this), database)
+      LocalDatabaseFlow::flow(DataFlow::exprNode(this), database)
     )
   }
 
   override Expr getAStore() {
-    exists(LocalDatabaseFlowConfig config, DataFlow::Node database |
+    exists(DataFlow::Node database |
       localDatabaseStore(database, result) and
-      config.hasFlow(DataFlow::exprNode(this), database)
+      LocalDatabaseFlow::flow(DataFlow::exprNode(this), database)
     )
   }
 }
@@ -93,19 +93,17 @@ private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store
   )
 }
 
-private class LocalDatabaseFlowConfig extends DataFlow::Configuration {
-  LocalDatabaseFlowConfig() { this = "LocalDatabaseFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
+private module LocalDatabaseFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     source.asExpr() instanceof LocalDatabaseOpenMethodAccess
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     localDatabaseInput(sink, _) or
     localDatabaseStore(sink, _)
   }
 
-  override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
+  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
     // Adds a step for tracking databases through field flow, that is, a database is opened and
     // assigned to a field, and then an input or store method is called on that field elsewhere.
     exists(Field f |
@@ -115,3 +113,5 @@ private class LocalDatabaseFlowConfig extends DataFlow::Configuration {
     )
   }
 }
+
+private module LocalDatabaseFlow = DataFlow::Global<LocalDatabaseFlowConfig>;

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

@@ -24,16 +24,16 @@ class LocalFileOpenCall extends Storable {
   }
 
   override Expr getAnInput() {
-    exists(FilesystemFlowConfig conf, DataFlow::Node n |
+    exists(DataFlow::Node n |
       filesystemInput(n, result) and
-      conf.hasFlow(DataFlow::exprNode(this), n)
+      FilesystemFlow::flow(DataFlow::exprNode(this), n)
     )
   }
 
   override Expr getAStore() {
-    exists(FilesystemFlowConfig conf, DataFlow::Node n |
+    exists(DataFlow::Node n |
       closesFile(n, result) and
-      conf.hasFlow(DataFlow::exprNode(this), n)
+      FilesystemFlow::flow(DataFlow::exprNode(this), n)
     )
   }
 }
@@ -79,17 +79,15 @@ private class CloseFileMethod extends Method {
   }
 }
 
-private class FilesystemFlowConfig extends DataFlow::Configuration {
-  FilesystemFlowConfig() { this = "FilesystemFlowConfig" }
+private module FilesystemFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall }
 
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     filesystemInput(sink, _) or
     closesFile(sink, _)
   }
 
-  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     // Add nested Writer constructors as extra data flow steps
     exists(ClassInstanceExpr cie |
       cie.getConstructedType().getAnAncestor().hasQualifiedName("java.io", "Writer") and
@@ -98,3 +96,5 @@ private class FilesystemFlowConfig extends DataFlow::Configuration {
     )
   }
 }
+
+private module FilesystemFlow = DataFlow::Global<FilesystemFlowConfig>;

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

@@ -14,8 +14,8 @@ private class ClassCleartextStorageSink extends CleartextStorageSink {
 abstract class ClassStore extends Storable, ClassInstanceExpr {
   /** Gets an input, for example `input` in `instance.password = input`. */
   override Expr getAnInput() {
-    exists(ClassStoreFlowConfig conf, DataFlow::Node instance |
-      conf.hasFlow(DataFlow::exprNode(this), instance) and
+    exists(DataFlow::Node instance |
+      ClassStoreFlow::flow(DataFlow::exprNode(this), instance) and
       result = getInstanceInput(instance, this.getConstructor().getDeclaringType())
     )
   }
@@ -40,9 +40,9 @@ private class Serializable extends ClassStore {
 
   /** Gets a store, for example `outputStream.writeObject(instance)`. */
   override Expr getAStore() {
-    exists(ClassStoreFlowConfig conf, DataFlow::Node n |
+    exists(DataFlow::Node n |
       serializableStore(n, result) and
-      conf.hasFlow(DataFlow::exprNode(this), n)
+      ClassStoreFlow::flow(DataFlow::exprNode(this), n)
     )
   }
 }
@@ -53,9 +53,9 @@ private class Marshallable extends ClassStore {
 
   /** Gets a store, for example `marshaller.marshal(instance)`. */
   override Expr getAStore() {
-    exists(ClassStoreFlowConfig conf, DataFlow::Node n |
+    exists(DataFlow::Node n |
       marshallableStore(n, result) and
-      conf.hasFlow(DataFlow::exprNode(this), n)
+      ClassStoreFlow::flow(DataFlow::exprNode(this), n)
     )
   }
 }
@@ -73,20 +73,20 @@ private Expr getInstanceInput(DataFlow::Node instance, RefType t) {
   )
 }
 
-private class ClassStoreFlowConfig extends DataFlow::Configuration {
-  ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" }
+private module ClassStoreFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }
 
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(getInstanceInput(sink, _)) or
     serializableStore(sink, _) or
     marshallableStore(sink, _)
   }
 
-  override int fieldFlowBranchLimit() { result = 1 }
+  int fieldFlowBranchLimit() { result = 1 }
 }
 
+private module ClassStoreFlow = DataFlow::Global<ClassStoreFlowConfig>;
+
 private predicate serializableStore(DataFlow::Node instance, Expr store) {
   exists(MethodAccess m |
     store = m and