From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Winson <chiuwinson@google.com>
Date: Tue, 26 May 2020 10:52:23 -0700
Subject: [PATCH] Add PackageInstaller SessionParams restrictions

To mitigate a boot loop with reading a massive
install_sessions.xml file, this restricts the amount of
data that can be written by limiting the size of
unbounded parameters like package name and app label.

This introduces a lowered max session count. 50 for general
applications without the INSTALL_PACKAGES permission, and
the same 1024 for those with the permission.

Also truncates labels read from PackageItemInfo to 1000
characters, which is probably enough.

These changes restrict a malicious third party app to ~0.15 MB
written to disk, and a valid installer to ~3.6 MB, as opposed to
the >1000 MB previously allowed.

These numbers assume no install granted runtime permissions.
Those were not restricted since there's no good way to do so,
but it's assumed that any installer with that permission is
highly privleged and doesn't need to be limited.

Along the same lines, DataLoaderParams are also not restricted.
This will have to be added if that API is ever made public.

However, installer package was restricted, even though the API is
hidden. It was an easy add and may have some effect since the value
is derived from other data and passed through by other system
components.

It's still possible to inflate the file size if a lot of
different apps attempt to install a large number of packages,
but that would require thousands of malicious apps to be installed.

Bug: 157224146

Test: atest android.content.pm.PackageSessionTests

Change-Id: Iec42bee08d19d4ac53b361a92be6bc1401d9efc8
---
 .../android/content/pm/PackageInstaller.java  |  12 +-
 .../android/content/pm/PackageItemInfo.java   |  12 +-
 .../tests/PackageInstallerSessions/Android.bp |  42 ++++
 .../AndroidManifest.xml                       |  29 +++
 .../android/content/pm/PackageSessionTests.kt | 188 ++++++++++++++++++
 .../server/pm/PackageInstallerService.java    |  34 +++-
 .../permission/PermissionManagerService.java  |  14 ++
 .../PermissionManagerServiceInternal.java     |   7 +
 8 files changed, 332 insertions(+), 6 deletions(-)
 create mode 100644 core/tests/PackageInstallerSessions/Android.bp
 create mode 100644 core/tests/PackageInstallerSessions/AndroidManifest.xml
 create mode 100644 core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt

diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java
index b44b6d90811e..fce60faf6170 100644
--- a/core/java/android/content/pm/PackageInstaller.java
+++ b/core/java/android/content/pm/PackageInstaller.java
@@ -1277,6 +1277,13 @@ public class PackageInstaller {
         /** {@hide} */
         public static final int UID_UNKNOWN = -1;
 
+        /**
+         * This value is derived from the maximum file name length. No package above this limit
+         * can ever be successfully installed on the device.
+         * @hide
+         */
+        public static final int MAX_PACKAGE_NAME_LENGTH = 255;
+
         /** {@hide} */
         @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
         public int mode = MODE_INVALID;
@@ -1450,6 +1457,8 @@ public class PackageInstaller {
 
         /**
          * Optionally set a label representing the app being installed.
+         *
+         * This value will be trimmed to the first 1000 characters.
          */
         public void setAppLabel(@Nullable CharSequence appLabel) {
             this.appLabel = (appLabel != null) ? appLabel.toString() : null;
@@ -1519,7 +1528,8 @@ public class PackageInstaller {
          *
          * <p>Initially, all restricted permissions are whitelisted but you can change
          * which ones are whitelisted by calling this method or the corresponding ones
-         * on the {@link PackageManager}.
+         * on the {@link PackageManager}. Only soft or hard restricted permissions on the current
+         * Android version are supported and any invalid entries will be removed.
          *
          * @see PackageManager#addWhitelistedRestrictedPermission(String, String, int)
          * @see PackageManager#removeWhitelistedRestrictedPermission(String, String, int)
diff --git a/core/java/android/content/pm/PackageItemInfo.java b/core/java/android/content/pm/PackageItemInfo.java
index aa8e84262049..7cef05ac8733 100644
--- a/core/java/android/content/pm/PackageItemInfo.java
+++ b/core/java/android/content/pm/PackageItemInfo.java
@@ -48,8 +48,16 @@ import java.util.Comparator;
  * in the implementation of Parcelable in subclasses.
  */
 public class PackageItemInfo {
-    /** The maximum length of a safe label, in characters */
-    private static final int MAX_SAFE_LABEL_LENGTH = 1000;
+
+    /**
+     * The maximum length of a safe label, in characters
+     *
+     * TODO(b/157997155): It may make sense to expose this publicly so that apps can check for the
+     *  value and truncate the strings/use a different label, without having to hardcode and make
+     *  assumptions about the value.
+     * @hide
+     */
+    public static final int MAX_SAFE_LABEL_LENGTH = 1000;
 
     /** @hide */
     public static final float DEFAULT_MAX_LABEL_SIZE_PX = 500f;
diff --git a/core/tests/PackageInstallerSessions/Android.bp b/core/tests/PackageInstallerSessions/Android.bp
new file mode 100644
index 000000000000..a564d30f3008
--- /dev/null
+++ b/core/tests/PackageInstallerSessions/Android.bp
@@ -0,0 +1,42 @@
+//
+// Copyright 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+android_test {
+    name: "FrameworksCorePackageInstallerSessionsTests",
+
+    srcs: [
+        "src/**/*.kt",
+    ],
+    static_libs: [
+        "androidx.test.rules",
+        "compatibility-device-util-axt",
+        "frameworks-base-testutils",
+        "platform-test-annotations",
+        "testng",
+        "truth-prebuilt",
+    ],
+
+    libs: [
+        "android.test.runner",
+        "android.test.base",
+        "framework",
+        "framework-res",
+    ],
+
+    platform_apis: true,
+    sdk_version: "test_current",
+    test_suites: ["device-tests"],
+}
diff --git a/core/tests/PackageInstallerSessions/AndroidManifest.xml b/core/tests/PackageInstallerSessions/AndroidManifest.xml
new file mode 100644
index 000000000000..5b22d2b4f3e3
--- /dev/null
+++ b/core/tests/PackageInstallerSessions/AndroidManifest.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ -->
+
+<manifest
+    xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.android.frameworks.coretests.package_installer_sessions"
+    >
+
+    <application>
+        <uses-library android:name="android.test.runner" />
+    </application>
+
+    <instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
+        android:targetPackage="com.android.frameworks.coretests.package_installer_sessions"/>
+</manifest>
diff --git a/core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt b/core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt
new file mode 100644
index 000000000000..494c92a8aa3f
--- /dev/null
+++ b/core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt
@@ -0,0 +1,188 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.content.pm
+
+import android.content.Context
+import android.content.pm.PackageInstaller.SessionParams
+import android.platform.test.annotations.Presubmit
+import androidx.test.InstrumentationRegistry
+import androidx.test.filters.LargeTest
+import com.android.compatibility.common.util.ShellIdentityUtils
+import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import org.testng.Assert.assertThrows
+import kotlin.random.Random
+
+/**
+ * For verifying public [PackageInstaller] session APIs. This differs from
+ * [com.android.server.pm.PackageInstallerSessionTest] in services because that mocks the session,
+ * whereas this test uses the installer on device.
+ */
+@Presubmit
+class PackageSessionTests {
+
+    companion object {
+        /**
+         * Permissions marked "hardRestricted" or "softRestricted" in core/res/AndroidManifest.xml.
+         */
+        private val RESTRICTED_PERMISSIONS = listOf(
+                "android.permission.SEND_SMS",
+                "android.permission.RECEIVE_SMS",
+                "android.permission.READ_SMS",
+                "android.permission.RECEIVE_WAP_PUSH",
+                "android.permission.RECEIVE_MMS",
+                "android.permission.READ_CELL_BROADCASTS",
+                "android.permission.ACCESS_BACKGROUND_LOCATION",
+                "android.permission.READ_CALL_LOG",
+                "android.permission.WRITE_CALL_LOG",
+                "android.permission.PROCESS_OUTGOING_CALLS"
+        )
+    }
+
+    private val context: Context = InstrumentationRegistry.getContext()
+
+    private val installer = context.packageManager.packageInstaller
+
+    @Before
+    @After
+    fun abandonAllSessions() {
+        installer.mySessions.asSequence()
+                .map { it.sessionId }
+                .forEach {
+                    try {
+                        installer.abandonSession(it)
+                    } catch (ignored: Exception) {
+                        // Querying for sessions checks by calling package name, but abandoning
+                        // checks by UID, which won't match if this test failed to clean up
+                        // on a previous install + run + uninstall, so ignore these failures.
+                    }
+                }
+    }
+
+    @Test
+    fun truncateAppLabel() {
+        val longLabel = invalidAppLabel()
+        val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply {
+            setAppLabel(longLabel)
+        }
+
+        createSession(params) {
+            assertThat(installer.getSessionInfo(it)?.appLabel)
+                    .isEqualTo(longLabel.take(PackageItemInfo.MAX_SAFE_LABEL_LENGTH))
+        }
+    }
+
+    @Test
+    fun removeInvalidAppPackageName() {
+        val longName = invalidPackageName()
+        val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply {
+            setAppPackageName(longName)
+        }
+
+        createSession(params) {
+            assertThat(installer.getSessionInfo(it)?.appPackageName)
+                    .isEqualTo(null)
+        }
+    }
+
+    @Test
+    fun removeInvalidInstallerPackageName() {
+        val longName = invalidPackageName()
+        val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply {
+            setInstallerPackageName(longName)
+        }
+
+        createSession(params) {
+            // If a custom installer name is dropped, it defaults to the caller
+            assertThat(installer.getSessionInfo(it)?.installerPackageName)
+                    .isEqualTo(context.packageName)
+        }
+    }
+
+    @Test
+    fun truncateWhitelistPermissions() {
+        val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply {
+            setWhitelistedRestrictedPermissions(invalidPermissions())
+        }
+
+        createSession(params) {
+            assertThat(installer.getSessionInfo(it)?.whitelistedRestrictedPermissions!!)
+                    .containsExactlyElementsIn(RESTRICTED_PERMISSIONS)
+        }
+    }
+
+    @LargeTest
+    @Test
+    fun allocateMaxSessionsWithPermission() {
+        ShellIdentityUtils.invokeWithShellPermissions {
+            repeat(1024) { createDummySession() }
+            assertThrows(IllegalStateException::class.java) { createDummySession() }
+        }
+    }
+
+    @LargeTest
+    @Test
+    fun allocateMaxSessionsNoPermission() {
+        repeat(50) { createDummySession() }
+        assertThrows(IllegalStateException::class.java) { createDummySession() }
+    }
+
+    private fun createDummySession() {
+        installer.createSession(SessionParams(SessionParams.MODE_FULL_INSTALL)
+                .apply {
+                    setAppPackageName(invalidPackageName())
+                    setAppLabel(invalidAppLabel())
+                    setWhitelistedRestrictedPermissions(invalidPermissions())
+                })
+    }
+
+    private fun invalidPackageName(maxLength: Int = SessionParams.MAX_PACKAGE_NAME_LENGTH): String {
+        return (0 until (maxLength + 10))
+                .asSequence()
+                .mapIndexed { index, _ ->
+                    // A package name needs at least one separator
+                    if (index == 2) {
+                        '.'
+                    } else {
+                        Random.nextInt('z' - 'a').toChar() + 'a'.toInt()
+                    }
+                }
+                .joinToString(separator = "")
+    }
+
+    private fun invalidAppLabel() = (0 until PackageItemInfo.MAX_SAFE_LABEL_LENGTH + 10)
+            .asSequence()
+            .map { Random.nextInt(Char.MAX_VALUE.toInt()).toChar() }
+            .joinToString(separator = "")
+
+    private fun invalidPermissions() = RESTRICTED_PERMISSIONS.toMutableSet()
+            .apply {
+                // Add some invalid permission names
+                repeat(10) { add(invalidPackageName(300)) }
+            }
+
+    private fun createSession(params: SessionParams, block: (Int) -> Unit = {}) {
+        val sessionId = installer.createSession(params)
+        try {
+            block(sessionId)
+        } finally {
+            installer.abandonSession(sessionId)
+        }
+    }
+}
diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index ea144fd7c4d5..7ab0c243ac58 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -42,6 +42,7 @@ import android.content.pm.PackageInfo;
 import android.content.pm.PackageInstaller;
 import android.content.pm.PackageInstaller.SessionInfo;
 import android.content.pm.PackageInstaller.SessionParams;
+import android.content.pm.PackageItemInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.ParceledListSlice;
 import android.content.pm.VersionedPackage;
@@ -126,8 +127,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
     private static final long MAX_AGE_MILLIS = 3 * DateUtils.DAY_IN_MILLIS;
     /** Automatically destroy staged sessions that have not changed state in this time */
     private static final long MAX_TIME_SINCE_UPDATE_MILLIS = 7 * DateUtils.DAY_IN_MILLIS;
-    /** Upper bound on number of active sessions for a UID */
-    private static final long MAX_ACTIVE_SESSIONS = 1024;
+    /** Upper bound on number of active sessions for a UID that has INSTALL_PACKAGES */
+    private static final long MAX_ACTIVE_SESSIONS_WITH_PERMISSION = 1024;
+    /** Upper bound on number of active sessions for a UID without INSTALL_PACKAGES */
+    private static final long MAX_ACTIVE_SESSIONS_NO_PERMISSION = 50;
     /** Upper bound on number of historical sessions for a UID */
     private static final long MAX_HISTORICAL_SESSIONS = 1048576;
     /** Destroy sessions older than this on storage free request */
@@ -535,6 +538,20 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
             throw new SecurityException("User restriction prevents installing");
         }
 
+        // App package name and label length is restricted so that really long strings aren't
+        // written to disk.
+        if (params.appPackageName != null
+                && params.appPackageName.length() > SessionParams.MAX_PACKAGE_NAME_LENGTH) {
+            params.appPackageName = null;
+        }
+
+        params.appLabel = TextUtils.trimToSize(params.appLabel,
+                PackageItemInfo.MAX_SAFE_LABEL_LENGTH);
+
+        String requestedInstallerPackageName = (params.installerPackageName != null
+                && params.installerPackageName.length() < SessionParams.MAX_PACKAGE_NAME_LENGTH)
+                ? params.installerPackageName : installerPackageName;
+
         if ((callingUid == Process.SHELL_UID) || (callingUid == Process.ROOT_UID)) {
             params.installFlags |= PackageManager.INSTALL_FROM_ADB;
 
@@ -638,12 +655,23 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
             }
         }
 
+        if (params.whitelistedRestrictedPermissions != null) {
+            mPermissionManager.retainHardAndSoftRestrictedPermissions(
+                    params.whitelistedRestrictedPermissions);
+        }
+
         final int sessionId;
         final PackageInstallerSession session;
         synchronized (mSessions) {
             // Sanity check that installer isn't going crazy
             final int activeCount = getSessionCount(mSessions, callingUid);
-            if (activeCount >= MAX_ACTIVE_SESSIONS) {
+            if (mContext.checkCallingOrSelfPermission(Manifest.permission.INSTALL_PACKAGES)
+                    == PackageManager.PERMISSION_GRANTED) {
+                if (activeCount >= MAX_ACTIVE_SESSIONS_WITH_PERMISSION) {
+                    throw new IllegalStateException(
+                            "Too many active sessions for UID " + callingUid);
+                }
+            } else if (activeCount >= MAX_ACTIVE_SESSIONS_NO_PERMISSION) {
                 throw new IllegalStateException(
                         "Too many active sessions for UID " + callingUid);
             }
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
index 9f8e6eae3ba8..2fcdec7c92d6 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -3468,5 +3468,19 @@ public class PermissionManagerService {
             PermissionManagerService.this.removeOnRuntimePermissionStateChangedListener(
                     listener);
         }
+
+        @Override
+        public void retainHardAndSoftRestrictedPermissions(@NonNull List<String> permissions) {
+            synchronized (mLock) {
+                Iterator<String> iterator = permissions.iterator();
+                while (iterator.hasNext()) {
+                    String permission = iterator.next();
+                    BasePermission basePermission = mSettings.mPermissions.get(permission);
+                    if (basePermission == null || !basePermission.isHardOrSoftRestricted()) {
+                        iterator.remove();
+                    }
+                }
+            }
+        }
     }
 }
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
index a2f64eafe151..39ae8f589db2 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
@@ -36,6 +36,7 @@ import java.util.List;
  * TODO: Should be merged into PermissionManagerInternal, but currently uses internal classes.
  */
 public abstract class PermissionManagerServiceInternal extends PermissionManagerInternal {
+
     /**
      * Callbacks invoked when interesting actions have been taken on a permission.
      * <p>
@@ -212,4 +213,10 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
     /** Get all permission that have a certain protection level */
     public abstract @NonNull ArrayList<PermissionInfo> getAllPermissionWithProtectionLevel(
             @PermissionInfo.Protection int protectionLevel);
+
+    /**
+     * Removes invalid permissions which are not {@link PermissionInfo#FLAG_HARD_RESTRICTED} or
+     * {@link PermissionInfo#FLAG_SOFT_RESTRICTED} from the input.
+     */
+    public abstract void retainHardAndSoftRestrictedPermissions(@NonNull List<String> permissions);
 }
