[image_picker_android] Refactor interactions with ContentProvider provided filenames

packages/image_picker/image_picker_android/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.8.12+18
+
+* Fixes a security issue related to improperly trusting filenames provided by a `ContentProvider`.
+
 ## 0.8.12+17
 
 * Bumps androidx.annotation:annotation from 1.8.2 to 1.9.0.

packages/image_picker/image_picker_android/android/src/main/java/io/flutter/plugins/imagepicker/FileUtils.java

@@ -29,6 +29,8 @@
 import android.net.Uri;
 import android.provider.MediaStore;
 import android.webkit.MimeTypeMap;
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import io.flutter.Log;
 import java.io.File;
 import java.io.FileOutputStream;
@@ -42,6 +44,12 @@ class FileUtils {
    * Copies the file from the given content URI to a temporary directory, retaining the original
    * file name if possible.
    *
+   * <p>If the filename contains path indirection or separators (.. or /), the end file name will be
+   * the segment after the final separator, with indirection replaced by underscores. E.g.
+   * "example/../..file.png" -> "_file.png". See: <a
+   * href="https://developer.android.com/privacy-and-security/risks/untrustworthy-contentprovider-provided-filename">Improperly
+   * trusting ContentProvider-provided filename</a>.
+   *
    * <p>Each file is placed in its own directory to avoid conflicts according to the following
    * scheme: {cacheDir}/{randomUuid}/{fileName}
    *
@@ -69,10 +77,11 @@ String getPathFromUri(final Context context, final Uri uri) {
       } else if (extension != null) {
         fileName = getBaseName(fileName) + extension;
       }
-      File file = new File(targetDirectory, fileName);
-      try (OutputStream outputStream = new FileOutputStream(file)) {
+      String filePath = new File(targetDirectory, fileName).getPath();
+      File outputFile = saferOpenFile(filePath, targetDirectory.getCanonicalPath());
+      try (OutputStream outputStream = new FileOutputStream(outputFile)) {
         copy(inputStream, outputStream);
-        return file.getPath();
+        return outputFile.getPath();
       }
     } catch (IOException e) {
       // If closing the output stream fails, we cannot be sure that the
@@ -86,6 +95,10 @@ String getPathFromUri(final Context context, final Uri uri) {
       //
       // See https://github.com/flutter/flutter/issues/100025 for more details.
       return null;
+    } catch (IllegalArgumentException e) {
+      // This is likely a result of an IllegalArgumentException that we have thrown in
+      // saferOpenFile(). TODO(gmackall): surface this error in dart.
+      return null;
     }
   }
 
@@ -110,14 +123,49 @@ private static String getImageExtension(Context context, Uri uriImage) {
       return null;
     }
 
-    return "." + extension;
+    return "." + sanitizeFilename(extension);
+  }
+
+  // From https://developer.android.com/privacy-and-security/risks/untrustworthy-contentprovider-provided-filename#sanitize-provided-filenames.
+  protected static @Nullable String sanitizeFilename(@Nullable String displayName) {
+    if (displayName == null) {
+      return null;
+    }
+
+    String[] badCharacters = new String[] {"..", "/"};
+    String[] segments = displayName.split("/");
+    String fileName = segments[segments.length - 1];
+    for (String suspString : badCharacters) {
+      fileName = fileName.replace(suspString, "_");
+    }
+    return fileName;
+  }
+
+  /**
+   * Use with file name sanitization and an non-guessable directory. From <a
+   * href="https://developer.android.com/privacy-and-security/risks/path-traversal#path-traversal-mitigations">...</a>.
+   */
+  protected static @NonNull File saferOpenFile(@NonNull String path, @NonNull String expectedDir)
+      throws IllegalArgumentException, IOException {
+    File f = new File(path);
+    String canonicalPath = f.getCanonicalPath();
+    if (!canonicalPath.startsWith(expectedDir)) {
+      throw new IllegalArgumentException(
+          "Trying to open path outside of the expected directory. File: "
+              + f.getCanonicalPath()
+              + " was expected to be within directory: "
+              + expectedDir
+              + ".");
+    }
+    return f;
   }
 
   /** @return name of the image provided by ContentResolver; this may be null. */
   private static String getImageName(Context context, Uri uriImage) {
     try (Cursor cursor = queryImageName(context, uriImage)) {
       if (cursor == null || !cursor.moveToFirst() || cursor.getColumnCount() < 1) return null;
-      return cursor.getString(0);
+      String unsanitizedImageName = cursor.getString(0);
+      return sanitizeFilename(unsanitizedImageName);
     }
   }
 

