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

diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java
index a9d0559..ddde3fb 100644 (file)
--- a/core/java/android/content/ContentProvider.java
+++ b/core/java/android/content/ContentProvider.java
@@ -398,135 +398,137 @@ 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;
+        private int enforceWritePermission(String callingPkg, Uri uri) throws SecurityException {
+            enforceWritePermissionInner(uri);
+            if (mWriteOp != AppOpsManager.OP_NONE) {
+                return mAppOpsManager.noteOp(mWriteOp, Binder.getCallingUid(), callingPkg);
             }
+            return AppOpsManager.MODE_ALLOWED;
+        }
+    }
 
-            if (mExported) {
-                final String componentPerm = getReadPermission();
-                if (componentPerm != null) {
-                    if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) {
-                        return;
-                    } else {
-                        missingPerm = componentPerm;
-                    }
+    /** {@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 (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
-                // <path-permission> 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 <path-permission> means we lose
-                                // default <provider> access.
-                                allowDefaultRead = false;
-                                missingPerm = pathPerm;
-                            }
+            // track if unprotected read is allowed; any denied
+            // <path-permission> 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 <path-permission> means we lose
+                            // default <provider> access.
+                            allowDefaultRead = false;
+                            missingPerm = pathPerm;
                         }
                     }
                 }
-
-                // if we passed <path-permission> checks above, and no default
-                // <provider> 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);
+            // if we passed <path-permission> checks above, and no default
+            // <provider> permission, then allow access.
+            if (allowDefaultRead) return;
         }
 
-        private int enforceWritePermission(String callingPkg, Uri uri) throws SecurityException {
-            enforceWritePermissionInner(uri);
-            if (mWriteOp != AppOpsManager.OP_NONE) {
-                return mAppOpsManager.noteOp(mWriteOp, Binder.getCallingUid(), callingPkg);
-            }
-            return AppOpsManager.MODE_ALLOWED;
+        // last chance, check against any uri grants
+        if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION)
+                == PERMISSION_GRANTED) {
+            return;
         }
 
-        private void enforceWritePermissionInner(Uri uri) throws SecurityException {
-            final Context context = getContext();
-            final int pid = Binder.getCallingPid();
-            final int uid = Binder.getCallingUid();
-            String missingPerm = null;
+        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);
+    }
 
-            if (UserHandle.isSameApp(uid, mMyUid)) {
-                return;
-            }
+    /** {@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 (mExported) {
-                final String componentPerm = getWritePermission();
-                if (componentPerm != null) {
-                    if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) {
-                        return;
-                    } else {
-                        missingPerm = componentPerm;
-                    }
+        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
-                // <path-permission> 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 <path-permission> means we lose
-                                // default <provider> access.
-                                allowDefaultWrite = false;
-                                missingPerm = pathPerm;
-                            }
+            // track if unprotected write is allowed; any denied
+            // <path-permission> 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 <path-permission> means we lose
+                            // default <provider> access.
+                            allowDefaultWrite = false;
+                            missingPerm = pathPerm;
                         }
                     }
                 }
-
-                // if we passed <path-permission> checks above, and no default
-                // <provider> 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;
-            }
+            // if we passed <path-permission> checks above, and no default
+            // <provider> permission, then allow access.
+            if (allowDefaultWrite) 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);
+        // 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 e35b8eb..25d6f5b 100644 (file)
--- 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 {