diff options
author | Sergey Nikolaienkov <sergeynv@google.com> | 2023-03-28 12:22:31 +0200 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-08-22 02:16:03 +0000 |
commit | 0bbf3f4c3e75f1995d7555cc1643db049ef1bf11 (patch) | |
tree | 4cfeb192cd43fc3f771741bddf654a4c4b075256 | |
parent | fb4e7b9fc30cfe624abf30ed00b3232749c3ce29 (diff) | |
download | MediaProvider-0bbf3f4c3e75f1995d7555cc1643db049ef1bf11.tar.gz |
Fix path traversal vulnerabilities in MediaProvider
Canonicalize filepath provided by the caller when hanling SCAN_FILE_CALL
method call in MediaProvider.
Additionally, make sure to check access permission in SCAN_FILE_CALL
(using enforceCallingPermissionInternal()).
Preemptively canonicalize Files provided as an arguments to the public
API methods in ModernMediaScanner (scanFile(), scanDirectory() and
onDirectoryDirty()) to prevent path traversal attacks.
Bug: 262244882
Test: atest MediaProviderTests
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:4c867764086b90a27e745ec82e383d63fb9b6232)
Merged-In: I61e77d69ae857984b819fa0ea27bec5c26a34842
Change-Id: I61e77d69ae857984b819fa0ea27bec5c26a34842
-rw-r--r-- | src/com/android/providers/media/MediaProvider.java | 55 | ||||
-rw-r--r-- | src/com/android/providers/media/scan/ModernMediaScanner.java | 25 | ||||
-rw-r--r-- | src/com/android/providers/media/util/FileUtils.java | 47 |
3 files changed, 90 insertions, 37 deletions
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java index e96030627..7aa43dbcb 100644 --- a/src/com/android/providers/media/MediaProvider.java +++ b/src/com/android/providers/media/MediaProvider.java @@ -6418,38 +6418,51 @@ public class MediaProvider extends ContentProvider { } return null; } - case MediaStore.SCAN_FILE_CALL: + case MediaStore.SCAN_FILE_CALL: { + final LocalCallingIdentity token = clearLocalCallingIdentity(); + final CallingIdentity providerToken = clearCallingIdentity(); + + final String filePath = arg; + final Uri uri; + try { + File file; + try { + file = FileUtils.getCanonicalFile(filePath); + } catch (IOException e) { + file = null; + } + + uri = file != null ? scanFile(file, REASON_DEMAND) : null; + } finally { + restoreCallingIdentity(providerToken); + restoreLocalCallingIdentity(token); + } + + // TODO(b/262244882): maybe enforceCallingPermissionInternal(uri, ...) + + final Bundle res = new Bundle(); + res.putParcelable(Intent.EXTRA_STREAM, uri); + return res; + } case MediaStore.SCAN_VOLUME_CALL: { final int userId = uidToUserId(Binder.getCallingUid()); final LocalCallingIdentity token = clearLocalCallingIdentity(); final CallingIdentity providerToken = clearCallingIdentity(); + + final String volumeName = arg; try { - final Bundle res = new Bundle(); - switch (method) { - case MediaStore.SCAN_FILE_CALL: { - final File file = new File(arg); - res.putParcelable(Intent.EXTRA_STREAM, scanFile(file, REASON_DEMAND)); - break; - } - case MediaStore.SCAN_VOLUME_CALL: { - final String volumeName = arg; - try { - MediaVolume volume = mVolumeCache.findVolume(volumeName, - UserHandle.of(userId)); - MediaService.onScanVolume(getContext(), volume, REASON_DEMAND); - } catch (FileNotFoundException e) { - Log.w(TAG, "Failed to find volume " + volumeName, e); - } - break; - } - } - return res; + final MediaVolume volume = mVolumeCache.findVolume(volumeName, + UserHandle.of(userId)); + MediaService.onScanVolume(getContext(), volume, REASON_DEMAND); + } catch (FileNotFoundException e) { + Log.w(TAG, "Failed to find volume " + volumeName, e); } catch (IOException e) { throw new RuntimeException(e); } finally { restoreCallingIdentity(providerToken); restoreLocalCallingIdentity(token); } + return Bundle.EMPTY; } case MediaStore.GET_VERSION_CALL: { final String volumeName = extras.getString(Intent.EXTRA_TEXT); diff --git a/src/com/android/providers/media/scan/ModernMediaScanner.java b/src/com/android/providers/media/scan/ModernMediaScanner.java index 0a84838a5..55a4a813e 100644 --- a/src/com/android/providers/media/scan/ModernMediaScanner.java +++ b/src/com/android/providers/media/scan/ModernMediaScanner.java @@ -241,12 +241,17 @@ public class ModernMediaScanner implements MediaScanner { @Override public void scanDirectory(@NonNull File file, @ScanReason int reason) { requireNonNull(file); + try { + file = file.getCanonicalFile(); + } catch (IOException e) { + Log.e(TAG, "Couldn't canonicalize directory to scan" + file, e); + return; + } try (Scan scan = new Scan(file, reason)) { scan.run(); } catch (FileNotFoundException e) { Log.e(TAG, "Couldn't find directory to scan", e); - return; } catch (OperationCanceledException ignored) { // No-op. } @@ -256,6 +261,12 @@ public class ModernMediaScanner implements MediaScanner { @Nullable public Uri scanFile(@NonNull File file, @ScanReason int reason) { requireNonNull(file); + try { + file = file.getCanonicalFile(); + } catch (IOException e) { + Log.e(TAG, "Couldn't canonicalize file to scan" + file, e); + return null; + } try (Scan scan = new Scan(file, reason)) { scan.run(); @@ -292,10 +303,18 @@ public class ModernMediaScanner implements MediaScanner { } @Override - public void onDirectoryDirty(File dir) { + public void onDirectoryDirty(@NonNull File dir) { + requireNonNull(dir); + try { + dir = dir.getCanonicalFile(); + } catch (IOException e) { + Log.e(TAG, "Couldn't canonicalize directory" + dir, e); + return; + } + synchronized (mPendingCleanDirectories) { mPendingCleanDirectories.remove(dir.getPath()); - FileUtils.setDirectoryDirty(dir, /*isDirty*/ true); + FileUtils.setDirectoryDirty(dir, /* isDirty */ true); } } diff --git a/src/com/android/providers/media/util/FileUtils.java b/src/com/android/providers/media/util/FileUtils.java index 376cdf355..00504f4c7 100644 --- a/src/com/android/providers/media/util/FileUtils.java +++ b/src/com/android/providers/media/util/FileUtils.java @@ -1113,18 +1113,25 @@ public class FileUtils { } public static @Nullable String extractRelativePath(@Nullable String data) { - data = getCanonicalPath(data); if (data == null) return null; - final Matcher matcher = PATTERN_RELATIVE_PATH.matcher(data); + final String path; + try { + path = getCanonicalPath(data); + } catch (IOException e) { + Log.d(TAG, "Unable to get canonical path from invalid data path: " + data, e); + return null; + } + + final Matcher matcher = PATTERN_RELATIVE_PATH.matcher(path); if (matcher.find()) { - final int lastSlash = data.lastIndexOf('/'); + final int lastSlash = path.lastIndexOf('/'); if (lastSlash == -1 || lastSlash < matcher.end()) { // This is a file in the top-level directory, so relative path is "/" // which is different than null, which means unknown path return "/"; } else { - return data.substring(matcher.end(), lastSlash + 1); + return path.substring(matcher.end(), lastSlash + 1); } } else { return null; @@ -1817,15 +1824,29 @@ public class FileUtils { return new File(file.getPath().replaceFirst(FUSE_FS_PREFIX, LOWER_FS_PREFIX)); } - @Nullable - private static String getCanonicalPath(@Nullable String path) { - if (path == null) return null; + /** + * Returns the canonical {@link File} for the provided abstract pathname. + * + * @return The canonical pathname string denoting the same file or directory as this abstract + * pathname + * @see File#getCanonicalFile() + */ + @NonNull + public static File getCanonicalFile(@NonNull String path) throws IOException { + Objects.requireNonNull(path); + return new File(path).getCanonicalFile(); + } - try { - return new File(path).getCanonicalPath(); - } catch (IOException e) { - Log.d(TAG, "Unable to get canonical path from invalid data path: " + path, e); - return null; - } + /** + * Returns the canonical pathname string of the provided abstract pathname. + * + * @return The canonical pathname string denoting the same file or directory as this abstract + * pathname. + * @see File#getCanonicalPath() + */ + @NonNull + public static String getCanonicalPath(@NonNull String path) throws IOException { + Objects.requireNonNull(path); + return new File(path).getCanonicalPath(); } } |