From 0d8d89105c00edbad95a268aaae65f2ff94ed5a1 Mon Sep 17 00:00:00 2001
From: Steve Howard <showard@google.com>
Date: Wed, 21 Jul 2010 19:41:15 -0700
Subject: Make COLUMN_URI readable and tighten UID restrictions.

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
---
 .../providers/downloads/DownloadProvider.java      | 49 +++++++---------------
 1 file changed, 15 insertions(+), 34 deletions(-)

(limited to 'src')

diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index bb205ad4..e543c443 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -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();
@@ -647,6 +629,14 @@ public final class DownloadProvider extends ContentProvider {
                 Process.supportsProcesses();
     }
 
+    /**
+     * @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
      */
@@ -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);
-- 
cgit v1.2.3