From 8f2ce504259193d67d8382f94186e00864143ab3 Mon Sep 17 00:00:00 2001
From: Winson <chiuwinson@google.com>
Date: Thu, 23 Apr 2020 10:45:55 -0700
Subject: [PATCH] Verify INSTALL_PACKAGES permissions when adding installer
 package

Without this check, any package can set the installer package of
another package whose installer has been removed or was never set.
This provides access to other privileged actions and is undesired.

Bug: 150857253

Test: manual verify with proof of concept in linked bug
Test: atest android.appsecurity.cts.PackageSetInstallerTest

Change-Id: I2159c357911ff39ffd819054b42f96ae86bc98bc
---
 .../server/pm/PackageManagerService.java      | 58 +++++++++++++++----
 1 file changed, 46 insertions(+), 12 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index bde9d5735960c..21c293a14b6c6 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -148,6 +148,8 @@ import android.app.ResourcesManager;
 import android.app.admin.IDevicePolicyManager;
 import android.app.admin.SecurityLog;
 import android.app.backup.IBackupManager;
+import android.compat.annotation.ChangeId;
+import android.compat.annotation.EnabledAfter;
 import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.ContentResolver;
@@ -632,6 +634,19 @@ public class PackageManagerService extends IPackageManager.Stub
      */
     private static final int DEFAULT_VERIFICATION_RESPONSE = PackageManager.VERIFICATION_ALLOW;
 
+    /**
+     * Adding an installer package name to a package that does not have one set requires the
+     * INSTALL_PACKAGES permission.
+     *
+     * If the caller targets R, this will throw a SecurityException. Otherwise the request will
+     * fail silently. In both cases, and regardless of whether this change is enabled, the
+     * installer package will remain unchanged.
+     */
+    @ChangeId
+    @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.Q)
+    private static final long THROW_EXCEPTION_ON_REQUIRE_INSTALL_PACKAGES_TO_ADD_INSTALLER_PACKAGE =
+            150857253;
+
     public static final String PLATFORM_PACKAGE_NAME = "android";
 
     private static final String PACKAGE_MIME_TYPE = "application/vnd.android.package-archive";
@@ -14103,19 +14118,38 @@ public class PackageManagerService extends IPackageManager.Stub
             // be signed with the same cert as the caller.
             String targetInstallerPackageName =
                     targetPackageSetting.installSource.installerPackageName;
-            if (targetInstallerPackageName != null) {
-                PackageSetting setting = mSettings.mPackages.get(
-                        targetInstallerPackageName);
-                // If the currently set package isn't valid, then it's always
-                // okay to change it.
-                if (setting != null) {
-                    if (compareSignatures(callerSignature,
-                            setting.signatures.mSigningDetails.signatures)
-                            != PackageManager.SIGNATURE_MATCH) {
-                        throw new SecurityException(
-                                "Caller does not have same cert as old installer package "
-                                + targetInstallerPackageName);
+            PackageSetting targetInstallerPkgSetting = targetInstallerPackageName == null ? null :
+                    mSettings.mPackages.get(targetInstallerPackageName);
+
+            if (targetInstallerPkgSetting != null) {
+                if (compareSignatures(callerSignature,
+                        targetInstallerPkgSetting.signatures.mSigningDetails.signatures)
+                        != PackageManager.SIGNATURE_MATCH) {
+                    throw new SecurityException(
+                            "Caller does not have same cert as old installer package "
+                            + targetInstallerPackageName);
+                }
+            } else if (mContext.checkCallingOrSelfPermission(Manifest.permission.INSTALL_PACKAGES)
+                    != PackageManager.PERMISSION_GRANTED) {
+                // This is probably an attempt to exploit vulnerability b/150857253 of taking
+                // privileged installer permissions when the installer has been uninstalled or
+                // was never set.
+                EventLog.writeEvent(0x534e4554, "150857253", callingUid, "");
+
+                long binderToken = Binder.clearCallingIdentity();
+                try {
+                    if (mInjector.getCompatibility().isChangeEnabledByUid(
+                            THROW_EXCEPTION_ON_REQUIRE_INSTALL_PACKAGES_TO_ADD_INSTALLER_PACKAGE,
+                            callingUid)) {
+                        throw new SecurityException("Neither user " + callingUid
+                                + " nor current process has "
+                                + Manifest.permission.INSTALL_PACKAGES);
+                    } else {
+                        // If change disabled, fail silently for backwards compatibility
+                        return;
                     }
+                } finally {
+                    Binder.restoreCallingIdentity(binderToken);
                 }
             }