From 7623ac966ee0a8fd94925f14527565325ee9dbd8 Mon Sep 17 00:00:00 2001
From: Sumedh Sen <sumedhsen@google.com>
Date: Mon, 27 Feb 2023 11:10:47 -0800
Subject: [PATCH] Remove the resolved path of session's APK from user
 confirmation intent

Intent sent to the user for install confirmation includes the resolved
path of the session's base apk. A malicious app can modify this path, resulting
in the installer activity to show a different app icon and label in the
confirmation dialog.

To fix this, expose a SystemApi for getting the resolved path and
permission protect it with a privileged permission.

Bug: 269728874
Test: atest CtsPackageInstallTestCases:SessionTest
Change-Id: Ibe7fe3ab5e74e3e910d9f7bd5b6f3f2e3c0ca658
---
 core/api/system-current.txt                   |  3 ++-
 .../android/content/pm/PackageInstaller.java  | 24 ++++++++++---------
 core/res/AndroidManifest.xml                  |  9 +++++++
 packages/PackageInstaller/AndroidManifest.xml |  1 +
 .../PackageInstallerActivity.java             |  7 +++---
 packages/Shell/AndroidManifest.xml            |  3 +++
 .../server/pm/PackageInstallerSession.java    | 14 +++++------
 7 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/core/api/system-current.txt b/core/api/system-current.txt