packages/image_picker/image_picker_android/android/src/main/java/io/flutter/plugins/imagepicker/Messages.java

@@ -1,7 +1,7 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// Autogenerated from Pigeon (v22.4.1), do not edit directly.
+// Autogenerated from Pigeon (v22.6.2), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 
 package io.flutter.plugins.imagepicker;

packages/image_picker/image_picker_android/android/src/test/java/io/flutter/plugins/imagepicker/FileUtilTest.java

@@ -6,6 +6,8 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
@@ -129,6 +131,18 @@ public void FileUtil_getImageName_mismatchedType() throws IOException {
     assertTrue(path.endsWith("c.d.webp"));
   }
 
+  @Test
+  public void getPathFromUri_sanitizesPathIndirection() {
+    Uri uri = Uri.parse(MockMaliciousContentProvider.PNG_URI);
+    Robolectric.buildContentProvider(MockMaliciousContentProvider.class).create("dummy");
+    shadowContentResolver.registerInputStream(
+        uri, new ByteArrayInputStream("fileStream".getBytes(UTF_8)));
+    String path = fileUtils.getPathFromUri(context, uri);
+    assertNotNull(path);
+    assertTrue(path.endsWith("_bar.png"));
+    assertFalse(path.contains(".."));
+  }
+
   @Test
   public void FileUtil_getImageName_unknownType() throws IOException {
     Uri uri = MockContentProvider.UNKNOWN_URI;
@@ -193,4 +207,56 @@ public int update(
       return 0;
     }
   }
+
+  // Mocks a malicious content provider attempting to use path indirection to modify files outside
+  // of the intended directory.
+  // See https://developer.android.com/privacy-and-security/risks/untrustworthy-contentprovider-provided-filename#don%27t-trust-user-input.
+  private static class MockMaliciousContentProvider extends ContentProvider {
+    public static String PNG_URI = "content://dummy/a.png";
+
+    @Override
+    public boolean onCreate() {
+      return true;
+    }
+
+    @Nullable
+    @Override
+    public Cursor query(
+        @NonNull Uri uri,
+        @Nullable String[] projection,
+        @Nullable String selection,
+        @Nullable String[] selectionArgs,
+        @Nullable String sortOrder) {
+      MatrixCursor cursor = new MatrixCursor(new String[] {MediaStore.MediaColumns.DISPLAY_NAME});
+      cursor.addRow(new Object[] {"foo/../..bar.png"});
+      return cursor;
+    }
+
+    @Nullable
+    @Override
+    public String getType(@NonNull Uri uri) {
+      return "image/png";
+    }
+
+    @Nullable
+    @Override
+    public Uri insert(@NonNull Uri uri, @Nullable ContentValues values) {
+      return null;
+    }
+
+    @Override
+    public int delete(
+        @NonNull Uri uri, @Nullable String selection, @Nullable String[] selectionArgs) {
+      return 0;
+    }
+
+    @Override
+    public int update(
+        @NonNull Uri uri,
+        @Nullable ContentValues values,
+        @Nullable String selection,
+        @Nullable String[] selectionArgs) {
+      return 0;
+    }
+  }
 }

packages/image_picker/image_picker_android/lib/src/messages.g.dart

@@ -1,7 +1,7 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// Autogenerated from Pigeon (v22.4.1), do not edit directly.
+// Autogenerated from Pigeon (v22.6.2), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers
 

packages/image_picker/image_picker_android/pubspec.yaml

@@ -2,7 +2,7 @@ name: image_picker_android
 description: Android implementation of the image_picker plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/image_picker/image_picker_android
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22
-version: 0.8.12+17
+version: 0.8.12+18
 
 environment:
   sdk: ^3.5.0

packages/image_picker/image_picker_android/test/test_api.g.dart

@@ -1,7 +1,7 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// Autogenerated from Pigeon (v22.4.1), do not edit directly.
+// Autogenerated from Pigeon (v22.6.2), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, unnecessary_import, no_leading_underscores_for_local_identifiers
 // ignore_for_file: avoid_relative_lib_imports