From dc90670bb4eb8c6d0ee4a19036df99475983e55a Mon Sep 17 00:00:00 2001 From: Nate Myren Date: Mon, 26 Oct 2020 11:43:27 -0700 Subject: [PATCH] Revoke storage on SDK downgrade or new full storage request When an app downgrades from above API 28 to below, or when an app begins requesting full storage in api 29, revoke storage permissions Test: Atest StorageEscalationTest Bug: 171430330 Change-Id: Ia6bd5658b77a26ab15fea013ec875050457473e0 --- .../permission/PermissionManagerService.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) 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 fb1ed2f6b58bd..13c866d0d8e21 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -205,6 +205,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { private static final int USER_PERMISSION_FLAGS = FLAG_PERMISSION_USER_SET | FLAG_PERMISSION_USER_FIXED; + /** All storage permissions */ + private static final List STORAGE_PERMISSIONS = new ArrayList<>(); + /** If the permission of the value is granted, so is the key */ private static final Map FULLER_PERMISSION_MAP = new HashMap<>(); @@ -213,6 +216,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { Manifest.permission.ACCESS_FINE_LOCATION); FULLER_PERMISSION_MAP.put(Manifest.permission.INTERACT_ACROSS_USERS, Manifest.permission.INTERACT_ACROSS_USERS_FULL); + STORAGE_PERMISSIONS.add(Manifest.permission.READ_EXTERNAL_STORAGE); + STORAGE_PERMISSIONS.add(Manifest.permission.WRITE_EXTERNAL_STORAGE); + STORAGE_PERMISSIONS.add(Manifest.permission.ACCESS_MEDIA_LOCATION); } /** Lock to protect internal data access */ @@ -2357,6 +2363,48 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } + /** + * If the app is updated, and has scoped storage permissions, then it is possible that the + * app updated in an attempt to get unscoped storage. If so, revoke all storage permissions. + * @param newPackage The new package that was installed + * @param oldPackage The old package that was updated + */ + private void revokeStoragePermissionsIfScopeExpandedInternal( + @NonNull AndroidPackage newPackage, + @NonNull AndroidPackage oldPackage) { + boolean downgradedSdk = oldPackage.getTargetSdkVersion() >= Build.VERSION_CODES.Q + && newPackage.getTargetSdkVersion() < Build.VERSION_CODES.Q; + boolean upgradedSdk = oldPackage.getTargetSdkVersion() < Build.VERSION_CODES.Q + && newPackage.getTargetSdkVersion() >= Build.VERSION_CODES.Q; + boolean newlyRequestsLegacy = !upgradedSdk && !oldPackage.isRequestLegacyExternalStorage() + && newPackage.isRequestLegacyExternalStorage(); + + if (!newlyRequestsLegacy && !downgradedSdk) { + return; + } + + final int callingUid = Binder.getCallingUid(); + final int userId = UserHandle.getUserId(newPackage.getUid()); + int numRequestedPermissions = newPackage.getRequestedPermissions().size(); + for (int i = 0; i < numRequestedPermissions; i++) { + PermissionInfo permInfo = getPermissionInfo(newPackage.getRequestedPermissions().get(i), + newPackage.getPackageName(), 0); + if (permInfo == null || !STORAGE_PERMISSIONS.contains(permInfo.name)) { + continue; + } + + EventLog.writeEvent(0x534e4554, "171430330", newPackage.getUid(), + "Revoking permission " + permInfo.name + " from package " + + newPackage.getPackageName() + " as either the sdk downgraded " + + downgradedSdk + " or newly requested legacy full storage " + + newlyRequestsLegacy); + + revokeRuntimePermissionInternal(permInfo.name, newPackage.getPackageName(), + false, callingUid, userId, null, mDefaultPermissionCallback); + } + + } + /** * We might auto-grant permissions if any permission of the group is already granted. Hence if * the group of a granted permission changes we need to revoke it to avoid having permissions of @@ -4884,6 +4932,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { AsyncTask.execute(() -> { if (hasOldPkg) { revokeRuntimePermissionsIfGroupChangedInternal(pkg, oldPkg); + revokeStoragePermissionsIfScopeExpandedInternal(pkg, oldPkg); } if (hasPermissionDefinitionChanges) { revokeRuntimePermissionsIfPermissionDefinitionChangedInternal(