From 6947a091f1d9d2b62f5d4a6429df2679ffe5434b Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@google.com>
Date: Fri, 15 Jan 2016 18:51:24 +0000
Subject: Revert "Use resolved path for both checking and opening."

This reverts commit 5accb135178325878840c6e36fc3e640ae582dea.

Change-Id: I5ec1719b28feafb5b0850ec7c17cf23571ab0bba
---
 .../providers/downloads/DownloadProvider.java      | 12 +----
 src/com/android/providers/downloads/Helpers.java   | 62 ++++------------------
 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;
             }
         }
@@ -367,47 +366,6 @@ public class Helpers {
         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[] 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
      */
-- 
cgit v1.2.3