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: Improperly + * trusting ContentProvider-provided filename. + * *
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 .... + */ + 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); } } diff --git a/packages/image_picker/image_picker_android/android/src/main/java/io/flutter/plugins/imagepicker/Messages.java b/packages/image_picker/image_picker_android/android/src/main/java/io/flutter/plugins/imagepicker/Messages.java index c11f20e228d4..5c72a30b5915 100644 --- a/packages/image_picker/image_picker_android/android/src/main/java/io/flutter/plugins/imagepicker/Messages.java +++ b/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; diff --git a/packages/image_picker/image_picker_android/android/src/test/java/io/flutter/plugins/imagepicker/FileUtilTest.java b/packages/image_picker/image_picker_android/android/src/test/java/io/flutter/plugins/imagepicker/FileUtilTest.java index 620bac74a17b..1a33302e1fdc 100644 --- a/packages/image_picker/image_picker_android/android/src/test/java/io/flutter/plugins/imagepicker/FileUtilTest.java +++ b/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; + } + } } diff --git a/packages/image_picker/image_picker_android/lib/src/messages.g.dart b/packages/image_picker/image_picker_android/lib/src/messages.g.dart index e221123fd800..cbeb6a2e61a1 100644 --- a/packages/image_picker/image_picker_android/lib/src/messages.g.dart +++ b/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 diff --git a/packages/image_picker/image_picker_android/pubspec.yaml b/packages/image_picker/image_picker_android/pubspec.yaml index 605dc0c1585a..b3f294e716ae 100755 --- a/packages/image_picker/image_picker_android/pubspec.yaml +++ b/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 diff --git a/packages/image_picker/image_picker_android/test/test_api.g.dart b/packages/image_picker/image_picker_android/test/test_api.g.dart index 4343c4958436..f28bafe9ea7f 100644 --- a/packages/image_picker/image_picker_android/test/test_api.g.dart +++ b/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