White list file names and do not allow ".."

Fixes security vulnerability where application can pass in relative file paths
with ".." in the string to access files outside of the dumpedfiles directory.

Bug: 9607306
Change-Id: Iad219cb48fa560d837498c2dc75127294dcf401b

2 files changed, 25 insertions, 3 deletions

diff --git a/src/com/android/providers/contacts/debug/DataExporter.java b/src/com/android/providers/contacts/debug/DataExporter.java
index 84dc072..c7c7dea 100644
--- a/src/com/android/providers/contacts/debug/DataExporter.java
+++ b/src/com/android/providers/contacts/debug/DataExporter.java
@@ -46,6 +46,7 @@ public class DataExporter {
     public static final String DUMP_FILE_DIRECTORY_NAME = "dumpedfiles";
 
     public static final String OUT_FILE_SUFFIX = "-contacts-db.zip";
+    public static final String VALID_FILE_NAME_REGEX = "[0-9A-Fa-f]+-contacts-db\\.zip";
 
     /**
      * Compress all files under the app data dir into a single zip file, and return the content://
@@ -81,6 +82,20 @@ public class DataExporter {
         return Hex.encodeHex(random, true);
     }
 
+    public static void ensureValidFileName(String fileName) {
+        // Do not allow queries to use relative paths to leave the root directory. Otherwise they
+        // can gain access to other files such as the contacts database.
+        if (fileName.contains("..")) {
+            throw new IllegalArgumentException(".. path specifier not allowed. Bad file name: " +
+                    fileName);
+        }
+        // White list dump files.
+        if (!fileName.matches(VALID_FILE_NAME_REGEX)) {
+            throw new IllegalArgumentException("Only " + VALID_FILE_NAME_REGEX +
+                    " files are supported. Bad file name: " + fileName);
+        }
+    }
+
     private static File getOutputDirectory(Context context) {
         return new File(context.getCacheDir(), DUMP_FILE_DIRECTORY_NAME);
     }
diff --git a/src/com/android/providers/contacts/debug/DumpFileProvider.java b/src/com/android/providers/contacts/debug/DumpFileProvider.java
index f349dd2..b294573 100644
--- a/src/com/android/providers/contacts/debug/DumpFileProvider.java
+++ b/src/com/android/providers/contacts/debug/DumpFileProvider.java
@@ -76,7 +76,10 @@ public class DumpFileProvider extends ContentProvider {
         if (!"r".equals(mode)) {
             throw new UnsupportedOperationException();
         }
-        final File file = DataExporter.getOutputFile(getContext(), extractFileName(uri));
+
+        final String fileName = extractFileName(uri);
+        DataExporter.ensureValidFileName(fileName);
+        final File file = DataExporter.getOutputFile(getContext(), fileName);
         return ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY);
     }
 
@@ -87,6 +90,9 @@ public class DumpFileProvider extends ContentProvider {
     @Override
     public Cursor query(Uri uri, String[] inProjection, String selection, String[] selectionArgs,
             String sortOrder) {
+        final String fileName = extractFileName(uri);
+        DataExporter.ensureValidFileName(fileName);
+
         final String[] projection = (inProjection != null) ? inProjection
                 : new String[] {OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE};
 
@@ -100,9 +106,9 @@ public class DumpFileProvider extends ContentProvider {
             if (OpenableColumns.DISPLAY_NAME.equals(column)) {
                 // Just return the requested path as the display name.  We don't care if the file
                 // really exists.
-                b.add(extractFileName(uri));
+                b.add(fileName);
             } else if (OpenableColumns.SIZE.equals(column)) {
-                final File file = DataExporter.getOutputFile(getContext(), extractFileName(uri));
+                final File file = DataExporter.getOutputFile(getContext(), fileName);
 
                 if (file.exists()) {
                     b.add(file.length());
@@ -117,4 +123,5 @@ public class DumpFileProvider extends ContentProvider {
 
         return c;
     }
+
 }