From ea58c20339de60fbbc50945f14cd8a7bbfd86645 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Mon, 3 Feb 2020 12:52:49 -0500 Subject: Protect against bad uris Test: atest Fixes: 148260893 Change-Id: I0b7663a674689ef957c81c6ba55c4b90466bcd75 --- .../notification/NotificationManagerService.java | 4 +++- .../NotificationManagerServiceTest.java | 28 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index f6276fbf21a1..bcbc0e495b39 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -7594,7 +7594,7 @@ public class NotificationManagerService extends SystemService { for (int i = 0; i < newUris.size(); i++) { final Uri uri = newUris.valueAt(i); if (oldUris == null || !oldUris.contains(uri)) { - if (DBG) Slog.d(TAG, key + ": granting " + uri); + Slog.d(TAG, key + ": granting " + uri); grantUriPermission(permissionOwner, uri, newRecord.getUid(), targetPkg, targetUserId); } @@ -7631,6 +7631,8 @@ public class NotificationManagerService extends SystemService { targetUserId); } catch (RemoteException ignored) { // Ignored because we're in same process + } catch (SecurityException e) { + Slog.e(TAG, "Cannot grant uri access; " + sourceUid + " does not own " + uri); } finally { Binder.restoreCallingIdentity(ident); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index ad5be43e11e6..e0ee3ce3aa57 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -70,6 +70,7 @@ import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -3643,6 +3644,33 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { anyInt(), anyInt()); } + @Test + public void updateUriPermissions_posterDoesNotOwnUri() throws Exception { + NotificationChannel c = new NotificationChannel( + TEST_CHANNEL_ID, TEST_CHANNEL_ID, IMPORTANCE_DEFAULT); + c.setSound(null, Notification.AUDIO_ATTRIBUTES_DEFAULT); + Message message1 = new Message("", 0, ""); + message1.setData("", + ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1)); + + Notification.Builder nbA = new Notification.Builder(mContext, c.getId()) + .setContentTitle("foo") + .setSmallIcon(android.R.drawable.sym_def_app_icon) + .setStyle(new Notification.MessagingStyle("") + .addMessage(message1)); + NotificationRecord recordA = new NotificationRecord(mContext, new StatusBarNotification( + PKG, PKG, 0, "tag", mUid, 0, nbA.build(), new UserHandle(mUid), null, 0), c); + + doThrow(new SecurityException("no access")).when(mUgm) + .grantUriPermissionFromOwner( + any(), anyInt(), any(), any(), anyInt(), anyInt(), anyInt()); + + when(mUgmInternal.newUriPermissionOwner(any())).thenReturn(new Binder()); + mService.updateUriPermissions(recordA, null, mContext.getPackageName(), USER_SYSTEM); + + // yay, no crash + } + @Test public void testVisitUris() throws Exception { final Uri audioContents = Uri.parse("content://com.example/audio"); -- cgit v1.2.3-59-g8ed1b