Merge pull request #50 from egregius313/egregius313/refactor-apk-query-using-dataflow-modules

Convert dataflow configurations in Arbitrary APK Installation query to use new module-configuration

Author: egregius313

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

@@ -2,46 +2,45 @@
 
 import java
 import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.TaintTracking2
-import semmle.code.java.dataflow.TaintTracking3
+import semmle.code.java.dataflow.TaintTracking
 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.
  */
-class ApkConfiguration extends DataFlow::Configuration {
-  ApkConfiguration() { this = "ApkConfiguration" }
+private module ApkConf implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof ExternalApkSource }
 
-  override predicate isSource(DataFlow::Node node) { node instanceof ExternalApkSource }
-
-  override predicate isSink(DataFlow::Node node) {
+  predicate isSink(DataFlow::Node node) {
     exists(MethodAccess ma |
       ma.getMethod() instanceof SetDataMethod and
       ma.getArgument(0) = node.asExpr() and
       (
-        any(PackageArchiveMimeTypeConfiguration c).hasFlowToExpr(ma.getQualifier())
+        PackageArchiveMimeTypeConfiguration::hasFlowToExpr(ma.getQualifier())
         or
-        any(InstallPackageActionConfiguration c).hasFlowToExpr(ma.getQualifier())
+        InstallPackageActionConfiguration::hasFlowToExpr(ma.getQualifier())
       )
     )
   }
 }
 
+module ApkConfiguration = DataFlow::Make<ApkConf>;
+
 /**
  * A dataflow configuration tracking the flow from the `android.content.Intent.ACTION_INSTALL_PACKAGE`
  * constant to either the constructor of an intent or the `setAction` method of an intent.
  *
  * This is used to track if an intent is used to install an APK.
  */
-private class InstallPackageActionConfiguration extends TaintTracking3::Configuration {
-  InstallPackageActionConfiguration() { this = "InstallPackageActionConfiguration" }
+private module InstallPackageActionConfig implements DataFlow::StateConfigSig {
+  class FlowState = string;
 
-  override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof InstallPackageAction
+  predicate isSource(DataFlow::Node source, FlowState state) {
+    source.asExpr() instanceof InstallPackageAction and state instanceof DataFlow::FlowStateEmpty
   }
 
-  override predicate isAdditionalTaintStep(
+  predicate isAdditionalFlowStep(
     DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
     DataFlow::FlowState state2
   ) {
@@ -63,24 +62,30 @@ private class InstallPackageActionConfiguration extends TaintTracking3::Configur
     )
   }
 
-  override predicate isSink(DataFlow::Node node, DataFlow::FlowState state) {
+  predicate isSink(DataFlow::Node node, DataFlow::FlowState state) {
     state = "hasPackageInstallAction" and node.asExpr().getType() instanceof TypeIntent
   }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) { none() }
 }
 
+private module InstallPackageActionConfiguration =
+  TaintTracking::MakeWithState<InstallPackageActionConfig>;
+
 /**
  * A dataflow configuration tracking the flow of the Android APK MIME type to
  * the `setType` or `setTypeAndNormalize` method of an intent, followed by a call
  * to `setData[AndType][AndNormalize]`.
  */
-private class PackageArchiveMimeTypeConfiguration extends TaintTracking2::Configuration {
-  PackageArchiveMimeTypeConfiguration() { this = "PackageArchiveMimeTypeConfiguration" }
+private module PackageArchiveMimeTypeConfig implements DataFlow::StateConfigSig {
+  class FlowState = string;
 
-  override predicate isSource(DataFlow::Node node) {
-    node.asExpr() instanceof PackageArchiveMimeTypeLiteral
+  predicate isSource(DataFlow::Node node, FlowState state) {
+    node.asExpr() instanceof PackageArchiveMimeTypeLiteral and
+    state instanceof DataFlow::FlowStateEmpty
   }
 
-  override predicate isAdditionalTaintStep(
+  predicate isAdditionalFlowStep(
     DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
     DataFlow::FlowState state2
   ) {
@@ -98,8 +103,13 @@ private class PackageArchiveMimeTypeConfiguration extends TaintTracking2::Config
     )
   }
 
-  override predicate isSink(DataFlow::Node node, DataFlow::FlowState state) {
+  predicate isSink(DataFlow::Node node, DataFlow::FlowState state) {
     state = "typeSet" and
     node instanceof SetDataSink
   }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) { none() }
 }
+
+private module PackageArchiveMimeTypeConfiguration =
+  TaintTracking::MakeWithState<PackageArchiveMimeTypeConfig>;

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

@@ -12,8 +12,8 @@
 
 import java
 import semmle.code.java.security.ArbitraryApkInstallationQuery
-import DataFlow::PathGraph
+import ApkConfiguration::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, ApkConfiguration config
-where config.hasFlowPath(source, sink)
+from ApkConfiguration::PathNode source, ApkConfiguration::PathNode sink
+where ApkConfiguration::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Arbitrary Android APK installation."

java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.ql

@@ -10,7 +10,7 @@ class HasApkInstallationTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasApkInstallation" and
-    exists(DataFlow::Node sink, ApkConfiguration conf | conf.hasFlowTo(sink) |
+    exists(DataFlow::Node sink | ApkConfiguration::hasFlowTo(sink) |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""