diff options
Diffstat (limited to 'src/com/android/providers/downloads')
6 files changed, 149 insertions, 44 deletions
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java index 2423c0d7..bee5c4a9 100644 --- a/src/com/android/providers/downloads/DownloadInfo.java +++ b/src/com/android/providers/downloads/DownloadInfo.java @@ -16,6 +16,8 @@ package com.android.providers.downloads; +import static com.android.providers.downloads.Constants.TAG; + import android.app.DownloadManager; import android.content.ContentResolver; import android.content.ContentUris; @@ -31,12 +33,14 @@ import android.os.Environment; import android.provider.Downloads; import android.provider.Downloads.Impl; import android.text.TextUtils; +import android.util.Log; import android.util.Pair; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.IndentingPrintWriter; import java.io.CharArrayWriter; +import java.io.File; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -344,7 +348,15 @@ public class DownloadInfo { return restartTime(now) <= now; case Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR: // is the media mounted? - return Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED); + final Uri uri = Uri.parse(mUri); + if (ContentResolver.SCHEME_FILE.equals(uri.getScheme())) { + final File file = new File(uri.getPath()); + return Environment.MEDIA_MOUNTED + .equals(Environment.getExternalStorageState(file)); + } else { + Log.w(TAG, "Expected file URI on external storage: " + mUri); + return false; + } case Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR: // avoids repetition of retrying download return false; diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index 87084130..13ab5455 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -16,6 +16,7 @@ package com.android.providers.downloads; +import android.app.AppOpsManager; import android.app.DownloadManager; import android.app.DownloadManager.Request; import android.content.ContentProvider; @@ -34,8 +35,8 @@ import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteOpenHelper; import android.net.Uri; import android.os.Binder; -import android.os.Environment; import android.os.Handler; +import android.os.HandlerThread; import android.os.ParcelFileDescriptor; import android.os.ParcelFileDescriptor.OnCloseListener; import android.os.Process; @@ -46,12 +47,12 @@ import android.text.TextUtils; import android.text.format.DateUtils; import android.util.Log; +import libcore.io.IoUtils; + import com.android.internal.util.IndentingPrintWriter; import com.google.android.collect.Maps; import com.google.common.annotations.VisibleForTesting; -import libcore.io.IoUtils; - import java.io.File; import java.io.FileDescriptor; import java.io.FileNotFoundException; @@ -441,7 +442,10 @@ public final class DownloadProvider extends ContentProvider { mSystemFacade = new RealSystemFacade(getContext()); } - mHandler = new Handler(); + HandlerThread handlerThread = + new HandlerThread("DownloadProvider handler", Process.THREAD_PRIORITY_BACKGROUND); + handlerThread.start(); + mHandler = new Handler(handlerThread.getLooper()); mOpenHelper = new DatabaseHelper(getContext()); // Initialize the system uid @@ -564,11 +568,19 @@ public final class DownloadProvider extends ContentProvider { dest = Downloads.Impl.DESTINATION_CACHE_PARTITION; } if (dest == Downloads.Impl.DESTINATION_FILE_URI) { - getContext().enforcePermission( - android.Manifest.permission.WRITE_EXTERNAL_STORAGE, - Binder.getCallingPid(), Binder.getCallingUid(), - "need WRITE_EXTERNAL_STORAGE permission to use DESTINATION_FILE_URI"); checkFileUriDestination(values); + + } else if (dest == Downloads.Impl.DESTINATION_EXTERNAL) { + getContext().enforceCallingOrSelfPermission( + android.Manifest.permission.WRITE_EXTERNAL_STORAGE, + "No permission to write"); + + final AppOpsManager appOps = getContext().getSystemService(AppOpsManager.class); + if (appOps.noteProxyOp(AppOpsManager.OP_WRITE_EXTERNAL_STORAGE, + getCallingPackage()) != AppOpsManager.MODE_ALLOWED) { + throw new SecurityException("No permission to write"); + } + } else if (dest == Downloads.Impl.DESTINATION_SYSTEMCACHE_PARTITION) { getContext().enforcePermission( android.Manifest.permission.ACCESS_CACHE_FILESYSTEM, @@ -716,14 +728,31 @@ public final class DownloadProvider extends ContentProvider { if (path == null) { throw new IllegalArgumentException("Invalid file URI: " + uri); } + + final File file; try { - final String canonicalPath = new File(path).getCanonicalPath(); - final String externalPath = Environment.getExternalStorageDirectory().getAbsolutePath(); - if (!canonicalPath.startsWith(externalPath)) { - throw new SecurityException("Destination must be on external storage: " + uri); - } + file = new File(path).getCanonicalFile(); } catch (IOException e) { - throw new SecurityException("Problem resolving path: " + uri); + throw new SecurityException(e); + } + + if (Helpers.isFilenameValidInExternalPackage(getContext(), file, getCallingPackage())) { + // No permissions required for paths belonging to calling package + return; + } else if (Helpers.isFilenameValidInExternal(getContext(), file)) { + // Otherwise we require write permission + getContext().enforceCallingOrSelfPermission( + android.Manifest.permission.WRITE_EXTERNAL_STORAGE, + "No permission to write to " + file); + + final AppOpsManager appOps = getContext().getSystemService(AppOpsManager.class); + if (appOps.noteProxyOp(AppOpsManager.OP_WRITE_EXTERNAL_STORAGE, + getCallingPackage()) != AppOpsManager.MODE_ALLOWED) { + throw new SecurityException("No permission to write to " + file); + } + + } else { + throw new SecurityException("Unsupported path " + file); } } @@ -1156,9 +1185,7 @@ public final class DownloadProvider extends ContentProvider { * Deletes a row in the database */ @Override - public int delete(final Uri uri, final String where, - final String[] whereArgs) { - + public int delete(final Uri uri, final String where, final String[] whereArgs) { if (shouldRestrictVisibility()) { Helpers.validateSelection(where, sAppReadableColumnsSet); } @@ -1175,13 +1202,26 @@ public final class DownloadProvider extends ContentProvider { deleteRequestHeaders(db, selection.getSelection(), selection.getParameters()); final Cursor cursor = db.query(DB_TABLE, new String[] { - Downloads.Impl._ID }, selection.getSelection(), selection.getParameters(), - null, null, null); + Downloads.Impl._ID, Downloads.Impl._DATA + }, selection.getSelection(), selection.getParameters(), null, null, null); try { while (cursor.moveToNext()) { final long id = cursor.getLong(0); revokeAllDownloadsPermission(id); DownloadStorageProvider.onDownloadProviderDelete(getContext(), id); + + final String path = cursor.getString(1); + if (!TextUtils.isEmpty(path)) { + try { + final File file = new File(path).getCanonicalFile(); + if (Helpers.isFilenameValid(getContext(), file)) { + Log.v(Constants.TAG, + "Deleting " + file + " via provider delete"); + file.delete(); + } + } catch (IOException ignored) { + } + } } } finally { IoUtils.closeQuietly(cursor); diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index 58cf380c..b0b73297 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -219,11 +219,13 @@ public class DownloadService extends Service { private boolean needToScheduleCleanup(JobScheduler js) { List<JobInfo> myJobs = js.getAllPendingJobs(); - final int N = myJobs.size(); - for (int i = 0; i < N; i++) { - if (myJobs.get(i).getId() == CLEANUP_JOB_ID) { - // It's already been (persistently) scheduled; no need to do it again - return false; + if (myJobs != null) { + final int N = myJobs.size(); + for (int i = 0; i < N; i++) { + if (myJobs.get(i).getId() == CLEANUP_JOB_ID) { + // It's already been (persistently) scheduled; no need to do it again + return false; + } } } return true; diff --git a/src/com/android/providers/downloads/DownloadStorageProvider.java b/src/com/android/providers/downloads/DownloadStorageProvider.java index 80d78551..1b5dc844 100644 --- a/src/com/android/providers/downloads/DownloadStorageProvider.java +++ b/src/com/android/providers/downloads/DownloadStorageProvider.java @@ -29,6 +29,7 @@ import android.net.Uri; import android.os.Binder; import android.os.CancellationSignal; import android.os.Environment; +import android.os.FileUtils; import android.os.ParcelFileDescriptor; import android.provider.DocumentsContract; import android.provider.DocumentsContract.Document; @@ -105,6 +106,8 @@ public class DownloadStorageProvider extends DocumentsProvider { @Override public String createDocument(String docId, String mimeType, String displayName) throws FileNotFoundException { + displayName = FileUtils.buildValidFatFilename(displayName); + if (Document.MIME_TYPE_DIR.equals(mimeType)) { throw new FileNotFoundException("Directory creation not supported"); } @@ -116,14 +119,7 @@ public class DownloadStorageProvider extends DocumentsProvider { // Delegate to real provider final long token = Binder.clearCallingIdentity(); try { - displayName = removeExtension(mimeType, displayName); - File file = new File(parent, addExtension(mimeType, displayName)); - - // If conflicting file, try adding counter suffix - int n = 0; - while (file.exists() && n++ < 32) { - file = new File(parent, addExtension(mimeType, displayName + " (" + n + ")")); - } + final File file = FileUtils.buildUniqueFile(parent, mimeType, displayName); try { if (!file.createNewFile()) { diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java index 1b4c911e..d01cbff2 100644 --- a/src/com/android/providers/downloads/Helpers.java +++ b/src/com/android/providers/downloads/Helpers.java @@ -16,6 +16,10 @@ package com.android.providers.downloads; +import static android.os.Environment.buildExternalStorageAppCacheDirs; +import static android.os.Environment.buildExternalStorageAppFilesDirs; +import static android.os.Environment.buildExternalStorageAppMediaDirs; +import static android.os.Environment.buildExternalStorageAppObbDirs; import static com.android.providers.downloads.Constants.TAG; import android.content.Context; @@ -23,6 +27,9 @@ import android.net.Uri; import android.os.Environment; import android.os.FileUtils; import android.os.SystemClock; +import android.os.UserHandle; +import android.os.storage.StorageManager; +import android.os.storage.StorageVolume; import android.provider.Downloads; import android.util.Log; import android.webkit.MimeTypeMap; @@ -334,30 +341,77 @@ public class Helpers { throw new IOException("Failed to generate an available filename"); } + static boolean isFilenameValid(Context context, File file) { + return isFilenameValid(context, file, true); + } + + static boolean isFilenameValidInExternal(Context context, File file) { + return isFilenameValid(context, file, false); + } + + /** + * Test if given file exists in one of the package-specific external storage + * directories that are always writable to apps, regardless of storage + * permission. + */ + static boolean isFilenameValidInExternalPackage(Context context, File file, + String packageName) { + try { + if (containsCanonical(buildExternalStorageAppFilesDirs(packageName), file) || + containsCanonical(buildExternalStorageAppObbDirs(packageName), file) || + containsCanonical(buildExternalStorageAppCacheDirs(packageName), file) || + containsCanonical(buildExternalStorageAppMediaDirs(packageName), file)) { + return true; + } + } catch (IOException e) { + Log.w(TAG, "Failed to resolve canonical path: " + e); + return false; + } + + Log.w(TAG, "Path appears to be invalid: " + file); + return false; + } + /** * Checks whether the filename looks legitimate for security purposes. This * prevents us from opening files that aren't actually downloads. */ - static boolean isFilenameValid(Context context, File file) { - final File[] whitelist; + static boolean isFilenameValid(Context context, File file, boolean allowInternal) { try { - whitelist = new File[] { - context.getFilesDir().getCanonicalFile(), - context.getCacheDir().getCanonicalFile(), - Environment.getDownloadCacheDirectory().getCanonicalFile(), - Environment.getExternalStorageDirectory().getCanonicalFile(), - }; + if (allowInternal) { + if (containsCanonical(context.getFilesDir(), file) + || containsCanonical(context.getCacheDir(), file) + || containsCanonical(Environment.getDownloadCacheDirectory(), file)) { + return true; + } + } + + final StorageVolume[] volumes = StorageManager.getVolumeList(UserHandle.myUserId(), + StorageManager.FLAG_FOR_WRITE); + for (StorageVolume volume : volumes) { + if (containsCanonical(volume.getPathFile(), file)) { + return true; + } + } } catch (IOException e) { Log.w(TAG, "Failed to resolve canonical path: " + e); return false; } - for (File testDir : whitelist) { - if (FileUtils.contains(testDir, file)) { + Log.w(TAG, "Path appears to be invalid: " + file); + return false; + } + + private static boolean containsCanonical(File dir, File file) throws IOException { + return FileUtils.contains(dir.getCanonicalFile(), file); + } + + private static boolean containsCanonical(File[] dirs, File file) throws IOException { + for (File dir : dirs) { + if (containsCanonical(dir, file)) { return true; } } - return false; } diff --git a/src/com/android/providers/downloads/StorageUtils.java b/src/com/android/providers/downloads/StorageUtils.java index 1817c758..3bb57c8e 100644 --- a/src/com/android/providers/downloads/StorageUtils.java +++ b/src/com/android/providers/downloads/StorageUtils.java @@ -105,6 +105,7 @@ public class StorageUtils { throw e.rethrowAsIOException(); } + // TODO: teach about evicting caches on adopted secondary storage devices final long dataDev = getDeviceId(Environment.getDataDirectory()); final long cacheDev = getDeviceId(Environment.getDownloadCacheDirectory()); final long externalDev = getDeviceId(Environment.getExternalStorageDirectory()); |