Commit 243e6294 authored by Jeff Sharkey's avatar Jeff Sharkey Committed by Adam Seaton
Browse files

Enforce calling identity before clearing.

Fix merge conflict into nyc-release
When opening a downloaded file, enforce that the caller can actually
see the requested download before clearing their identity to read
internal columns.

However, this means that we can no longer return the "my_downloads"
paths: if those Uris were shared beyond the app that requested the
download, access would be denied.  Instead, we need to switch to
using "all_downloads" Uris so that permission grants can be issued
to third-party viewer apps.

Since an app requesting a download doesn't normally have permission
to "all_downloads" paths, we issue narrow grants toward the owner of
each download, both at device boot and when new downloads are
started.

Bug: 30537115, 30945409
Change-Id: If944aada020878a91c363963728d0da9f6fae3ea
parent 73c8b92e
......@@ -468,6 +468,19 @@ public final class DownloadProvider extends ContentProvider {
if (appInfo != null) {
mDefContainerUid = appInfo.uid;
}
// Grant access permissions for all known downloads to the owning apps
final SQLiteDatabase db = mOpenHelper.getReadableDatabase();
final Cursor cursor = db.query(DB_TABLE, new String[] {
Downloads.Impl._ID, Constants.UID }, null, null, null, null, null);
try {
while (cursor.moveToNext()) {
grantAllDownloadsPermission(cursor.getLong(0), cursor.getInt(1));
}
} finally {
cursor.close();
}
return true;
}
......@@ -690,6 +703,7 @@ public final class DownloadProvider extends ContentProvider {
}
insertRequestHeaders(db, rowID, values);
grantAllDownloadsPermission(rowID, Binder.getCallingUid());
notifyContentChanged(uri, match);
final long token = Binder.clearCallingIdentity();
......@@ -1211,7 +1225,7 @@ public final class DownloadProvider extends ContentProvider {
while (cursor.moveToNext()) {
final long id = cursor.getLong(0);
scheduler.cancel((int) id);
revokeAllDownloadsPermission(id);
DownloadStorageProvider.onDownloadProviderDelete(getContext(), id);
final String path = cursor.getString(1);
......@@ -1260,6 +1274,19 @@ public final class DownloadProvider extends ContentProvider {
logVerboseOpenFileInfo(uri, mode);
}
// Perform normal query to enforce caller identity access before
// clearing it to reach internal-only columns
final Cursor probeCursor = query(uri, new String[] {
Downloads.Impl._DATA }, null, null, null);
try {
if ((probeCursor == null) || (probeCursor.getCount() == 0)) {
throw new FileNotFoundException(
"No file found for " + uri + " as UID " + Binder.getCallingUid());
}
} finally {
IoUtils.closeQuietly(probeCursor);
}
final Cursor cursor = queryCleared(uri, new String[] {
Downloads.Impl._DATA, Downloads.Impl.COLUMN_STATUS,
Downloads.Impl.COLUMN_DESTINATION, Downloads.Impl.COLUMN_MEDIA_SCANNED }, null,
......@@ -1442,4 +1469,20 @@ public final class DownloadProvider extends ContentProvider {
to.put(key, defaultValue);
}
}
private void grantAllDownloadsPermission(long id, int uid) {
final String[] packageNames = getContext().getPackageManager().getPackagesForUid(uid);
if (packageNames == null || packageNames.length == 0) return;
// We only need to grant to the first package, since the
// platform internally tracks based on UIDs
final Uri uri = ContentUris.withAppendedId(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id);
getContext().grantUriPermission(packageNames[0], uri,
Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
}
private void revokeAllDownloadsPermission(long id) {
final Uri uri = ContentUris.withAppendedId(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id);
getContext().revokeUriPermission(uri, ~0);
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment