[file_selector_android] Refactor interactions with `ContentProvider` provided filenames (#8184)
https://developer.android.com/privacy-and-security/risks/untrustworthy-contentprovider-provided-filename#don%27t-trust-user-input
Adapted from this option
https://developer.android.com/privacy-and-security/risks/untrustworthy-contentprovider-provided-filename#sanitize-provided-filenames
diff --git a/packages/file_selector/file_selector_android/CHANGELOG.md b/packages/file_selector/file_selector_android/CHANGELOG.md
index ca30679..534c75f 100644
--- a/packages/file_selector/file_selector_android/CHANGELOG.md
+++ b/packages/file_selector/file_selector_android/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.5.1+12
+
+* Fixes a security issue related to improperly trusting filenames provided by a `ContentProvider`.
+
## 0.5.1+11
* Bumps androidx.annotation:annotation from 1.9.0 to 1.9.1.
diff --git a/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileSelectorApiImpl.java b/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileSelectorApiImpl.java
index 555318b..37d38c2 100644
--- a/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileSelectorApiImpl.java
+++ b/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileSelectorApiImpl.java
@@ -4,6 +4,8 @@
package dev.flutter.packages.file_selector_android;
+import static dev.flutter.packages.file_selector_android.FileUtils.FILE_SELECTOR_EXCEPTION_PLACEHOLDER_PATH;
+
import android.annotation.TargetApi;
import android.app.Activity;
import android.content.ClipData;
@@ -357,8 +359,32 @@
return null;
}
- final String uriPath =
- FileUtils.getPathFromCopyOfFileFromUri(activityPluginBinding.getActivity(), uri);
+ String uriPath;
+ GeneratedFileSelectorApi.FileSelectorNativeException nativeError = null;
+
+ try {
+ uriPath = FileUtils.getPathFromCopyOfFileFromUri(activityPluginBinding.getActivity(), uri);
+ } catch (IOException e) {
+ // If closing the output stream fails, we cannot be sure that the
+ // target file was written in full. Flushing the stream merely moves
+ // the bytes into the OS, not necessarily to the file.
+ uriPath = null;
+ } catch (SecurityException e) {
+ // Calling `ContentResolver#openInputStream()` has been reported to throw a
+ // `SecurityException` on some devices in certain circumstances. Instead of crashing, we
+ // return `null`.
+ //
+ // See https://github.com/flutter/flutter/issues/100025 for more details.
+ uriPath = null;
+ } catch (IllegalArgumentException e) {
+ uriPath = FILE_SELECTOR_EXCEPTION_PLACEHOLDER_PATH;
+ nativeError =
+ new GeneratedFileSelectorApi.FileSelectorNativeException.Builder()
+ .setMessage(e.getMessage() == null ? "" : e.getMessage())
+ .setFileSelectorExceptionCode(
+ GeneratedFileSelectorApi.FileSelectorExceptionCode.ILLEGAL_ARGUMENT_EXCEPTION)
+ .build();
+ }
return new GeneratedFileSelectorApi.FileResponse.Builder()
.setName(name)
@@ -366,6 +392,7 @@
.setPath(uriPath)
.setMimeType(contentResolver.getType(uri))
.setSize(size.longValue())
+ .setFileSelectorNativeException(nativeError)
.build();
}
}
diff --git a/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileUtils.java b/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileUtils.java
index 5d0b613..e3cd812 100644
--- a/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileUtils.java
+++ b/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/FileUtils.java
@@ -45,6 +45,8 @@
/** URI authority that represents access to external storage providers. */
public static final String EXTERNAL_DOCUMENT_AUTHORITY = "com.android.externalstorage.documents";
+ public static final String FILE_SELECTOR_EXCEPTION_PLACEHOLDER_PATH = "FILE_SELECTOR_EXCEPTION";
+
/**
* Retrieves path of directory represented by the specified {@code Uri}.
*
@@ -98,6 +100,12 @@
* 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}
*
@@ -111,7 +119,8 @@
* or if a security exception is encountered when opening the input stream to start the copying.
*/
@Nullable
- public static String getPathFromCopyOfFileFromUri(@NonNull Context context, @NonNull Uri uri) {
+ public static String getPathFromCopyOfFileFromUri(@NonNull Context context, @NonNull Uri uri)
+ throws IOException, SecurityException, IllegalArgumentException {
try (InputStream inputStream = context.getContentResolver().openInputStream(uri)) {
String uuid = UUID.nameUUIDFromBytes(uri.toString().getBytes()).toString();
File targetDirectory = new File(context.getCacheDir(), uuid);
@@ -122,7 +131,7 @@
if (fileName == null) {
if (extension == null) {
- throw new IllegalArgumentException("No name nor extension found for file.");
+ throw new IllegalStateException("No name nor extension found for file.");
} else {
fileName = "file_selector" + extension;
}
@@ -130,24 +139,13 @@
fileName = getBaseName(fileName) + extension;
}
- File file = new File(targetDirectory, fileName);
+ String filePath = new File(targetDirectory, fileName).getPath();
+ File outputFile = saferOpenFile(filePath, targetDirectory.getCanonicalPath());
- try (OutputStream outputStream = new FileOutputStream(file)) {
+ 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
- // target file was written in full. Flushing the stream merely moves
- // the bytes into the OS, not necessarily to the file.
- return null;
- } catch (SecurityException e) {
- // Calling `ContentResolver#openInputStream()` has been reported to throw a
- // `SecurityException` on some devices in certain circumstances. Instead of crashing, we
- // return `null`.
- //
- // See https://github.com/flutter/flutter/issues/100025 for more details.
- return null;
}
}
@@ -172,14 +170,17 @@
return null;
}
- return "." + extension;
+ return "." + sanitizeFilename(extension);
}
/** Returns the name of the file provided by ContentResolver; this may be null. */
private static String getFileName(Context context, Uri uriFile) {
try (Cursor cursor = queryFileName(context, uriFile)) {
- if (cursor == null || !cursor.moveToFirst() || cursor.getColumnCount() < 1) return null;
- return cursor.getString(0);
+ if (cursor == null || !cursor.moveToFirst() || cursor.getColumnCount() < 1) {
+ return null;
+ }
+ String unsanitizedFileName = cursor.getString(0);
+ return sanitizeFilename(unsanitizedFileName);
}
}
@@ -206,4 +207,38 @@
// Basename is everything before the last '.'.
return fileName.substring(0, lastDotIndex);
}
+
+ // 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 sanatization and an non-guessable directory. From
+ * https://developer.android.com/privacy-and-security/risks/path-traversal#path-traversal-mitigations.
+ */
+ 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;
+ }
}
diff --git a/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/GeneratedFileSelectorApi.java b/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/GeneratedFileSelectorApi.java
index de595ef..ee744f5 100644
--- a/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/GeneratedFileSelectorApi.java
+++ b/packages/file_selector/file_selector_android/android/src/main/java/dev/flutter/packages/file_selector_android/GeneratedFileSelectorApi.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.2), do not edit directly.
+// Autogenerated from Pigeon (v22.6.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon
package dev.flutter.packages.file_selector_android;
@@ -66,6 +66,115 @@
@Retention(CLASS)
@interface CanIgnoreReturnValue {}
+ public enum FileSelectorExceptionCode {
+ SECURITY_EXCEPTION(0),
+ IO_EXCEPTION(1),
+ ILLEGAL_ARGUMENT_EXCEPTION(2),
+ ILLEGAL_STATE_EXCEPTION(3);
+
+ final int index;
+
+ FileSelectorExceptionCode(final int index) {
+ this.index = index;
+ }
+ }
+
+ /** Generated class from Pigeon that represents data sent in messages. */
+ public static final class FileSelectorNativeException {
+ private @NonNull FileSelectorExceptionCode fileSelectorExceptionCode;
+
+ public @NonNull FileSelectorExceptionCode getFileSelectorExceptionCode() {
+ return fileSelectorExceptionCode;
+ }
+
+ public void setFileSelectorExceptionCode(@NonNull FileSelectorExceptionCode setterArg) {
+ if (setterArg == null) {
+ throw new IllegalStateException("Nonnull field \"fileSelectorExceptionCode\" is null.");
+ }
+ this.fileSelectorExceptionCode = setterArg;
+ }
+
+ private @NonNull String message;
+
+ public @NonNull String getMessage() {
+ return message;
+ }
+
+ public void setMessage(@NonNull String setterArg) {
+ if (setterArg == null) {
+ throw new IllegalStateException("Nonnull field \"message\" is null.");
+ }
+ this.message = setterArg;
+ }
+
+ /** Constructor is non-public to enforce null safety; use Builder. */
+ FileSelectorNativeException() {}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ FileSelectorNativeException that = (FileSelectorNativeException) o;
+ return fileSelectorExceptionCode.equals(that.fileSelectorExceptionCode)
+ && message.equals(that.message);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(fileSelectorExceptionCode, message);
+ }
+
+ public static final class Builder {
+
+ private @Nullable FileSelectorExceptionCode fileSelectorExceptionCode;
+
+ @CanIgnoreReturnValue
+ public @NonNull Builder setFileSelectorExceptionCode(
+ @NonNull FileSelectorExceptionCode setterArg) {
+ this.fileSelectorExceptionCode = setterArg;
+ return this;
+ }
+
+ private @Nullable String message;
+
+ @CanIgnoreReturnValue
+ public @NonNull Builder setMessage(@NonNull String setterArg) {
+ this.message = setterArg;
+ return this;
+ }
+
+ public @NonNull FileSelectorNativeException build() {
+ FileSelectorNativeException pigeonReturn = new FileSelectorNativeException();
+ pigeonReturn.setFileSelectorExceptionCode(fileSelectorExceptionCode);
+ pigeonReturn.setMessage(message);
+ return pigeonReturn;
+ }
+ }
+
+ @NonNull
+ ArrayList<Object> toList() {
+ ArrayList<Object> toListResult = new ArrayList<>(2);
+ toListResult.add(fileSelectorExceptionCode);
+ toListResult.add(message);
+ return toListResult;
+ }
+
+ static @NonNull FileSelectorNativeException fromList(
+ @NonNull ArrayList<Object> pigeonVar_list) {
+ FileSelectorNativeException pigeonResult = new FileSelectorNativeException();
+ Object fileSelectorExceptionCode = pigeonVar_list.get(0);
+ pigeonResult.setFileSelectorExceptionCode(
+ (FileSelectorExceptionCode) fileSelectorExceptionCode);
+ Object message = pigeonVar_list.get(1);
+ pigeonResult.setMessage((String) message);
+ return pigeonResult;
+ }
+ }
+
/** Generated class from Pigeon that represents data sent in messages. */
public static final class FileResponse {
private @NonNull String path;
@@ -127,6 +236,16 @@
this.bytes = setterArg;
}
+ private @Nullable FileSelectorNativeException fileSelectorNativeException;
+
+ public @Nullable FileSelectorNativeException getFileSelectorNativeException() {
+ return fileSelectorNativeException;
+ }
+
+ public void setFileSelectorNativeException(@Nullable FileSelectorNativeException setterArg) {
+ this.fileSelectorNativeException = setterArg;
+ }
+
/** Constructor is non-public to enforce null safety; use Builder. */
FileResponse() {}
@@ -143,12 +262,13 @@
&& Objects.equals(mimeType, that.mimeType)
&& Objects.equals(name, that.name)
&& size.equals(that.size)
- && Arrays.equals(bytes, that.bytes);
+ && Arrays.equals(bytes, that.bytes)
+ && Objects.equals(fileSelectorNativeException, that.fileSelectorNativeException);
}
@Override
public int hashCode() {
- int pigeonVar_result = Objects.hash(path, mimeType, name, size);
+ int pigeonVar_result = Objects.hash(path, mimeType, name, size, fileSelectorNativeException);
pigeonVar_result = 31 * pigeonVar_result + Arrays.hashCode(bytes);
return pigeonVar_result;
}
@@ -195,6 +315,15 @@
return this;
}
+ private @Nullable FileSelectorNativeException fileSelectorNativeException;
+
+ @CanIgnoreReturnValue
+ public @NonNull Builder setFileSelectorNativeException(
+ @Nullable FileSelectorNativeException setterArg) {
+ this.fileSelectorNativeException = setterArg;
+ return this;
+ }
+
public @NonNull FileResponse build() {
FileResponse pigeonReturn = new FileResponse();
pigeonReturn.setPath(path);
@@ -202,18 +331,20 @@
pigeonReturn.setName(name);
pigeonReturn.setSize(size);
pigeonReturn.setBytes(bytes);
+ pigeonReturn.setFileSelectorNativeException(fileSelectorNativeException);
return pigeonReturn;
}
}
@NonNull
ArrayList<Object> toList() {
- ArrayList<Object> toListResult = new ArrayList<>(5);
+ ArrayList<Object> toListResult = new ArrayList<>(6);
toListResult.add(path);
toListResult.add(mimeType);
toListResult.add(name);
toListResult.add(size);
toListResult.add(bytes);
+ toListResult.add(fileSelectorNativeException);
return toListResult;
}
@@ -229,6 +360,9 @@
pigeonResult.setSize((Long) size);
Object bytes = pigeonVar_list.get(4);
pigeonResult.setBytes((byte[]) bytes);
+ Object fileSelectorNativeException = pigeonVar_list.get(5);
+ pigeonResult.setFileSelectorNativeException(
+ (FileSelectorNativeException) fileSelectorNativeException);
return pigeonResult;
}
}
@@ -334,8 +468,17 @@
protected Object readValueOfType(byte type, @NonNull ByteBuffer buffer) {
switch (type) {
case (byte) 129:
- return FileResponse.fromList((ArrayList<Object>) readValue(buffer));
+ {
+ Object value = readValue(buffer);
+ return value == null
+ ? null
+ : FileSelectorExceptionCode.values()[((Long) value).intValue()];
+ }
case (byte) 130:
+ return FileSelectorNativeException.fromList((ArrayList<Object>) readValue(buffer));
+ case (byte) 131:
+ return FileResponse.fromList((ArrayList<Object>) readValue(buffer));
+ case (byte) 132:
return FileTypes.fromList((ArrayList<Object>) readValue(buffer));
default:
return super.readValueOfType(type, buffer);
@@ -344,11 +487,17 @@
@Override
protected void writeValue(@NonNull ByteArrayOutputStream stream, Object value) {
- if (value instanceof FileResponse) {
+ if (value instanceof FileSelectorExceptionCode) {
stream.write(129);
+ writeValue(stream, value == null ? null : ((FileSelectorExceptionCode) value).index);
+ } else if (value instanceof FileSelectorNativeException) {
+ stream.write(130);
+ writeValue(stream, ((FileSelectorNativeException) value).toList());
+ } else if (value instanceof FileResponse) {
+ stream.write(131);
writeValue(stream, ((FileResponse) value).toList());
} else if (value instanceof FileTypes) {
- stream.write(130);
+ stream.write(132);
writeValue(stream, ((FileTypes) value).toList());
} else {
super.writeValue(stream, value);
diff --git a/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileSelectorAndroidPluginTest.java b/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileSelectorAndroidPluginTest.java
index 00533c2..b213e85 100644
--- a/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileSelectorAndroidPluginTest.java
+++ b/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileSelectorAndroidPluginTest.java
@@ -5,6 +5,8 @@
package dev.flutter.packages.file_selector_android;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
@@ -212,6 +214,206 @@
}
}
+ // This test was created when error handling was moved from FileUtils.java to FileSelectorApiImpl.java
+ // in https://github.com/flutter/packages/pull/8184, so as to maintain the existing test.
+ // The behavior is actually an error case and should be fixed,
+ // see: https://github.com/flutter/flutter/issues/159568.
+ // Remove when fixed!
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ @Test
+ public void
+ openFileThrowsIllegalStateException_whenSecurityExceptionInGetPathFromCopyOfFileFromUri()
+ throws FileNotFoundException {
+
+ try (MockedStatic<FileUtils> mockedFileUtils = mockStatic(FileUtils.class)) {
+
+ final ContentResolver mockContentResolver = mock(ContentResolver.class);
+
+ final Uri mockUri = mock(Uri.class);
+ final String mockUriPath = "some/path/";
+ mockedFileUtils
+ .when(() -> FileUtils.getPathFromCopyOfFileFromUri(any(Context.class), eq(mockUri)))
+ .thenThrow(SecurityException.class);
+ mockContentResolver(mockContentResolver, mockUri, "filename", 30, "text/plain");
+
+ final Uri mockUri2 = mock(Uri.class);
+ final String mockUri2Path = "some/other/path/";
+ mockedFileUtils
+ .when(() -> FileUtils.getPathFromCopyOfFileFromUri(any(Context.class), eq(mockUri2)))
+ .thenAnswer((Answer<String>) invocation -> mockUri2Path);
+ mockContentResolver(mockContentResolver, mockUri2, "filename2", 40, "image/jpg");
+
+ when(mockObjectFactory.newIntent(Intent.ACTION_OPEN_DOCUMENT)).thenReturn(mockIntent);
+ when(mockObjectFactory.newDataInputStream(any())).thenReturn(mock(DataInputStream.class));
+ when(mockActivity.getContentResolver()).thenReturn(mockContentResolver);
+ when(mockActivityBinding.getActivity()).thenReturn(mockActivity);
+ final FileSelectorApiImpl fileSelectorApi =
+ new FileSelectorApiImpl(
+ mockActivityBinding,
+ mockObjectFactory,
+ (version) -> Build.VERSION.SDK_INT >= version);
+
+ final GeneratedFileSelectorApi.Result mockResult =
+ mock(GeneratedFileSelectorApi.Result.class);
+ fileSelectorApi.openFiles(
+ null,
+ new GeneratedFileSelectorApi.FileTypes.Builder()
+ .setMimeTypes(Collections.emptyList())
+ .setExtensions(Collections.emptyList())
+ .build(),
+ mockResult);
+ verify(mockIntent).addCategory(Intent.CATEGORY_OPENABLE);
+ verify(mockIntent).putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true);
+
+ verify(mockActivity).startActivityForResult(mockIntent, 222);
+
+ final ArgumentCaptor<PluginRegistry.ActivityResultListener> listenerArgumentCaptor =
+ ArgumentCaptor.forClass(PluginRegistry.ActivityResultListener.class);
+ verify(mockActivityBinding).addActivityResultListener(listenerArgumentCaptor.capture());
+
+ final Intent resultMockIntent = mock(Intent.class);
+ final ClipData mockClipData = mock(ClipData.class);
+ when(mockClipData.getItemCount()).thenReturn(2);
+
+ final ClipData.Item mockClipDataItem = mock(ClipData.Item.class);
+ when(mockClipDataItem.getUri()).thenReturn(mockUri);
+ when(mockClipData.getItemAt(0)).thenReturn(mockClipDataItem);
+
+ final ClipData.Item mockClipDataItem2 = mock(ClipData.Item.class);
+ when(mockClipDataItem2.getUri()).thenReturn(mockUri2);
+ when(mockClipData.getItemAt(1)).thenReturn(mockClipDataItem2);
+
+ when(resultMockIntent.getClipData()).thenReturn(mockClipData);
+
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ listenerArgumentCaptor
+ .getValue()
+ .onActivityResult(222, Activity.RESULT_OK, resultMockIntent));
+ }
+ }
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ @Test
+ public void
+ openFileReturnsNativeException_whenIllegalArgumentExceptionInGetPathFromCopyOfFileFromUri()
+ throws FileNotFoundException {
+ try (MockedStatic<FileUtils> mockedFileUtils = mockStatic(FileUtils.class)) {
+ final ContentResolver mockContentResolver = mock(ContentResolver.class);
+
+ final Uri mockUri = mock(Uri.class);
+ mockedFileUtils
+ .when(() -> FileUtils.getPathFromCopyOfFileFromUri(any(Context.class), eq(mockUri)))
+ .thenThrow(IllegalArgumentException.class);
+ mockContentResolver(mockContentResolver, mockUri, "filename", 30, "text/plain");
+
+ when(mockObjectFactory.newIntent(Intent.ACTION_OPEN_DOCUMENT)).thenReturn(mockIntent);
+ when(mockObjectFactory.newDataInputStream(any())).thenReturn(mock(DataInputStream.class));
+ when(mockActivity.getContentResolver()).thenReturn(mockContentResolver);
+ when(mockActivityBinding.getActivity()).thenReturn(mockActivity);
+ final FileSelectorApiImpl fileSelectorApi =
+ new FileSelectorApiImpl(
+ mockActivityBinding,
+ mockObjectFactory,
+ (version) -> Build.VERSION.SDK_INT >= version);
+
+ final GeneratedFileSelectorApi.NullableResult mockResult =
+ mock(GeneratedFileSelectorApi.NullableResult.class);
+ fileSelectorApi.openFile(
+ null,
+ new GeneratedFileSelectorApi.FileTypes.Builder()
+ .setMimeTypes(Collections.emptyList())
+ .setExtensions(Collections.emptyList())
+ .build(),
+ mockResult);
+ verify(mockIntent).addCategory(Intent.CATEGORY_OPENABLE);
+
+ verify(mockActivity).startActivityForResult(mockIntent, 221);
+
+ final ArgumentCaptor<PluginRegistry.ActivityResultListener> listenerArgumentCaptor =
+ ArgumentCaptor.forClass(PluginRegistry.ActivityResultListener.class);
+ verify(mockActivityBinding).addActivityResultListener(listenerArgumentCaptor.capture());
+
+ final Intent resultMockIntent = mock(Intent.class);
+ when(resultMockIntent.getData()).thenReturn(mockUri);
+ listenerArgumentCaptor.getValue().onActivityResult(221, Activity.RESULT_OK, resultMockIntent);
+
+ final ArgumentCaptor<GeneratedFileSelectorApi.FileResponse> fileCaptor =
+ ArgumentCaptor.forClass(GeneratedFileSelectorApi.FileResponse.class);
+ verify(mockResult).success(fileCaptor.capture());
+
+ final GeneratedFileSelectorApi.FileResponse file = fileCaptor.getValue();
+ assertNotNull(file.getFileSelectorNativeException());
+ assertEquals(file.getPath(), FileUtils.FILE_SELECTOR_EXCEPTION_PLACEHOLDER_PATH);
+ }
+ }
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ @Test
+ public void
+ openFilesReturnsNativeException_whenIllegalArgumentExceptionInGetPathFromCopyOfFileFromUri()
+ throws FileNotFoundException {
+ try (MockedStatic<FileUtils> mockedFileUtils = mockStatic(FileUtils.class)) {
+
+ final ContentResolver mockContentResolver = mock(ContentResolver.class);
+
+ final Uri mockUri = mock(Uri.class);
+ final String mockUriPath = "some/path/";
+ mockedFileUtils
+ .when(() -> FileUtils.getPathFromCopyOfFileFromUri(any(Context.class), eq(mockUri)))
+ .thenThrow(IllegalArgumentException.class);
+ mockContentResolver(mockContentResolver, mockUri, "filename", 30, "text/plain");
+
+ when(mockObjectFactory.newIntent(Intent.ACTION_OPEN_DOCUMENT)).thenReturn(mockIntent);
+ when(mockObjectFactory.newDataInputStream(any())).thenReturn(mock(DataInputStream.class));
+ when(mockActivity.getContentResolver()).thenReturn(mockContentResolver);
+ when(mockActivityBinding.getActivity()).thenReturn(mockActivity);
+ final FileSelectorApiImpl fileSelectorApi =
+ new FileSelectorApiImpl(
+ mockActivityBinding,
+ mockObjectFactory,
+ (version) -> Build.VERSION.SDK_INT >= version);
+
+ final GeneratedFileSelectorApi.Result mockResult =
+ mock(GeneratedFileSelectorApi.Result.class);
+ fileSelectorApi.openFiles(
+ null,
+ new GeneratedFileSelectorApi.FileTypes.Builder()
+ .setMimeTypes(Collections.emptyList())
+ .setExtensions(Collections.emptyList())
+ .build(),
+ mockResult);
+ verify(mockIntent).addCategory(Intent.CATEGORY_OPENABLE);
+ verify(mockIntent).putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true);
+
+ verify(mockActivity).startActivityForResult(mockIntent, 222);
+
+ final ArgumentCaptor<PluginRegistry.ActivityResultListener> listenerArgumentCaptor =
+ ArgumentCaptor.forClass(PluginRegistry.ActivityResultListener.class);
+ verify(mockActivityBinding).addActivityResultListener(listenerArgumentCaptor.capture());
+
+ final Intent resultMockIntent = mock(Intent.class);
+ final ClipData mockClipData = mock(ClipData.class);
+ when(mockClipData.getItemCount()).thenReturn(1);
+
+ final ClipData.Item mockClipDataItem = mock(ClipData.Item.class);
+ when(mockClipDataItem.getUri()).thenReturn(mockUri);
+ when(mockClipData.getItemAt(0)).thenReturn(mockClipDataItem);
+
+ when(resultMockIntent.getClipData()).thenReturn(mockClipData);
+
+ listenerArgumentCaptor.getValue().onActivityResult(222, Activity.RESULT_OK, resultMockIntent);
+
+ final ArgumentCaptor<List> fileListCaptor = ArgumentCaptor.forClass(List.class);
+ verify(mockResult).success(fileListCaptor.capture());
+
+ final List<GeneratedFileSelectorApi.FileResponse> fileList = fileListCaptor.getValue();
+ assertEquals(fileList.get(0).getPath(), FileUtils.FILE_SELECTOR_EXCEPTION_PLACEHOLDER_PATH);
+ assertNotNull(fileList.get(0).getFileSelectorNativeException());
+ }
+ }
+
@SuppressWarnings({"rawtypes", "unchecked"})
@Test
public void getDirectoryPathReturnsSuccessfully() {
diff --git a/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileUtilsTest.java b/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileUtilsTest.java
index 7608743..a6d0f57 100644
--- a/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileUtilsTest.java
+++ b/packages/file_selector/file_selector_android/android/src/test/java/dev/flutter/packages/file_selector_android/FileUtilsTest.java
@@ -6,14 +6,12 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.when;
import static org.robolectric.Shadows.shadowOf;
import android.content.ContentProvider;
@@ -126,22 +124,6 @@
}
@Test
- public void getPathFromCopyOfFileFromUri_returnsNullPathWhenSecurityExceptionThrown()
- throws IOException {
- Uri uri = Uri.parse("content://dummy/dummy.png");
-
- ContentResolver mockContentResolver = mock(ContentResolver.class);
- when(mockContentResolver.openInputStream(any(Uri.class))).thenThrow(SecurityException.class);
-
- Context mockContext = mock(Context.class);
- when(mockContext.getContentResolver()).thenReturn(mockContentResolver);
-
- String path = FileUtils.getPathFromCopyOfFileFromUri(mockContext, uri);
-
- assertNull(path);
- }
-
- @Test
public void getFileExtension_returnsExpectedFileExtension() throws IOException {
Uri uri = MockContentProvider.TXT_URI;
Robolectric.buildContentProvider(MockContentProvider.class).create("dummy");
@@ -196,6 +178,18 @@
assertTrue(path.endsWith("e.f.g"));
}
+ @Test
+ public void getPathFromCopyOfFileFromUri_sanitizesPathIndirection() throws IOException {
+ 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.getPathFromCopyOfFileFromUri(context, uri);
+ assertNotNull(path);
+ assertTrue(path.endsWith("_bar.png"));
+ assertFalse(path.contains(".."));
+ }
+
private static class MockContentProvider extends ContentProvider {
public static final Uri TXT_URI = Uri.parse("content://dummy/dummydocument");
public static final Uri PNG_URI = Uri.parse("content://dummy/a.b.png");
@@ -252,4 +246,56 @@
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/file_selector/file_selector_android/lib/file_selector_android.dart b/packages/file_selector/file_selector_android/lib/file_selector_android.dart
index c86f2da..6775131 100644
--- a/packages/file_selector/file_selector_android/lib/file_selector_android.dart
+++ b/packages/file_selector/file_selector_android/lib/file_selector_android.dart
@@ -3,3 +3,4 @@
// found in the LICENSE file.
export 'src/file_selector_android.dart';
+export 'src/types/native_illegal_argument_exception.dart';
diff --git a/packages/file_selector/file_selector_android/lib/src/file_selector_android.dart b/packages/file_selector/file_selector_android/lib/src/file_selector_android.dart
index bc11695..8f9e46b 100644
--- a/packages/file_selector/file_selector_android/lib/src/file_selector_android.dart
+++ b/packages/file_selector/file_selector_android/lib/src/file_selector_android.dart
@@ -8,6 +8,7 @@
import 'package:flutter/cupertino.dart';
import 'file_selector_api.g.dart';
+import 'types/native_illegal_argument_exception.dart';
/// An implementation of [FileSelectorPlatform] for Android.
class FileSelectorAndroid extends FileSelectorPlatform {
@@ -56,6 +57,9 @@
}
XFile _xFileFromFileResponse(FileResponse file) {
+ if (file.fileSelectorNativeException != null) {
+ _resolveErrorCodeAndMaybeThrow(file.fileSelectorNativeException!);
+ }
return XFile.fromData(
file.bytes,
// Note: The name parameter is not used by XFile. The XFile.name returns
@@ -95,4 +99,19 @@
extensions: extensions.toList(),
);
}
+
+ /// Translates a [FileSelectorExceptionCode] to its corresponding error and
+ /// handles throwing.
+ void _resolveErrorCodeAndMaybeThrow(
+ FileSelectorNativeException fileSelectorNativeException) {
+ switch (fileSelectorNativeException.fileSelectorExceptionCode) {
+ case FileSelectorExceptionCode.illegalArgumentException:
+ throw NativeIllegalArgumentException(
+ fileSelectorNativeException.message);
+ case (FileSelectorExceptionCode.illegalStateException ||
+ FileSelectorExceptionCode.ioException ||
+ FileSelectorExceptionCode.securityException):
+ // unused for now
+ }
+ }
}
diff --git a/packages/file_selector/file_selector_android/lib/src/file_selector_api.g.dart b/packages/file_selector/file_selector_android/lib/src/file_selector_api.g.dart
index 558ab5f..7334142 100644
--- a/packages/file_selector/file_selector_android/lib/src/file_selector_api.g.dart
+++ b/packages/file_selector/file_selector_android/lib/src/file_selector_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.2), 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
@@ -18,6 +18,39 @@
);
}
+enum FileSelectorExceptionCode {
+ securityException,
+ ioException,
+ illegalArgumentException,
+ illegalStateException,
+}
+
+class FileSelectorNativeException {
+ FileSelectorNativeException({
+ required this.fileSelectorExceptionCode,
+ required this.message,
+ });
+
+ FileSelectorExceptionCode fileSelectorExceptionCode;
+
+ String message;
+
+ Object encode() {
+ return <Object?>[
+ fileSelectorExceptionCode,
+ message,
+ ];
+ }
+
+ static FileSelectorNativeException decode(Object result) {
+ result as List<Object?>;
+ return FileSelectorNativeException(
+ fileSelectorExceptionCode: result[0]! as FileSelectorExceptionCode,
+ message: result[1]! as String,
+ );
+ }
+}
+
class FileResponse {
FileResponse({
required this.path,
@@ -25,6 +58,7 @@
this.name,
required this.size,
required this.bytes,
+ this.fileSelectorNativeException,
});
String path;
@@ -37,6 +71,8 @@
Uint8List bytes;
+ FileSelectorNativeException? fileSelectorNativeException;
+
Object encode() {
return <Object?>[
path,
@@ -44,6 +80,7 @@
name,
size,
bytes,
+ fileSelectorNativeException,
];
}
@@ -55,6 +92,7 @@
name: result[2] as String?,
size: result[3]! as int,
bytes: result[4]! as Uint8List,
+ fileSelectorNativeException: result[5] as FileSelectorNativeException?,
);
}
}
@@ -92,11 +130,17 @@
if (value is int) {
buffer.putUint8(4);
buffer.putInt64(value);
- } else if (value is FileResponse) {
+ } else if (value is FileSelectorExceptionCode) {
buffer.putUint8(129);
+ writeValue(buffer, value.index);
+ } else if (value is FileSelectorNativeException) {
+ buffer.putUint8(130);
+ writeValue(buffer, value.encode());
+ } else if (value is FileResponse) {
+ buffer.putUint8(131);
writeValue(buffer, value.encode());
} else if (value is FileTypes) {
- buffer.putUint8(130);
+ buffer.putUint8(132);
writeValue(buffer, value.encode());
} else {
super.writeValue(buffer, value);
@@ -107,8 +151,13 @@
Object? readValueOfType(int type, ReadBuffer buffer) {
switch (type) {
case 129:
- return FileResponse.decode(readValue(buffer)!);
+ final int? value = readValue(buffer) as int?;
+ return value == null ? null : FileSelectorExceptionCode.values[value];
case 130:
+ return FileSelectorNativeException.decode(readValue(buffer)!);
+ case 131:
+ return FileResponse.decode(readValue(buffer)!);
+ case 132:
return FileTypes.decode(readValue(buffer)!);
default:
return super.readValueOfType(type, buffer);
diff --git a/packages/file_selector/file_selector_android/lib/src/types/native_illegal_argument_exception.dart b/packages/file_selector/file_selector_android/lib/src/types/native_illegal_argument_exception.dart
new file mode 100644
index 0000000..b22be80
--- /dev/null
+++ b/packages/file_selector/file_selector_android/lib/src/types/native_illegal_argument_exception.dart
@@ -0,0 +1,17 @@
+// 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.
+
+/// A representation of a Java IllegalArgumentException in dart.
+class NativeIllegalArgumentException implements Exception {
+ /// Creates a [NativeIllegalArgumentException].
+ NativeIllegalArgumentException(this.message);
+
+ /// The message provided by the native error.
+ final String message;
+
+ @override
+ String toString() {
+ return 'NativeIllegalArgumentException($message)';
+ }
+}
diff --git a/packages/file_selector/file_selector_android/pigeons/file_selector_api.dart b/packages/file_selector/file_selector_android/pigeons/file_selector_api.dart
index 3a6c522..409eec5 100644
--- a/packages/file_selector/file_selector_android/pigeons/file_selector_api.dart
+++ b/packages/file_selector/file_selector_android/pigeons/file_selector_api.dart
@@ -16,12 +16,25 @@
copyrightHeader: 'pigeons/copyright.txt',
),
)
+enum FileSelectorExceptionCode {
+ securityException, // unused
+ ioException, // unused
+ illegalArgumentException,
+ illegalStateException, //unused
+}
+
+class FileSelectorNativeException implements Exception {
+ late final FileSelectorExceptionCode fileSelectorExceptionCode;
+ late final String message;
+}
+
class FileResponse {
late final String path;
late final String? mimeType;
late final String? name;
late final int size;
late final Uint8List bytes;
+ late final FileSelectorNativeException? fileSelectorNativeException;
}
class FileTypes {
diff --git a/packages/file_selector/file_selector_android/pubspec.yaml b/packages/file_selector/file_selector_android/pubspec.yaml
index 76087f6..e70379f 100644
--- a/packages/file_selector/file_selector_android/pubspec.yaml
+++ b/packages/file_selector/file_selector_android/pubspec.yaml
@@ -2,7 +2,7 @@
description: Android implementation of the file_selector package.
repository: https://github.com/flutter/packages/tree/main/packages/file_selector/file_selector_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+file_selector%22
-version: 0.5.1+11
+version: 0.5.1+12
environment:
sdk: ^3.5.0
