From e1233195e8cc19ac0c84ef2cfc1259abc5d1ebc5 Mon Sep 17 00:00:00 2001
From: "Philip P. Moltmann" <moltmann@google.com>
Date: Thu, 18 Apr 2019 08:45:55 -0700
Subject: [PATCH] Read newImplicit perms before modifying perm state

For shared uids the original and new state are the same object. Hence we
loose knowledge of the old state when we modify it. Hence check if the
perm is new & implicit before modifying the state.

Bug: 129796592
Fixes: 130707789
Test: atest CtsPermissionTestCases:android.permission.cts.SplitPermissionTest
      (added new test case for shared uids)
Change-Id: I647bb28e667c3b8a93e11bd1771ad472c1a132c1
---
 .../permission/PermissionManagerService.java  | 35 ++++++++-----------
 .../android/server/pm/permission/TEST_MAPPING |  3 ++
 2 files changed, 18 insertions(+), 20 deletions(-)

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 9ede263284a12..d45a8ef4e0ae6 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -922,6 +922,8 @@ public class PermissionManagerService {
         permissionsState.setGlobalGids(mGlobalGids);
 
         synchronized (mLock) {
+            ArraySet<String> newImplicitPermissions = new ArraySet<>();
+
             final int N = pkg.requestedPermissions.size();
             for (int i = 0; i < N; i++) {
                 final String permName = pkg.requestedPermissions.get(i);
@@ -943,6 +945,17 @@ public class PermissionManagerService {
                     continue;
                 }
 
+                // Cache newImplicitPermissions before modifing permissionsState as for the shared
+                // uids the original and new state are the same object
+                if (!origPermissions.hasRequestedPermission(permName)
+                        && pkg.implicitPermissions.contains(permName)) {
+                    newImplicitPermissions.add(permName);
+
+                    if (DEBUG_PERMISSIONS) {
+                        Slog.i(TAG, permName + " is newly added for " + pkg.packageName);
+                    }
+                }
+
                 // Limit ephemeral apps to ephemeral allowed permissions.
                 if (pkg.applicationInfo.isInstantApp() && !bp.isInstant()) {
                     if (DEBUG_PERMISSIONS) {
@@ -1298,7 +1311,7 @@ public class PermissionManagerService {
             updatedUserIds = revokePermissionsNoLongerImplicitLocked(permissionsState, pkg,
                     updatedUserIds);
             updatedUserIds = setInitialGrantForNewImplicitPermissionsLocked(origPermissions,
-                    permissionsState, pkg, updatedUserIds);
+                    permissionsState, pkg, newImplicitPermissions, updatedUserIds);
         }
 
         // Persist the runtime permissions state for users with changes. If permissions
@@ -1437,27 +1450,9 @@ public class PermissionManagerService {
     private @NonNull int[] setInitialGrantForNewImplicitPermissionsLocked(
             @NonNull PermissionsState origPs,
             @NonNull PermissionsState ps, @NonNull PackageParser.Package pkg,
+            @NonNull ArraySet<String> newImplicitPermissions,
             @NonNull int[] updatedUserIds) {
         String pkgName = pkg.packageName;
-        ArraySet<String> newImplicitPermissions = new ArraySet<>();
-
-        int numRequestedPerms = pkg.requestedPermissions.size();
-        for (int i = 0; i < numRequestedPerms; i++) {
-            BasePermission bp = mSettings.getPermissionLocked(pkg.requestedPermissions.get(i));
-            if (bp != null) {
-                String perm = bp.getName();
-
-                if (!origPs.hasRequestedPermission(perm) && pkg.implicitPermissions.contains(
-                        perm)) {
-                    newImplicitPermissions.add(perm);
-
-                    if (DEBUG_PERMISSIONS) {
-                        Slog.i(TAG, perm + " is newly added for " + pkgName);
-                    }
-                }
-            }
-        }
-
         ArrayMap<String, ArraySet<String>> newToSplitPerms = new ArrayMap<>();
 
         int numSplitPerms = PermissionManager.SPLIT_PERMISSIONS.size();
diff --git a/services/core/java/com/android/server/pm/permission/TEST_MAPPING b/services/core/java/com/android/server/pm/permission/TEST_MAPPING
index 05e9b93bcf5ad..c27270712058a 100644
--- a/services/core/java/com/android/server/pm/permission/TEST_MAPPING
+++ b/services/core/java/com/android/server/pm/permission/TEST_MAPPING
@@ -11,6 +11,9 @@
                 },
                 {
                     "include-filter": "android.permission.cts.PermissionFlagsTest"
+                },
+                {
+                    "include-filter": "android.permission.cts.SharedUidPermissionsTest"
                 }
             ]
         },