index 41da5eaa77dbc..e0e9139ac500b 100644
--- a/core/api/system-current.txt
+++ b/core/api/system-current.txt
@@ -272,6 +272,7 @@ package android {
     field public static final String READ_DEVICE_CONFIG = "android.permission.READ_DEVICE_CONFIG";
     field public static final String READ_DREAM_STATE = "android.permission.READ_DREAM_STATE";
     field public static final String READ_GLOBAL_APP_SEARCH_DATA = "android.permission.READ_GLOBAL_APP_SEARCH_DATA";
+    field public static final String READ_INSTALLED_SESSION_PATHS = "android.permission.READ_INSTALLED_SESSION_PATHS";
     field public static final String READ_INSTALL_SESSIONS = "android.permission.READ_INSTALL_SESSIONS";
     field public static final String READ_NETWORK_USAGE_HISTORY = "android.permission.READ_NETWORK_USAGE_HISTORY";
     field public static final String READ_OEM_UNLOCK_STATE = "android.permission.READ_OEM_UNLOCK_STATE";
@@ -3802,7 +3803,6 @@ package android.content.pm {
     field public static final String EXTRA_CALLBACK = "android.content.pm.extra.CALLBACK";
     field public static final String EXTRA_DATA_LOADER_TYPE = "android.content.pm.extra.DATA_LOADER_TYPE";
     field public static final String EXTRA_LEGACY_STATUS = "android.content.pm.extra.LEGACY_STATUS";
-    field public static final String EXTRA_RESOLVED_BASE_PATH = "android.content.pm.extra.RESOLVED_BASE_PATH";
     field public static final int LOCATION_DATA_APP = 0; // 0x0
     field public static final int LOCATION_MEDIA_DATA = 2; // 0x2
     field public static final int LOCATION_MEDIA_OBB = 1; // 0x1
@@ -3839,6 +3839,7 @@ package android.content.pm {
     method public boolean getInstallAsVirtualPreload();
     method public int getPendingUserActionReason();
     method public boolean getRequestDowngrade();
+    method @Nullable @RequiresPermission(android.Manifest.permission.READ_INSTALLED_SESSION_PATHS) public String getResolvedBaseApkPath();
     method public int getRollbackDataPolicy();
     method @NonNull public java.util.Set<java.lang.String> getWhitelistedRestrictedPermissions();
   }
diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java
index cb988dfdb2039..77d32a5d4fd37 100644
--- a/core/java/android/content/pm/PackageInstaller.java
+++ b/core/java/android/content/pm/PackageInstaller.java
@@ -333,17 +333,6 @@ public class PackageInstaller {
     @SystemApi
     public static final String EXTRA_DATA_LOADER_TYPE = "android.content.pm.extra.DATA_LOADER_TYPE";
 
-    /**
-     * Path to the validated base APK for this session, which may point at an
-     * APK inside the session (when the session defines the base), or it may
-     * point at the existing base APK (when adding splits to an existing app).
-     *
-     * @hide
-     */
-    @SystemApi
-    public static final String EXTRA_RESOLVED_BASE_PATH =
-            "android.content.pm.extra.RESOLVED_BASE_PATH";
-
     /**
      * Streaming installation pending.
      * Caller should make sure DataLoader is able to prepare image and reinitiate the operation.
@@ -3549,6 +3538,19 @@ public class PackageInstaller {
             return referrerUri;
         }
 
+        /**
+         * @return the path to the validated base APK for this session, which may point at an
+         * APK inside the session (when the session defines the base), or it may
+         * point at the existing base APK (when adding splits to an existing app).
+         *
+         * @hide
+         */
+        @SystemApi
+        @RequiresPermission(Manifest.permission.READ_INSTALLED_SESSION_PATHS)
+        public @Nullable String getResolvedBaseApkPath() {
+            return resolvedBaseCodePath;
+        }
+
         /**
          * Get the value set in {@link SessionParams#setGrantedRuntimePermissions(String[])}.
          *
diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml
index 479ea4e7b46ba..239a12637f28c 100644
--- a/core/res/AndroidManifest.xml
+++ b/core/res/AndroidManifest.xml
@@ -5421,6 +5421,15 @@
     <permission android:name="android.permission.INSTALL_DPC_PACKAGES"
                 android:protectionLevel="signature|role" />
 
+    <!-- @SystemApi Allows an application to read resolved paths to the APKs (Base and any splits)
+         of a session based install.
+         <p>Not for use by third-party applications.
+         @hide
+    -->
+    <permission android:name="android.permission.READ_INSTALLED_SESSION_PATHS"
+                android:protectionLevel="signature|installer" />
+    <uses-permission android:name="android.permission.READ_INSTALLED_SESSION_PATHS" />
+
     <!-- Allows an application to use System Data Loaders.
          <p>Not for use by third-party applications.
          @hide
diff --git a/packages/PackageInstaller/AndroidManifest.xml b/packages/PackageInstaller/AndroidManifest.xml
index 9ee6fbde60a6c..6ccebfd076cde 100644
--- a/packages/PackageInstaller/AndroidManifest.xml
+++ b/packages/PackageInstaller/AndroidManifest.xml
@@ -9,6 +9,7 @@
     <uses-permission android:name="android.permission.INSTALL_PACKAGES" />
     <uses-permission android:name="android.permission.DELETE_PACKAGES" />
     <uses-permission android:name="android.permission.READ_INSTALL_SESSIONS" />
+    <uses-permission android:name="android.permission.READ_INSTALLED_SESSION_PATHS" />
     <uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED" />
     <uses-permission android:name="android.permission.HIDE_NON_SYSTEM_OVERLAY_WINDOWS" />
     <uses-permission android:name="android.permission.USE_RESERVED_DISK" />
diff --git a/packages/PackageInstaller/src/com/android/packageinstaller/PackageInstallerActivity.java b/packages/PackageInstaller/src/com/android/packageinstaller/PackageInstallerActivity.java
index d41cfbc2d4ce6..7cbd4bc6ff1d4 100644
--- a/packages/PackageInstaller/src/com/android/packageinstaller/PackageInstallerActivity.java
+++ b/packages/PackageInstaller/src/com/android/packageinstaller/PackageInstallerActivity.java
@@ -374,16 +374,15 @@ public class PackageInstallerActivity extends AlertActivity {
             final int sessionId = intent.getIntExtra(PackageInstaller.EXTRA_SESSION_ID,
                     -1 /* defaultValue */);
             final SessionInfo info = mInstaller.getSessionInfo(sessionId);
-            final String resolvedBaseCodePath = intent.getStringExtra(
-                    PackageInstaller.EXTRA_RESOLVED_BASE_PATH);
-            if (info == null || !info.isSealed() || resolvedBaseCodePath == null) {
+            String resolvedPath = info.getResolvedBaseApkPath();
+            if (info == null || !info.isSealed() || resolvedPath == null) {
                 Log.w(TAG, "Session " + mSessionId + " in funky state; ignoring");
                 finish();
                 return;
             }
 
             mSessionId = sessionId;
-            packageSource = Uri.fromFile(new File(resolvedBaseCodePath));
+            packageSource = Uri.fromFile(new File(resolvedPath));
             mOriginatingURI = null;
             mReferrerURI = null;
             mPendingUserActionReason = info.getPendingUserActionReason();
diff --git a/packages/Shell/AndroidManifest.xml b/packages/Shell/AndroidManifest.xml
index 4c48f0e63b16f..5e160cd7f06a8 100644
--- a/packages/Shell/AndroidManifest.xml
+++ b/packages/Shell/AndroidManifest.xml
@@ -830,6 +830,9 @@
 
     <uses-permission android:name="android.permission.LOG_FOREGROUND_RESOURCE_USE"/>
 
+    <!-- Permission required for CTS test - CtsPackageInstallTestCases -->
+    <uses-permission android:name="android.permission.READ_INSTALLED_SESSION_PATHS" />
+
     <application android:label="@string/app_label"
                 android:theme="@android:style/Theme.DeviceDefault.DayNight"
                 android:defaultToDeviceProtectedStorage="true"
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index fa535c38c5d20..b998721aae85e 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -1135,8 +1135,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             info.userId = userId;
             info.installerPackageName = mInstallSource.mInstallerPackageName;
             info.installerAttributionTag = mInstallSource.mInstallerAttributionTag;
-            info.resolvedBaseCodePath = (mResolvedBaseFile != null) ?
-                    mResolvedBaseFile.getAbsolutePath() : null;
+            if (mContext.checkCallingOrSelfPermission(
+                    Manifest.permission.READ_INSTALLED_SESSION_PATHS)
+                            == PackageManager.PERMISSION_GRANTED && mResolvedBaseFile != null) {
+                info.resolvedBaseCodePath = mResolvedBaseFile.getAbsolutePath();
+            } else {
+                info.resolvedBaseCodePath = null;
+            }
             info.progress = progress;
             info.sealed = mSealed;
             info.isCommitted = isCommitted();
@@ -2763,11 +2768,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                         : PackageInstaller.ACTION_CONFIRM_INSTALL);
         intent.setPackage(mPm.getPackageInstallerPackageName());
         intent.putExtra(PackageInstaller.EXTRA_SESSION_ID, sessionId);
-        synchronized (mLock) {
-            intent.putExtra(PackageInstaller.EXTRA_RESOLVED_BASE_PATH,
-                    mResolvedBaseFile != null ? mResolvedBaseFile.getAbsolutePath() : null);
-        }
-
         sendOnUserActionRequired(mContext, target, sessionId, intent);
     }