From 12a8a0866a9a3790d08dd9972e0065661700a1a0 Mon Sep 17 00:00:00 2001
From: Rhed Jao <rhedjao@google.com>
Date: Fri, 25 Jun 2021 12:37:57 +0800
Subject: [PATCH] Enforce package visibility to the api
 getAppOpPermissionPackages

App can query which applications are installed on the device via
the api IPackageManager#getAppOpPermissionPackages. This cl enforcing
the package visibility filter to the api to fix the package information
disclosure issue.

Bug: 188913056
Test: atest AppEnumerationInternalTests
Change-Id: Ic2771051babab49d885d0b21d9d8f7e9ae1d8418
---
 .../server/pm/PackageManagerService.java      | 17 +++++++-
 .../appenumeration/AndroidTest.xml            |  1 +
 .../AppEnumerationInternalTests.java          | 39 ++++++++++++++++---
 .../test-apps/target/Android.bp               | 14 +++++++
 .../AndroidManifest-hasAppOpPermission.xml    | 25 ++++++++++++
 5 files changed, 90 insertions(+), 6 deletions(-)
 create mode 100644 services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-hasAppOpPermission.xml

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 5917415204e07..79405fece0c80 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -10134,7 +10134,22 @@ public class PackageManagerService extends IPackageManager.Stub
         if (getInstantAppPackageName(getCallingUid()) != null) {
             return EmptyArray.STRING;
         }
-        return mPermissionManager.getAppOpPermissionPackages(permissionName);
+        final int callingUid = Binder.getCallingUid();
+        final int callingUserId = UserHandle.getUserId(callingUid);
+
+        final ArraySet<String> packageNames = new ArraySet(
+                mPermissionManager.getAppOpPermissionPackages(permissionName));
+        synchronized (mLock) {
+            for (int i = packageNames.size() - 1; i >= 0; i--) {
+                final String packageName = packageNames.valueAt(i);
+                if (!shouldFilterApplicationLocked(mSettings.getPackageLPr(packageName),
+                        callingUid, callingUserId)) {
+                    continue;
+                }
+                packageNames.removeAt(i);
+            }
+        }
+        return packageNames.toArray(new String[packageNames.size()]);
     }
 
     @Override
diff --git a/services/tests/PackageManagerServiceTests/appenumeration/AndroidTest.xml b/services/tests/PackageManagerServiceTests/appenumeration/AndroidTest.xml
index 6f168a3888b33..7b821c0606a09 100644
--- a/services/tests/PackageManagerServiceTests/appenumeration/AndroidTest.xml
+++ b/services/tests/PackageManagerServiceTests/appenumeration/AndroidTest.xml
@@ -32,6 +32,7 @@
     <!-- Load additional APKs onto device -->
     <target_preparer class="com.android.compatibility.common.tradefed.targetprep.FilePusher">
         <option name="push" value="AppEnumerationSyncProviderTestApp.apk->/data/local/tmp/appenumerationtests/AppEnumerationSyncProviderTestApp.apk" />
+        <option name="push" value="AppEnumerationHasAppOpPermissionTestApp.apk->/data/local/tmp/appenumerationtests/AppEnumerationHasAppOpPermissionTestApp.apk" />
     </target_preparer>
 
     <option name="test-tag" value="AppEnumerationInternalTest" />
