From ddca60058e69c5e751b217ea24610b188243944c Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Fri, 18 Jun 2021 13:22:13 -0700 Subject: [PATCH] Prevent app check via content provider. Pass in the calling UID when asking PackageManager to resolve the content provider. This makes sure PM is resolving the provider based on the calling UID's permissions and not the system server's permissions. Bug: 189122544 Bug: 189122545 Test: atest CtsContentTestCases Test: atest CtsJobSchedulerTestCases Test: atest FrameworksServicesTests:UriGrantsManagerServiceTest Change-Id: Iaa421e982fca1e29828eda43f0762893199b5ce1 --- .../content/pm/PackageManagerInternal.java | 6 ++++ .../server/pm/PackageManagerService.java | 17 +++++++++- .../server/uri/UriGrantsManagerService.java | 8 ++++- .../uri/UriGrantsManagerServiceTest.java | 34 +++++++++++-------- .../server/uri/UriGrantsMockContext.java | 15 ++++++++ 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index f7ddd2922b254..95186ee444e40 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -565,6 +565,12 @@ public abstract class PackageManagerInternal implements PackageSettingsSnapshotP */ public abstract ProviderInfo resolveContentProvider(String name, int flags, int userId); + /** + * Resolves a content provider intent. + */ + public abstract ProviderInfo resolveContentProvider(String name, int flags, int userId, + int callingUid); + /** * Track the creator of a new isolated uid. * @param isolatedUid The newly created isolated uid. diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 889785d5f766a..64af266231cfb 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -11608,9 +11608,17 @@ public class PackageManagerService extends IPackageManager.Stub return resolveContentProviderInternal(name, flags, userId); } + public ProviderInfo resolveContentProvider(String name, int flags, int userId, int callingUid) { + return resolveContentProviderInternal(name, flags, userId, callingUid); + } + private ProviderInfo resolveContentProviderInternal(String name, int flags, int userId) { + return resolveContentProviderInternal(name, flags, userId, Binder.getCallingUid()); + } + + private ProviderInfo resolveContentProviderInternal(String name, int flags, int userId, + int callingUid) { if (!mUserManager.exists(userId)) return null; - final int callingUid = Binder.getCallingUid(); flags = updateFlagsForComponent(flags, userId); final ProviderInfo providerInfo = mComponentResolver.queryProvider(name, flags, userId); boolean checkedGrants = false; @@ -27753,6 +27761,13 @@ public class PackageManagerService extends IPackageManager.Stub name, flags, userId); } + @Override + public ProviderInfo resolveContentProvider(String name, int flags, int userId, + int callingUid) { + return PackageManagerService.this.resolveContentProviderInternal( + name, flags, userId, callingUid); + } + @Override public void addIsolatedUid(int isolatedUid, int ownerUid) { synchronized (mLock) { diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java index 44545ed4898af..4e453f378cbc2 100644 --- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java +++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java @@ -1061,6 +1061,12 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub { PackageManager.GET_URI_PERMISSION_PATTERNS | pmFlags, userHandle); } + private ProviderInfo getProviderInfo(String authority, int userHandle, int pmFlags, + int callingUid) { + return mPmInternal.resolveContentProvider(authority, + PackageManager.GET_URI_PERMISSION_PATTERNS | pmFlags, userHandle, callingUid); + } + /** * Check if the targetPkg can be granted permission to access uri by * the callingUid using the given modeFlags. Throws a security exception @@ -1106,7 +1112,7 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub { final String authority = grantUri.uri.getAuthority(); final ProviderInfo pi = getProviderInfo(authority, grantUri.sourceUserId, - MATCH_DEBUG_TRIAGED_MISSING); + MATCH_DEBUG_TRIAGED_MISSING, callingUid); if (pi == null) { Slog.w(TAG, "No content provider found for permission check: " + grantUri.uri.toSafeString()); diff --git a/services/tests/servicestests/src/com/android/server/uri/UriGrantsManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/uri/UriGrantsManagerServiceTest.java index 614949c91b9a8..69f006568c8e5 100644 --- a/services/tests/servicestests/src/com/android/server/uri/UriGrantsManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/uri/UriGrantsManagerServiceTest.java @@ -292,23 +292,29 @@ public class UriGrantsManagerServiceTest { intent.setClipData(clip); { - // When granting towards primary, persistable can't be honored so - // the entire grant fails - try { - mService.checkGrantUriPermissionFromIntent( - intent, UID_PRIMARY_CAMERA, PKG_SOCIAL, USER_PRIMARY); - fail(); - } catch (SecurityException expected) { + // The camera package shouldn't be able to see other packages or their providers, + // so make sure the grant only succeeds for the camera's URIs. + final NeededUriGrants nug = mService.checkGrantUriPermissionFromIntent( + intent, UID_PRIMARY_CAMERA, PKG_SOCIAL, USER_PRIMARY); + if (nug != null && nug.uris != null) { + for (GrantUri gu : nug.uris) { + if (!gu.uri.getAuthority().equals(PKG_CAMERA)) { + fail(); + } + } } } { - // When granting towards secondary, persistable can't be honored so - // the entire grant fails - try { - mService.checkGrantUriPermissionFromIntent( - intent, UID_PRIMARY_CAMERA, PKG_SOCIAL, USER_SECONDARY); - fail(); - } catch (SecurityException expected) { + // The camera package shouldn't be able to see other packages or their providers, + // so make sure the grant only succeeds for the camera's URIs. + final NeededUriGrants nug = mService.checkGrantUriPermissionFromIntent( + intent, UID_PRIMARY_CAMERA, PKG_SOCIAL, USER_SECONDARY); + if (nug != null && nug.uris != null) { + for (GrantUri gu : nug.uris) { + if (!gu.uri.getAuthority().equals(PKG_CAMERA)) { + fail(); + } + } } } } diff --git a/services/tests/servicestests/src/com/android/server/uri/UriGrantsMockContext.java b/services/tests/servicestests/src/com/android/server/uri/UriGrantsMockContext.java index 2c719ff9e8b3b..a6307b38f6a53 100644 --- a/services/tests/servicestests/src/com/android/server/uri/UriGrantsMockContext.java +++ b/services/tests/servicestests/src/com/android/server/uri/UriGrantsMockContext.java @@ -135,14 +135,29 @@ public class UriGrantsMockContext extends ContextWrapper { when(mPmInternal.resolveContentProvider(eq(PKG_CAMERA), anyInt(), eq(userId))) .thenReturn(buildCameraProvider(userId)); + when(mPmInternal.resolveContentProvider(eq(PKG_CAMERA), anyInt(), eq(userId), + eq(UserHandle.getUid(userId, UID_CAMERA)))) + .thenReturn(buildCameraProvider(userId)); when(mPmInternal.resolveContentProvider(eq(PKG_PRIVATE), anyInt(), eq(userId))) .thenReturn(buildPrivateProvider(userId)); + when(mPmInternal.resolveContentProvider(eq(PKG_PRIVATE), anyInt(), eq(userId), + eq(UserHandle.getUid(userId, UID_PRIVATE)))) + .thenReturn(buildPrivateProvider(userId)); when(mPmInternal.resolveContentProvider(eq(PKG_PUBLIC), anyInt(), eq(userId))) .thenReturn(buildPublicProvider(userId)); + when(mPmInternal.resolveContentProvider(eq(PKG_PUBLIC), anyInt(), eq(userId), + eq(UserHandle.getUid(userId, UID_PUBLIC)))) + .thenReturn(buildPublicProvider(userId)); when(mPmInternal.resolveContentProvider(eq(PKG_FORCE), anyInt(), eq(userId))) .thenReturn(buildForceProvider(userId)); + when(mPmInternal.resolveContentProvider(eq(PKG_FORCE), anyInt(), eq(userId), + eq(UserHandle.getUid(userId, UID_FORCE)))) + .thenReturn(buildForceProvider(userId)); when(mPmInternal.resolveContentProvider(eq(PKG_COMPLEX), anyInt(), eq(userId))) .thenReturn(buildComplexProvider(userId)); + when(mPmInternal.resolveContentProvider(eq(PKG_COMPLEX), anyInt(), eq(userId), + eq(UserHandle.getUid(userId, UID_COMPLEX)))) + .thenReturn(buildComplexProvider(userId)); } }