Have getVolumeList() take a userId instead of a UID.

getVolumeList() previously accepted a uid and a packagename. This
approach has a few issues:
1) The uid wasn't actually checked in the implementation to match the
calling UID, nor was the package name checked against the UID in all
cases.
2) The implementation assumed that the UID also specificies the userId
that we want to get the volumes for. This was historically true, but
with the new app cloning implementation, MediaProvider running in user 0
might ask for volumes in the clone user.

To fix this, explicitly pass in the userId that we're requesting volumes
for, and verify that the calling UID and the passed in package name
match.

Bug: 206624903
Test: atest packages/providers/MediaProvider
Change-Id: Idffbb4ae7740c1b9725af0974948dc55bf1f1fde
This commit is contained in:
Martijn Coenen
2021-11-10 16:21:25 +01:00
parent eeb9d427ce
commit e57620446b
3 changed files with 19 additions and 13 deletions

View File

@@ -90,9 +90,9 @@ interface IStorageManager {
*/
int changeEncryptionPassword(int type, in String password) = 28;
/**
* Returns list of all mountable volumes.
* Returns list of all mountable volumes for the specified userId
*/
StorageVolume[] getVolumeList(int uid, in String packageName, int flags) = 29;
StorageVolume[] getVolumeList(int userId, in String callingPackage, int flags) = 29;
/**
* Determines the encryption state of the volume.
* @return a numerical value. See {@code ENCRYPTION_STATE_*} for possible

View File

@@ -1391,13 +1391,7 @@ public class StorageManager {
}
packageName = packageNames[0];
}
final int uid = ActivityThread.getPackageManager().getPackageUid(packageName,
PackageManager.MATCH_DEBUG_TRIAGED_MISSING, userId);
if (uid <= 0) {
Log.w(TAG, "Missing UID; no storage volumes available");
return new StorageVolume[0];
}
return storageManager.getVolumeList(uid, packageName, flags);
return storageManager.getVolumeList(userId, packageName, flags);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}

View File

@@ -3713,8 +3713,19 @@ class StorageManagerService extends IStorageManager.Stub
}
@Override
public StorageVolume[] getVolumeList(int uid, String packageName, int flags) {
final int userId = UserHandle.getUserId(uid);
public StorageVolume[] getVolumeList(int userId, String callingPackage, int flags) {
final int callingUid = Binder.getCallingUid();
final int callingUserId = UserHandle.getUserId(callingUid);
if (!isUidOwnerOfPackageOrSystem(callingPackage, callingUid)) {
throw new SecurityException("callingPackage does not match UID");
}
if (callingUserId != userId) {
// Callers can ask for volumes of different users, but only with the correct permissions
mContext.enforceCallingOrSelfPermission(
android.Manifest.permission.INTERACT_ACROSS_USERS,
"Need INTERACT_ACROSS_USERS to get volumes for another user");
}
final boolean forWrite = (flags & StorageManager.FLAG_FOR_WRITE) != 0;
final boolean realState = (flags & StorageManager.FLAG_REAL_STATE) != 0;
@@ -3730,7 +3741,7 @@ class StorageManagerService extends IStorageManager.Stub
// 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(Binder.getCallingUid(),
final boolean callerIsMediaStore = UserHandle.isSameApp(callingUid,
mMediaStoreAuthorityAppId);
final boolean userIsDemo;
@@ -3740,8 +3751,9 @@ class StorageManagerService extends IStorageManager.Stub
try {
userIsDemo = LocalServices.getService(UserManagerInternal.class)
.getUserInfo(userId).isDemo();
storagePermission = mStorageManagerInternal.hasExternalStorage(callingUid,
callingPackage);
userKeyUnlocked = isUserKeyUnlocked(userId);
storagePermission = mStorageManagerInternal.hasExternalStorage(uid, packageName);
} finally {
Binder.restoreCallingIdentity(token);
}