From b61338ecb307a2f595a8f78e290b5287a6503aaa Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 21 Nov 2016 10:33:54 -0700 Subject: [PATCH] DO NOT MERGE. No direct Uri grants from system. The system should never be extending Uri permission grants from itself, since it automatically holds all the permissions. Instead, the system should always be a mediator between two specific app, and it should be using startActivityAsCaller() if it needs to extend permissions. Blocking at this level fixes an entire class of confused deputy security issues. Test: builds, normal intent resolution UI works Bug: 33019296, 32990341, 32879915, 32879772 Change-Id: Iaa57c393a386d8068e807d0dd0caccc89d8a11db --- .../core/java/com/android/server/am/ActivityManagerService.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index bc9d58eec3b1..1fe3ee2d8e12 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -8150,7 +8150,12 @@ public final class ActivityManagerService extends ActivityManagerNative // Third... does the caller itself have permission to access // this uri? - if (UserHandle.getAppId(callingUid) != Process.SYSTEM_UID) { + final int callingAppId = UserHandle.getAppId(callingUid); + if ((callingAppId == Process.SYSTEM_UID) || (callingAppId == Process.ROOT_UID)) { + Slog.w(TAG, "For security reasons, the system cannot issue a Uri permission" + + " grant to " + grantUri + "; use startActivityAsCaller() instead"); + return -1; + } else { if (!checkHoldingPermissionsLocked(pm, pi, grantUri, callingUid, modeFlags)) { // Require they hold a strong enough Uri permission if (!checkUriPermissionLocked(grantUri, callingUid, modeFlags)) { -- 2.11.0