From 552ec32590308b7488038b6208446d9450063833 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Tue, 8 Aug 2023 16:19:48 -0500 Subject: RESTRICT AUTOMERGE: Check URI permissions for resumable media artwork When resumable media is added that has artwork set via URI, check the permissions for the URI before attempting to load it Test: atest MediaDataManagerTest UriGrantsManagerServiceTest Test: manual with test app Bug: 284297452 Change-Id: Ie79915d3d1712f08dc2e8dfbd5bc7fd32bb308a3 Merged-In: Ie79915d3d1712f08dc2e8dfbd5bc7fd32bb308a3 --- core/java/android/app/IUriGrantsManager.aidl | 3 + .../com/android/systemui/media/MediaDataManager.kt | 35 +++++- .../android/systemui/media/MediaDataManagerTest.kt | 135 +++++++++++++++++++-- .../server/uri/UriGrantsManagerService.java | 42 +++++++ 4 files changed, 207 insertions(+), 8 deletions(-) diff --git a/core/java/android/app/IUriGrantsManager.aidl b/core/java/android/app/IUriGrantsManager.aidl index 9e7f2fecfea0..b630d034dca9 100644 --- a/core/java/android/app/IUriGrantsManager.aidl +++ b/core/java/android/app/IUriGrantsManager.aidl @@ -39,4 +39,7 @@ interface IUriGrantsManager { void clearGrantedUriPermissions(in String packageName, int userId); ParceledListSlice getUriPermissions(in String packageName, boolean incoming, boolean persistedOnly); + + int checkGrantUriPermission_ignoreNonSystem( + int sourceUid, String targetPkg, in Uri uri, int modeFlags, int userId); } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt index 0344e8bf593c..ffa08035253f 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt @@ -18,11 +18,14 @@ package com.android.systemui.media import android.app.Notification import android.app.PendingIntent +import android.app.UriGrantsManager import android.content.BroadcastReceiver +import android.content.ContentProvider import android.content.ContentResolver import android.content.Context import android.content.Intent import android.content.IntentFilter +import android.content.pm.PackageManager import android.graphics.Bitmap import android.graphics.Canvas import android.graphics.Color @@ -34,6 +37,7 @@ import android.media.MediaMetadata import android.media.session.MediaController import android.media.session.MediaSession import android.net.Uri +import android.os.Process import android.os.UserHandle import android.service.notification.StatusBarNotification import android.text.TextUtils @@ -374,7 +378,13 @@ class MediaDataManager( // Album art var artworkBitmap = desc.iconBitmap if (artworkBitmap == null && desc.iconUri != null) { - artworkBitmap = loadBitmapFromUri(desc.iconUri!!) + val appUid = try { + context.packageManager.getApplicationInfo(packageName, 0)?.uid!! + } catch (e: PackageManager.NameNotFoundException) { + Log.w(TAG, "Could not get app UID for $packageName", e) + Process.INVALID_UID + } + artworkBitmap = loadBitmapFromUriForUser(desc.iconUri!!, userId, appUid, packageName) } val artworkIcon = if (artworkBitmap != null) { Icon.createWithBitmap(artworkBitmap) @@ -531,6 +541,29 @@ class MediaDataManager( return null } + /** Returns a bitmap if the user can access the given URI, else null */ + private fun loadBitmapFromUriForUser( + uri: Uri, + userId: Int, + appUid: Int, + packageName: String + ): Bitmap? { + try { + val ugm = UriGrantsManager.getService() + ugm.checkGrantUriPermission_ignoreNonSystem( + appUid, + packageName, + ContentProvider.getUriWithoutUserId(uri), + Intent.FLAG_GRANT_READ_URI_PERMISSION, + ContentProvider.getUserIdFromUri(uri, userId) + ) + return loadBitmapFromUri(uri) + } catch (e: SecurityException) { + Log.e(TAG, "Failed to get URI permission: $e") + } + return null + } + /** * Load a bitmap from a URI * @param uri the uri to load diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt index 780e64af3b4d..37003d178c67 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt @@ -1,16 +1,21 @@ package com.android.systemui.media +import android.app.IUriGrantsManager import android.app.Notification.MediaStyle import android.app.PendingIntent +import android.app.UriGrantsManager import android.graphics.Bitmap +import android.graphics.ImageDecoder import android.media.MediaDescription import android.media.MediaMetadata import android.media.session.MediaController import android.media.session.MediaSession +import android.net.Uri import android.service.notification.StatusBarNotification import android.testing.AndroidTestingRunner import android.testing.TestableLooper.RunWithLooper import androidx.test.filters.SmallTest +import com.android.dx.mockito.inline.extended.ExtendedMockito import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.broadcast.BroadcastDispatcher @@ -27,6 +32,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatchers.anyInt import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito @@ -36,6 +42,7 @@ import org.mockito.Mockito.reset import org.mockito.Mockito.verify import org.mockito.junit.MockitoJUnit import org.mockito.Mockito.`when` as whenever +import org.mockito.quality.Strictness private const val KEY = "KEY" private const val KEY_2 = "KEY_2" @@ -76,6 +83,8 @@ class MediaDataManagerTest : SysuiTestCase() { lateinit var mediaDataManager: MediaDataManager lateinit var mediaNotification: StatusBarNotification @Captor lateinit var mediaDataCaptor: ArgumentCaptor + @Mock private lateinit var ugm: IUriGrantsManager + @Mock private lateinit var imageSource: ImageDecoder.Source @Before fun setup() { @@ -126,13 +135,10 @@ class MediaDataManagerTest : SysuiTestCase() { } @Test - fun testSetTimedOut_deactivatesMedia() { - val data = MediaData(userId = USER_ID, initialized = true, backgroundColor = 0, app = null, - appIcon = null, artist = null, song = null, artwork = null, actions = emptyList(), - actionsToShowInCompact = emptyList(), packageName = "INVALID", token = null, - clickIntent = null, device = null, active = true, resumeAction = null) - mediaDataManager.onNotificationAdded(KEY, mediaNotification) - mediaDataManager.onMediaDataLoaded(KEY, oldKey = null, data = data) + fun testSetTimedOut_active_deactivatesMedia() { + addNotificationAndLoad() + val data = mediaDataCaptor.value + assertThat(data.active).isTrue() mediaDataManager.setTimedOut(KEY, timedOut = true) assertThat(data.active).isFalse() @@ -361,4 +367,119 @@ class MediaDataManagerTest : SysuiTestCase() { verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor)) assertThat(mediaDataCaptor.value!!.backgroundColor).isEqualTo(DEFAULT_COLOR) } + + @Test + fun testResumeMediaLoaded_hasArtPermission_artLoaded() { + // When resume media is loaded and user/app has permission to access the art URI, + var mockSession = ExtendedMockito.mockitoSession() + .mockStatic(UriGrantsManager::class.java) + .mockStatic(ImageDecoder::class.java) + .strictness(Strictness.LENIENT) + .startMocking() + try { + whenever(UriGrantsManager.getService()).thenReturn(ugm) + whenever( + ugm.checkGrantUriPermission_ignoreNonSystem( + anyInt(), + anyObject(), + anyObject(), + anyInt(), + anyInt() + ) + ) + .thenReturn(1) + val artwork = Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888) + val uri = Uri.parse("content://example") + whenever(ImageDecoder.createSource(anyObject(), eq(uri))).thenReturn(imageSource) + whenever(ImageDecoder.decodeBitmap(anyObject(), anyObject())).thenReturn(artwork) + + val desc = + MediaDescription.Builder().run { + setTitle(SESSION_TITLE) + setIconUri(uri) + build() + } + addResumeControlAndLoad(desc) + + // Then the artwork is loaded + assertThat(mediaDataCaptor.value.artwork).isNotNull() + } finally { + mockSession.finishMocking() + } + } + + @Test + fun testResumeMediaLoaded_noArtPermission_noArtLoaded() { + // When resume media is loaded and user/app does not have permission to access the art URI + var mockSession = ExtendedMockito.mockitoSession() + .mockStatic(UriGrantsManager::class.java) + .mockStatic(ImageDecoder::class.java) + .strictness(Strictness.LENIENT) + .startMocking() + try { + whenever(UriGrantsManager.getService()).thenReturn(ugm) + whenever( + ugm.checkGrantUriPermission_ignoreNonSystem( + anyInt(), + anyObject(), + anyObject(), + anyInt(), + anyInt() + ) + ) + .thenThrow(SecurityException("Test no permission")) + val artwork = Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888) + val uri = Uri.parse("content://example") + whenever(ImageDecoder.createSource(anyObject(), eq(uri))).thenReturn(imageSource) + whenever(ImageDecoder.decodeBitmap(anyObject(), anyObject())).thenReturn(artwork) + + val desc = + MediaDescription.Builder().run { + setTitle(SESSION_TITLE) + setIconUri(uri) + build() + } + addResumeControlAndLoad(desc) + + // Then the artwork is not loaded + assertThat(mediaDataCaptor.value.artwork).isNull() + } finally { + mockSession.finishMocking() + } + } + + /** + * Helper function to add a media notification and capture the resulting MediaData + */ + private fun addNotificationAndLoad() { + mediaDataManager.onNotificationAdded(KEY, mediaNotification) + assertThat(backgroundExecutor.runAllReady()).isEqualTo(1) + assertThat(foregroundExecutor.runAllReady()).isEqualTo(1) + verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor)) + } + + /** Helper function to add a resumption control and capture the resulting MediaData */ + private fun addResumeControlAndLoad( + desc: MediaDescription, + packageName: String = PACKAGE_NAME + ) { + mediaDataManager.addResumptionControls( + USER_ID, + desc, + Runnable {}, + session.sessionToken, + APP_NAME, + pendingIntent, + packageName + ) + assertThat(backgroundExecutor.runAllReady()).isEqualTo(1) + assertThat(foregroundExecutor.runAllReady()).isEqualTo(1) + + verify(listener) + .onMediaDataLoaded( + eq(packageName), + eq(null), + capture(mediaDataCaptor) + ) + } } diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java index e8a0379473da..0871497e4143 100644 --- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java +++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java @@ -45,6 +45,7 @@ import static org.xmlpull.v1.XmlPullParser.END_DOCUMENT; import static org.xmlpull.v1.XmlPullParser.START_TAG; import android.annotation.Nullable; +import android.annotation.RequiresPermission; import android.app.ActivityManager; import android.app.ActivityManagerInternal; import android.app.AppGlobals; @@ -70,6 +71,7 @@ import android.os.Handler; import android.os.IBinder; import android.os.Looper; import android.os.Message; +import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; import android.os.UserHandle; @@ -1331,6 +1333,46 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub { return false; } + /** + * Check if the targetPkg can be granted permission to access uri by + * the callingUid using the given modeFlags. See {@link #checkGrantUriPermissionUnlocked}. + * + * @param callingUid The uid of the grantor app that has permissions to the uri. + * @param targetPkg The package name of the granted app that needs permissions to the uri. + * @param uri The uri for which permissions should be granted. + * @param modeFlags The modes to grant. See {@link Intent#FLAG_GRANT_READ_URI_PERMISSION}, etc. + * @param userId The userId in which the uri is to be resolved. + * @return uid of the target or -1 if permission grant not required. Returns -1 if the caller + * does not hold INTERACT_ACROSS_USERS_FULL + * @throws SecurityException if the grant is not allowed. + */ + @Override + @RequiresPermission(android.Manifest.permission.INTERACT_ACROSS_USERS_FULL) + public int checkGrantUriPermission_ignoreNonSystem(int callingUid, String targetPkg, Uri uri, + int modeFlags, int userId) { + if (!isCallerIsSystemOrPrivileged()) { + return Process.INVALID_UID; + } + final long origId = Binder.clearCallingIdentity(); + try { + return checkGrantUriPermissionUnlocked(callingUid, targetPkg, uri, modeFlags, + userId); + } finally { + Binder.restoreCallingIdentity(origId); + } + } + + private boolean isCallerIsSystemOrPrivileged() { + final int uid = Binder.getCallingUid(); + if (uid == Process.SYSTEM_UID || uid == Process.ROOT_UID) { + return true; + } + return ActivityManager.checkComponentPermission( + android.Manifest.permission.INTERACT_ACROSS_USERS_FULL, + uid, /* owningUid = */-1, /* exported = */ true) + == PackageManager.PERMISSION_GRANTED; + } + @GuardedBy("mLock") private void writeGrantedUriPermissionsLocked() { if (DEBUG) Slog.v(TAG, "writeGrantedUriPermissions()"); -- cgit v1.2.3-59-g8ed1b