Add test cases relating to intents with the ACTION_INSTALL_PACKAGE action

Author: egregius313

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

@@ -14,6 +14,7 @@ import java
 import semmle.code.java.frameworks.android.Intent
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.TaintTracking2
+import semmle.code.java.dataflow.TaintTracking3
 private import semmle.code.java.dataflow.ExternalFlow
 import DataFlow::PathGraph
 
@@ -22,6 +23,17 @@ class PackageArchiveMimeTypeLiteral extends StringLiteral {
   PackageArchiveMimeTypeLiteral() { this.getValue() = "application/vnd.android.package-archive" }
 }
 
+class InstallPackageAction extends Expr {
+  InstallPackageAction() {
+    this.(StringLiteral).getValue() = "android.intent.action.INSTALL_PACKAGE"
+    or
+    exists(VarAccess va |
+      va.getVariable().hasName("ACTION_INSTALL_PACKAGE") and
+      va.getQualifier().getType() instanceof TypeIntent
+    )
+  }
+}
+
 /** A method that sets the MIME type of an intent. */
 class SetTypeMethod extends Method {
   SetTypeMethod() {
@@ -48,7 +60,12 @@ class SetDataMethod extends Method {
 
 /** A dataflow sink for the URI of an intent. */
 class SetDataSink extends DataFlow::ExprNode {
-  SetDataSink() { this.getExpr().(MethodAccess).getMethod() instanceof SetDataMethod }
+  SetDataSink() {
+    exists(MethodAccess ma |
+      this.getExpr() = ma.getQualifier() and
+      ma.getMethod() instanceof SetDataMethod
+    )
+  }
 }
 
 /** A method that generates a URI. */
@@ -84,14 +101,44 @@ class ApkConfiguration extends DataFlow::Configuration {
     exists(MethodAccess ma |
       ma.getMethod() instanceof SetDataMethod and
       ma.getArgument(0) = node.asExpr() and
-      any(PackageArchiveMimeTypeConfiguration c).hasFlowToExpr(ma)
+      (
+        any(PackageArchiveMimeTypeConfiguration c).hasFlowToExpr(ma.getQualifier())
+        or
+        any(InstallPackageActionConfiguration c).hasFlowToExpr(ma.getQualifier())
+      )
     )
   }
 }
 
+private class InstallPackageActionConfiguration extends TaintTracking3::Configuration {
+  InstallPackageActionConfiguration() { this = "InstallPackageActionConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof InstallPackageAction
+  }
+
+  override predicate isAdditionalTaintStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    state1 instanceof DataFlow::FlowStateEmpty and
+    state2 = "hasPackageInstallAction" and
+    exists(ConstructorCall cc |
+      cc.getConstructedType() instanceof TypeIntent and
+      node1.asExpr() = cc.getArgument(0) and
+      node2.asExpr() = cc
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node, DataFlow::FlowState state) {
+    state = "hasPackageInstallAction" and node.asExpr().getType() instanceof TypeIntent
+  }
+}
+
 /**
  * A dataflow configuration tracking the flow of the Android APK MIME type to
- * the `setType` or `setTypeAndNormalize` method of an intent.
+ * 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" }

java/ql/test/query-tests/security/CWE-094/APKInstallation.expected

@@ -5,10 +5,14 @@ nodes
 | APKInstallation.java:29:24:29:38 | parse(...) | semmle.label | parse(...) |
 | APKInstallation.java:36:24:36:51 | fromFile(...) | semmle.label | fromFile(...) |
 | APKInstallation.java:43:31:43:48 | fromFile(...) | semmle.label | fromFile(...) |
+| APKInstallation.java:50:24:50:41 | fromFile(...) | semmle.label | fromFile(...) |
+| APKInstallation.java:57:24:57:41 | fromFile(...) | semmle.label | fromFile(...) |
 subpaths
 #select
 | APKInstallation.java:14:31:14:58 | fromFile(...) | APKInstallation.java:14:31:14:58 | fromFile(...) | APKInstallation.java:14:31:14:58 | fromFile(...) | Arbitrary Android APK installation. |
 | APKInstallation.java:21:31:21:44 | parse(...) | APKInstallation.java:21:31:21:44 | parse(...) | APKInstallation.java:21:31:21:44 | parse(...) | Arbitrary Android APK installation. |
 | APKInstallation.java:29:24:29:38 | parse(...) | APKInstallation.java:29:24:29:38 | parse(...) | APKInstallation.java:29:24:29:38 | parse(...) | Arbitrary Android APK installation. |
 | APKInstallation.java:36:24:36:51 | fromFile(...) | APKInstallation.java:36:24:36:51 | fromFile(...) | APKInstallation.java:36:24:36:51 | fromFile(...) | Arbitrary Android APK installation. |
 | APKInstallation.java:43:31:43:48 | fromFile(...) | APKInstallation.java:43:31:43:48 | fromFile(...) | APKInstallation.java:43:31:43:48 | fromFile(...) | Arbitrary Android APK installation. |
+| APKInstallation.java:50:24:50:41 | fromFile(...) | APKInstallation.java:50:24:50:41 | fromFile(...) | APKInstallation.java:50:24:50:41 | fromFile(...) | Arbitrary Android APK installation. |
+| APKInstallation.java:57:24:57:41 | fromFile(...) | APKInstallation.java:57:24:57:41 | fromFile(...) | APKInstallation.java:57:24:57:41 | fromFile(...) | Arbitrary Android APK installation. |

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

@@ -43,4 +43,18 @@ public void installAPK4(String path) {
         intent.setDataAndType(Uri.fromFile(file), APK_MIMETYPE);
         startActivity(intent);
     }
+
+    public void installAPK5(String path) {
+        File file = new File(Environment.getExternalStorageDirectory(), path);
+        Intent intent = new Intent(Intent.ACTION_INSTALL_PACKAGE);
+        intent.setData(Uri.fromFile(file));
+        startActivity(intent);
+    }
+
+    public void installAPK6(String path) {
+        File file = new File(Environment.getExternalStorageDirectory(), path);
+        Intent intent = new Intent("android.intent.action.INSTALL_PACKAGE");
+        intent.setData(Uri.fromFile(file));
+        startActivity(intent);
+    }
 }