Make COLUMN_URI readable and tighten UID restrictions.
Steve Howard [Thu, 22 Jul 2010 02:41:15 +0000 (19:41 -0700)]
I need to make COLUMN_URI readable by apps, since the public API
exposes that field.  In order to avoid any possible security issues, I
got rid of the feature that potentially allowed apps to view downloads
from other UIDs.  No one was using that feature and the public API
exposes no such feature (yet).

While at it, I cleaned up some related code in update() and delete().

Change-Id: I5384115d2a865255d009fbe37449488fd2269389

AndroidManifest.xml
src/com/android/providers/downloads/DownloadProvider.java

index 1b111f0..374b985 100644 (file)
         android:description="@string/permdesc_downloadCompletedIntent"
         android:protectionLevel="signature" />
 
-    <permission android:name="android.permission.SEE_ALL_EXTERNAL"
-        android:label="@string/permlab_seeAllExternal"
-        android:description="@string/permdesc_seeAllExternal"
-        android:protectionLevel="normal"/>
-
     <uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED" />
     <uses-permission android:name="android.permission.ACCESS_DOWNLOAD_MANAGER" />
     <uses-permission android:name="android.permission.ACCESS_DRM" />
index bb205ad..e543c44 100644 (file)
@@ -96,6 +96,7 @@ public final class DownloadProvider extends ContentProvider {
         Downloads.Impl.COLUMN_CURRENT_BYTES,
         Downloads.Impl.COLUMN_TITLE,
         Downloads.Impl.COLUMN_DESCRIPTION,
+        Downloads.Impl.COLUMN_URI,
     };
 
     private static HashSet<String> sAppReadableColumnsSet;
@@ -481,40 +482,21 @@ public final class DownloadProvider extends ContentProvider {
         }
 
         if (shouldRestrictVisibility()) {
-            boolean canSeeAllExternal;
             if (projection == null) {
                 projection = sAppReadableColumnsArray;
-                // sAppReadableColumnsArray includes _DATA, which is not allowed
-                // to be seen except by the initiating application
-                canSeeAllExternal = false;
             } else {
-                canSeeAllExternal = getContext().checkCallingPermission(
-                        Downloads.Impl.PERMISSION_SEE_ALL_EXTERNAL)
-                        == PackageManager.PERMISSION_GRANTED;
                 for (int i = 0; i < projection.length; ++i) {
                     if (!sAppReadableColumnsSet.contains(projection[i])) {
                         throw new IllegalArgumentException(
                                 "column " + projection[i] + " is not allowed in queries");
                     }
-                    canSeeAllExternal = canSeeAllExternal
-                            && !projection[i].equals(Downloads.Impl._DATA);
                 }
             }
             if (!emptyWhere) {
                 qb.appendWhere(" AND ");
                 emptyWhere = false;
             }
-            String validUid = "( " + Constants.UID + "="
-                    + Binder.getCallingUid() + " OR "
-                    + Downloads.Impl.COLUMN_OTHER_UID + "="
-                    + Binder.getCallingUid() + " )";
-            if (canSeeAllExternal) {
-                qb.appendWhere("( " + validUid + " OR "
-                        + Downloads.Impl.DESTINATION_EXTERNAL + " = "
-                        + Downloads.Impl.COLUMN_DESTINATION + " )");
-            } else {
-                qb.appendWhere(validUid);
-            }
+            qb.appendWhere(getRestrictedUidClause());
         }
 
         if (Constants.LOGVV) {
@@ -637,7 +619,7 @@ public final class DownloadProvider extends ContentProvider {
     }
 
     /**
-     * @return true if we should restrict this call to viewing only its own downloads
+     * @return true if we should restrict this caller to viewing only its own downloads
      */
     private boolean shouldRestrictVisibility() {
         int callingUid = Binder.getCallingUid();
@@ -648,6 +630,14 @@ public final class DownloadProvider extends ContentProvider {
     }
 
     /**
+     * @return a SQL WHERE clause to restrict the query to downloads accessible to the caller's UID
+     */
+    private String getRestrictedUidClause() {
+        return "( " + Constants.UID + "=" +  Binder.getCallingUid() + " OR "
+                + Downloads.Impl.COLUMN_OTHER_UID + "=" +  Binder.getCallingUid() + " )";
+    }
+
+    /**
      * Updates a row in the database
      */
     @Override
@@ -707,12 +697,8 @@ public final class DownloadProvider extends ContentProvider {
                     rowId = Long.parseLong(segment);
                     myWhere += " ( " + Downloads.Impl._ID + " = " + rowId + " ) ";
                 }
-                int callingUid = Binder.getCallingUid();
-                if (Binder.getCallingPid() != Process.myPid() &&
-                        callingUid != mSystemUid &&
-                        callingUid != mDefContainerUid) {
-                    myWhere += " AND ( " + Constants.UID + "=" +  Binder.getCallingUid() + " OR "
-                            + Downloads.Impl.COLUMN_OTHER_UID + "=" +  Binder.getCallingUid() + " )";
+                if (shouldRestrictVisibility()) {
+                    myWhere += " AND " + getRestrictedUidClause();
                 }
                 if (filteredValues.size() > 0) {
                     count = db.update(DB_TABLE, filteredValues, myWhere, whereArgs);
@@ -766,13 +752,8 @@ public final class DownloadProvider extends ContentProvider {
                     long rowId = Long.parseLong(segment);
                     myWhere += " ( " + Downloads.Impl._ID + " = " + rowId + " ) ";
                 }
-                int callingUid = Binder.getCallingUid();
-                if (Binder.getCallingPid() != Process.myPid() &&
-                        callingUid != mSystemUid &&
-                        callingUid != mDefContainerUid) {
-                    myWhere += " AND ( " + Constants.UID + "=" +  Binder.getCallingUid() + " OR "
-                            + Downloads.Impl.COLUMN_OTHER_UID + "="
-                            +  Binder.getCallingUid() + " )";
+                if (shouldRestrictVisibility()) {
+                    myWhere += " AND " + getRestrictedUidClause();
                 }
                 deleteRequestHeaders(db, where, whereArgs);
                 count = db.delete(DB_TABLE, myWhere, whereArgs);