From 8a2998eade93032a78d681c66ebadbfa6f802f76 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 31 Oct 2013 14:55:44 -0700 Subject: [PATCH] Better enforcement in DocumentsProvider.call(). Use ContentProvider.enforceWritePermissionInner() to handle all edge cases around checking if caller has write permissions. This fixes bug where call() would throw if caller and provider were the same app. Bug: 11464234 Change-Id: Iace8e0e4243d56ed1cdcc9680383103975107036 --- .../java/android/content/ContentProvider.java | 224 +++++++++--------- .../android/provider/DocumentsProvider.java | 5 +- 2 files changed, 114 insertions(+), 115 deletions(-) diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java index a9d055988862e..ddde3fb00d69d 100644 --- a/core/java/android/content/ContentProvider.java +++ b/core/java/android/content/ContentProvider.java @@ -398,67 +398,6 @@ public abstract class ContentProvider implements ComponentCallbacks2 { return AppOpsManager.MODE_ALLOWED; } - private void enforceReadPermissionInner(Uri uri) throws SecurityException { - final Context context = getContext(); - final int pid = Binder.getCallingPid(); - final int uid = Binder.getCallingUid(); - String missingPerm = null; - - if (UserHandle.isSameApp(uid, mMyUid)) { - return; - } - - if (mExported) { - final String componentPerm = getReadPermission(); - if (componentPerm != null) { - if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) { - return; - } else { - missingPerm = componentPerm; - } - } - - // track if unprotected read is allowed; any denied - // below removes this ability - boolean allowDefaultRead = (componentPerm == null); - - final PathPermission[] pps = getPathPermissions(); - if (pps != null) { - final String path = uri.getPath(); - for (PathPermission pp : pps) { - final String pathPerm = pp.getReadPermission(); - if (pathPerm != null && pp.match(path)) { - if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) { - return; - } else { - // any denied means we lose - // default access. - allowDefaultRead = false; - missingPerm = pathPerm; - } - } - } - } - - // if we passed checks above, and no default - // permission, then allow access. - if (allowDefaultRead) return; - } - - // last chance, check against any uri grants - if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION) - == PERMISSION_GRANTED) { - return; - } - - final String failReason = mExported - ? " requires " + missingPerm + ", or grantUriPermission()" - : " requires the provider be exported, or grantUriPermission()"; - throw new SecurityException("Permission Denial: reading " - + ContentProvider.this.getClass().getName() + " uri " + uri + " from pid=" + pid - + ", uid=" + uid + failReason); - } - private int enforceWritePermission(String callingPkg, Uri uri) throws SecurityException { enforceWritePermissionInner(uri); if (mWriteOp != AppOpsManager.OP_NONE) { @@ -466,67 +405,130 @@ public abstract class ContentProvider implements ComponentCallbacks2 { } return AppOpsManager.MODE_ALLOWED; } + } - private void enforceWritePermissionInner(Uri uri) throws SecurityException { - final Context context = getContext(); - final int pid = Binder.getCallingPid(); - final int uid = Binder.getCallingUid(); - String missingPerm = null; + /** {@hide} */ + protected void enforceReadPermissionInner(Uri uri) throws SecurityException { + final Context context = getContext(); + final int pid = Binder.getCallingPid(); + final int uid = Binder.getCallingUid(); + String missingPerm = null; - if (UserHandle.isSameApp(uid, mMyUid)) { - return; + if (UserHandle.isSameApp(uid, mMyUid)) { + return; + } + + if (mExported) { + final String componentPerm = getReadPermission(); + if (componentPerm != null) { + if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) { + return; + } else { + missingPerm = componentPerm; + } } - if (mExported) { - final String componentPerm = getWritePermission(); - if (componentPerm != null) { - if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) { - return; - } else { - missingPerm = componentPerm; - } - } + // track if unprotected read is allowed; any denied + // below removes this ability + boolean allowDefaultRead = (componentPerm == null); - // track if unprotected write is allowed; any denied - // below removes this ability - boolean allowDefaultWrite = (componentPerm == null); - - final PathPermission[] pps = getPathPermissions(); - if (pps != null) { - final String path = uri.getPath(); - for (PathPermission pp : pps) { - final String pathPerm = pp.getWritePermission(); - if (pathPerm != null && pp.match(path)) { - if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) { - return; - } else { - // any denied means we lose - // default access. - allowDefaultWrite = false; - missingPerm = pathPerm; - } + final PathPermission[] pps = getPathPermissions(); + if (pps != null) { + final String path = uri.getPath(); + for (PathPermission pp : pps) { + final String pathPerm = pp.getReadPermission(); + if (pathPerm != null && pp.match(path)) { + if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) { + return; + } else { + // any denied means we lose + // default access. + allowDefaultRead = false; + missingPerm = pathPerm; } } } - - // if we passed checks above, and no default - // permission, then allow access. - if (allowDefaultWrite) return; } - // last chance, check against any uri grants - if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_WRITE_URI_PERMISSION) - == PERMISSION_GRANTED) { - return; - } - - final String failReason = mExported - ? " requires " + missingPerm + ", or grantUriPermission()" - : " requires the provider be exported, or grantUriPermission()"; - throw new SecurityException("Permission Denial: writing " - + ContentProvider.this.getClass().getName() + " uri " + uri + " from pid=" + pid - + ", uid=" + uid + failReason); + // if we passed checks above, and no default + // permission, then allow access. + if (allowDefaultRead) return; } + + // last chance, check against any uri grants + if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION) + == PERMISSION_GRANTED) { + return; + } + + final String failReason = mExported + ? " requires " + missingPerm + ", or grantUriPermission()" + : " requires the provider be exported, or grantUriPermission()"; + throw new SecurityException("Permission Denial: reading " + + ContentProvider.this.getClass().getName() + " uri " + uri + " from pid=" + pid + + ", uid=" + uid + failReason); + } + + /** {@hide} */ + protected void enforceWritePermissionInner(Uri uri) throws SecurityException { + final Context context = getContext(); + final int pid = Binder.getCallingPid(); + final int uid = Binder.getCallingUid(); + String missingPerm = null; + + if (UserHandle.isSameApp(uid, mMyUid)) { + return; + } + + if (mExported) { + final String componentPerm = getWritePermission(); + if (componentPerm != null) { + if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) { + return; + } else { + missingPerm = componentPerm; + } + } + + // track if unprotected write is allowed; any denied + // below removes this ability + boolean allowDefaultWrite = (componentPerm == null); + + final PathPermission[] pps = getPathPermissions(); + if (pps != null) { + final String path = uri.getPath(); + for (PathPermission pp : pps) { + final String pathPerm = pp.getWritePermission(); + if (pathPerm != null && pp.match(path)) { + if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) { + return; + } else { + // any denied means we lose + // default access. + allowDefaultWrite = false; + missingPerm = pathPerm; + } + } + } + } + + // if we passed checks above, and no default + // permission, then allow access. + if (allowDefaultWrite) return; + } + + // last chance, check against any uri grants + if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_WRITE_URI_PERMISSION) + == PERMISSION_GRANTED) { + return; + } + + final String failReason = mExported + ? " requires " + missingPerm + ", or grantUriPermission()" + : " requires the provider be exported, or grantUriPermission()"; + throw new SecurityException("Permission Denial: writing " + + ContentProvider.this.getClass().getName() + " uri " + uri + " from pid=" + pid + + ", uid=" + uid + failReason); } /** diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java index e35b8eb425280..25d6f5b412b14 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -512,10 +512,7 @@ public abstract class DocumentsProvider extends ContentProvider { final boolean callerHasManage = context.checkCallingOrSelfPermission(android.Manifest.permission.MANAGE_DOCUMENTS) == PackageManager.PERMISSION_GRANTED; - if (!callerHasManage) { - getContext().enforceCallingOrSelfUriPermission( - documentUri, Intent.FLAG_GRANT_WRITE_URI_PERMISSION, method); - } + enforceWritePermissionInner(documentUri); final Bundle out = new Bundle(); try {