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

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 84dc072a..c7c7dea4 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 f349dd25..b294573c 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;
     }
+
 }