From 65a5b2db39c4f5a32657130d8e95ca1c141734a1 Mon Sep 17 00:00:00 2001
From: Himanshu Gupta <himanshuz@google.com>
Date: Wed, 19 Oct 2022 14:30:04 +0000
Subject: [PATCH] Fixing Storage Volume(s) Retrieval.

With ag/19901205 shared_profile's volumes were also listed in
StorageManager#updateExternalStorageFileQuotaType.

However, the above API can be called from MediaProvider process,
without MANAGE_EXTERNAL_STORAGE permissions, resulting in
SecurityException("Only File Manager Apps permitted") to be thrown
from StorageManagerService#getVolumeList

This fix allows the exception to be bypassed in case the caller is
Media Store process.

Bug: 235321217
Test: atest android.appsecurity.cts.StorageHostTest
Change-Id: I6835cc4d29f3e9c85731979aaf9ab12a30f6419b
---
 .../android/server/StorageManagerService.java | 23 +++++++++++--------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java
index 0cf79153ce777..1150b83083cf2 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -3584,6 +3584,13 @@ class StorageManagerService extends IStorageManager.Stub
         final boolean includeSharedProfile =
                 (flags & StorageManager.FLAG_INCLUDE_SHARED_PROFILE) != 0;
 
+        // When the caller is the app actually hosting external storage, we
+        // should never attempt to augment the actual storage volume state,
+        // otherwise we risk confusing it with race conditions as users go
+        // through various unlocked states
+        final boolean callerIsMediaStore = UserHandle.isSameApp(callingUid,
+                mMediaStoreAuthorityAppId);
+
         // Only Apps with MANAGE_EXTERNAL_STORAGE should call the API with includeSharedProfile
         if (includeSharedProfile) {
             try {
@@ -3596,8 +3603,13 @@ class StorageManagerService extends IStorageManager.Stub
                 // Checking first entry in packagesFromUid is enough as using "sharedUserId"
                 // mechanism is rare and discouraged. Also, Apps that share same UID share the same
                 // permissions.
-                if (!mStorageManagerInternal.hasExternalStorageAccess(callingUid,
-                        packagesFromUid[0])) {
+                // Allowing Media Provider is an exception, Media Provider process should be allowed
+                // to query users across profiles, even without MANAGE_EXTERNAL_STORAGE access.
+                // Note that ordinarily Media provider process has the above permission, but if they
+                // are revoked, Storage Volume(s) should still be returned.
+                if (!callerIsMediaStore
+                        && !mStorageManagerInternal.hasExternalStorageAccess(callingUid,
+                                packagesFromUid[0])) {
                     throw new SecurityException("Only File Manager Apps permitted");
                 }
             } catch (RemoteException re) {
@@ -3610,13 +3622,6 @@ class StorageManagerService extends IStorageManager.Stub
         // point
         final boolean systemUserUnlocked = isSystemUnlocked(UserHandle.USER_SYSTEM);
 
-        // When the caller is the app actually hosting external storage, we
-        // should never attempt to augment the actual storage volume state,
-        // otherwise we risk confusing it with race conditions as users go
-        // through various unlocked states
-        final boolean callerIsMediaStore = UserHandle.isSameApp(callingUid,
-                mMediaStoreAuthorityAppId);
-
         final boolean userIsDemo;
         final boolean userKeyUnlocked;
         final boolean storagePermission;