diff --git a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java
index 9337845604105..63aaf2422116c 100644
--- a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java
+++ b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java
@@ -43,8 +43,16 @@ public class AppEnumerationInternalTests {
     private static final String TEST_DATA_PATH = "/data/local/tmp/appenumerationtests/";
     private static final String SYNC_PROVIDER_APK_PATH =
             TEST_DATA_PATH + "AppEnumerationSyncProviderTestApp.apk";
-    private static final String SYNC_PROVIDER_PKG_NAME = "com.android.appenumeration.syncprovider";
-    private static final String SYNC_PROVIDER_AUTHORITY = SYNC_PROVIDER_PKG_NAME;
+    private static final String HAS_APPOP_PERMISSION_APK_PATH =
+            TEST_DATA_PATH + "AppEnumerationHasAppOpPermissionTestApp.apk";
+
+    private static final String TARGET_SYNC_PROVIDER = "com.android.appenumeration.syncprovider";
+    private static final String TARGET_HAS_APPOP_PERMISSION =
+            "com.android.appenumeration.hasappoppermission";
+
+    private static final String SYNC_PROVIDER_AUTHORITY = TARGET_SYNC_PROVIDER;
+    private static final String PERMISSION_REQUEST_INSTALL_PACKAGES =
+            "android.permission.REQUEST_INSTALL_PACKAGES";
 
     private IPackageManager mIPackageManager;
 
@@ -55,7 +63,8 @@ public class AppEnumerationInternalTests {
 
     @After
     public void tearDown() throws Exception {
-        uninstallPackage(SYNC_PROVIDER_PKG_NAME);
+        uninstallPackage(TARGET_SYNC_PROVIDER);
+        uninstallPackage(TARGET_HAS_APPOP_PERMISSION);
     }
 
     @Test
@@ -67,7 +76,7 @@ public class AppEnumerationInternalTests {
 
         assertThat(names).contains(SYNC_PROVIDER_AUTHORITY);
         assertThat(infos.stream().map(info -> info.packageName).collect(Collectors.toList()))
-                .contains(SYNC_PROVIDER_PKG_NAME);
+                .contains(TARGET_SYNC_PROVIDER);
     }
 
     @Test
@@ -79,7 +88,27 @@ public class AppEnumerationInternalTests {
 
         assertThat(names).doesNotContain(SYNC_PROVIDER_AUTHORITY);
         assertThat(infos.stream().map(info -> info.packageName).collect(Collectors.toList()))
-                .doesNotContain(SYNC_PROVIDER_PKG_NAME);
+                .doesNotContain(TARGET_SYNC_PROVIDER);
+    }
+
+    @Test
+    public void getAppOpPermissionPackages_canSeeForceQueryable() throws Exception {
+        installPackage(HAS_APPOP_PERMISSION_APK_PATH, true /* forceQueryable */);
+
+        final String[] packageNames = mIPackageManager.getAppOpPermissionPackages(
+                PERMISSION_REQUEST_INSTALL_PACKAGES);
+
+        assertThat(packageNames).asList().contains(TARGET_HAS_APPOP_PERMISSION);
+    }
+
+    @Test
+    public void getAppOpPermissionPackages_cannotSeeHasAppOpPermission() throws Exception {
+        installPackage(HAS_APPOP_PERMISSION_APK_PATH, false /* forceQueryable */);
+
+        final String[] packageNames = mIPackageManager.getAppOpPermissionPackages(
+                PERMISSION_REQUEST_INSTALL_PACKAGES);
+
+        assertThat(packageNames).asList().doesNotContain(TARGET_HAS_APPOP_PERMISSION);
     }
 
     private static void installPackage(String apkPath, boolean forceQueryable) {
diff --git a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/Android.bp b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/Android.bp
index 64239b4c6b2d0..7aa300b177ee8 100644
--- a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/Android.bp
+++ b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/Android.bp
@@ -34,3 +34,17 @@ android_test_helper_app {
     test_suites: ["device-tests"],
     platform_apis: true,
 }
+
+android_test_helper_app {
+    name: "AppEnumerationHasAppOpPermissionTestApp",
+    srcs: ["src/**/*.java"],
+    manifest: "AndroidManifest-hasAppOpPermission.xml",
+    dex_preopt: {
+        enabled: false,
+    },
+    optimize: {
+        enabled: false,
+    },
+    test_suites: ["device-tests"],
+    platform_apis: true,
+}
diff --git a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-hasAppOpPermission.xml b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-hasAppOpPermission.xml
new file mode 100644
index 0000000000000..be8a6dc8709cd
--- /dev/null
+++ b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-hasAppOpPermission.xml
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+    Copyright (C) 2021 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.appenumeration.hasappoppermission">
+
+    <uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES" />
+    <application>
+        <uses-library android:name="android.test.runner" />
+    </application>
+</manifest>