summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/com/android/providers/downloads/DownloadProvider.java12
-rw-r--r--src/com/android/providers/downloads/Helpers.java62
2 files changed, 12 insertions, 62 deletions
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 1595dfa9..ad3cf7ac 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -1205,16 +1205,8 @@ public final class DownloadProvider extends ContentProvider {
if (path == null) {
throw new FileNotFoundException("No filename found.");
}
-
- final File file;
- try {
- file = new File(path).getCanonicalFile();
- } catch (IOException e) {
- throw new FileNotFoundException(e.getMessage());
- }
-
- if (!Helpers.isFilenameValid(getContext(), file)) {
- throw new FileNotFoundException("Invalid file path: " + file);
+ if (!Helpers.isFilenameValid(path, mDownloadsDataDir)) {
+ throw new FileNotFoundException("Invalid filename: " + path);
}
final File file = new File(path);
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 61a49a2a..013faf27 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -341,25 +341,24 @@ public class Helpers {
}
/**
- * Checks whether the filename looks legitimate for security purposes. This
- * prevents us from opening files that aren't actually downloads.
+ * Checks whether the filename looks legitimate
*/
- static boolean isFilenameValid(Context context, File file) {
- final File[] whitelist;
+ static boolean isFilenameValid(String filename, File downloadsDataDir) {
+ final String[] whitelist;
try {
- whitelist = new File[] {
- context.getFilesDir().getCanonicalFile(),
- context.getCacheDir().getCanonicalFile(),
- Environment.getDownloadCacheDirectory().getCanonicalFile(),
- Environment.getExternalStorageDirectory().getCanonicalFile(),
+ filename = new File(filename).getCanonicalPath();
+ whitelist = new String[] {
+ downloadsDataDir.getCanonicalPath(),
+ Environment.getDownloadCacheDirectory().getCanonicalPath(),
+ Environment.getExternalStorageDirectory().getCanonicalPath(),
};
} catch (IOException e) {
Log.w(TAG, "Failed to resolve canonical path: " + e);
return false;
}
- for (File testDir : whitelist) {
- if (contains(testDir, file)) {
+ for (String test : whitelist) {
+ if (filename.startsWith(test)) {
return true;
}
}
@@ -368,47 +367,6 @@ public class Helpers {
}
/**
- * Test if a file lives under the given directory, either as a direct child
- * or a distant grandchild.
- * <p>
- * Both files <em>must</em> have been resolved using
- * {@link File#getCanonicalFile()} to avoid symlink or path traversal
- * attacks.
- */
- public static boolean contains(File[] dirs, File file) {
- for (File dir : dirs) {
- if (contains(dir, file)) {
- return true;
- }
- }
- return false;
- }
-
- /**
- * Test if a file lives under the given directory, either as a direct child
- * or a distant grandchild.
- * <p>
- * Both files <em>must</em> have been resolved using
- * {@link File#getCanonicalFile()} to avoid symlink or path traversal
- * attacks.
- */
- public static boolean contains(File dir, File file) {
- if (dir == null || file == null) return false;
-
- String dirPath = dir.getAbsolutePath();
- String filePath = file.getAbsolutePath();
-
- if (dirPath.equals(filePath)) {
- return true;
- }
-
- if (!dirPath.endsWith("/")) {
- dirPath += "/";
- }
- return filePath.startsWith(dirPath);
- }
-
- /**
* Checks whether this looks like a legitimate selection parameter
*/
public static void validateSelection(String selection, Set<String> allowedColumns) {