From 2d11bafb9e036129c1319794ff48be106e148e10 Mon Sep 17 00:00:00 2001 From: Dipankar Bhardwaj Date: Fri, 20 Jun 2025 15:15:29 +0000 Subject: [SP 2025-09-01] Fix misuse of MediaProvider#INCLUDED_DEFAULT_DIRECTORIES MediaProvider#INCLUDED_DEFAULT_DIRECTORIES is internally used to grant access for file rename based on relative path. It can be misused by setting it in the bundle. Updated the code to stop using this extra and stope respecting it in AccessChecker class. Test: atest MediaProviderTests Bug: 415783046 Flag: EXEMPT bug fix Change-Id: I91c44428dc730be4cf42b3cb9c02e8eca4440f81 (cherry picked from commit a631dca1a8c02d39d9a578ce331a7b23bf7ef962) Merged-In: I91c44428dc730be4cf42b3cb9c02e8eca4440f81 --- .../android/providers/media/AccessCheckerTest.java | 66 ++++++++++++++++------ 1 file changed, 49 insertions(+), 17 deletions(-) (limited to 'tests/src') diff --git a/tests/src/com/android/providers/media/AccessCheckerTest.java b/tests/src/com/android/providers/media/AccessCheckerTest.java index 677feb775..17286c93d 100644 --- a/tests/src/com/android/providers/media/AccessCheckerTest.java +++ b/tests/src/com/android/providers/media/AccessCheckerTest.java @@ -51,7 +51,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import android.os.Bundle; +import android.os.Environment; +import android.provider.MediaStore; import android.system.Os; import android.text.TextUtils; @@ -63,6 +64,8 @@ import org.junit.runner.RunWith; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; +import java.util.Optional; @RunWith(AndroidJUnit4.class) public class AccessCheckerTest { @@ -364,22 +367,27 @@ public class AccessCheckerTest { // App with no permissions only has access to owned files assertWithMessage("Expected owned access SQL for Audio collection") - .that(getWhereForConstrainedAccess(hasNoPerms, AUDIO_MEDIA, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasNoPerms, AUDIO_MEDIA, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasNoPerms) + " OR is_ringtone=1 OR is_alarm=1 OR is_notification=1"); assertWithMessage("Expected owned access SQL for Video collection") - .that(getWhereForConstrainedAccess(hasNoPerms, VIDEO_MEDIA, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasNoPerms, VIDEO_MEDIA, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasNoPerms)); assertWithMessage("Expected owned access SQL for Images collection") - .that(getWhereForConstrainedAccess(hasNoPerms, IMAGES_MEDIA, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasNoPerms, IMAGES_MEDIA, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasNoPerms)); // App with no permissions only has access to owned files assertWithMessage("Expected owned access SQL for Downloads collection") - .that(getWhereForConstrainedAccess(hasNoPerms, DOWNLOADS, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasNoPerms, DOWNLOADS, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasNoPerms)); assertWithMessage("Expected owned access SQL for FILES collection") - .that(getWhereForConstrainedAccess(hasNoPerms, FILES, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasNoPerms, FILES, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasNoPerms)); } @@ -394,12 +402,27 @@ public class AccessCheckerTest { // App with READ_EXTERNAL_STORAGE or READ_MEDIA_* permission has access to only owned // non-media files or media files. assertWithMessage("Expected owned access SQL for Downloads collection") - .that(getWhereForConstrainedAccess(hasReadMedia, DOWNLOADS, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasReadMedia, DOWNLOADS, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasReadMedia)); assertWithMessage("Expected owned access SQL for FILES collection") - .that(getWhereForConstrainedAccess(hasReadMedia, FILES, false, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasReadMedia, FILES, false, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo( getWhereForOwnerPackageMatch(hasReadMedia) + " OR " + getFilesAccessSql()); + assertWithMessage("Expected owned access SQL for FILES collection") + .that(getWhereForConstrainedAccess(hasReadMedia, FILES, false, + /* includedDefaultDirectoriesOptional */Optional.of( + List.of(Environment.DIRECTORY_DCIM, Environment.DIRECTORY_PICTURES, + Environment.DIRECTORY_MOVIES)))) + .isEqualTo( + getWhereForOwnerPackageMatch(hasReadMedia) + " OR " + + getFilesAccessSql() + " OR " + + MediaStore.Files.FileColumns.RELATIVE_PATH + " LIKE 'DCIM/%'" + + " OR " + MediaStore.Files.FileColumns.RELATIVE_PATH + + " LIKE 'Pictures/%'" + + " OR " + MediaStore.Files.FileColumns.RELATIVE_PATH + + " LIKE 'Movies/%'"); } @Test @@ -409,22 +432,27 @@ public class AccessCheckerTest { // App with no permissions only has access to owned files. assertWithMessage("Expected owned access SQL for Audio collection") - .that(getWhereForConstrainedAccess(noPerms, AUDIO_MEDIA, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(noPerms, AUDIO_MEDIA, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(noPerms) + " OR is_ringtone=1 OR is_alarm=1 OR is_notification=1"); assertWithMessage("Expected owned access SQL for Video collection") - .that(getWhereForConstrainedAccess(noPerms, VIDEO_MEDIA, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(noPerms, VIDEO_MEDIA, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(noPerms)); assertWithMessage("Expected owned access SQL for Images collection") - .that(getWhereForConstrainedAccess(noPerms, IMAGES_MEDIA, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(noPerms, IMAGES_MEDIA, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(noPerms)); // App with no permissions only has access to owned files assertWithMessage("Expected owned access SQL for Downloads collection") - .that(getWhereForConstrainedAccess(noPerms, DOWNLOADS, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(noPerms, DOWNLOADS, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(noPerms)); assertWithMessage("Expected owned access SQL for FILES collection") - .that(getWhereForConstrainedAccess(noPerms, FILES, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(noPerms, FILES, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(noPerms)); } @@ -439,10 +467,12 @@ public class AccessCheckerTest { // App with write permission to media files has access write access to media files and owned // files. assertWithMessage("Expected owned access SQL for Downloads collection") - .that(getWhereForConstrainedAccess(hasReadPerms, DOWNLOADS, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasReadPerms, DOWNLOADS, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasReadPerms)); assertWithMessage("Expected owned access SQL for FILES collection") - .that(getWhereForConstrainedAccess(hasReadPerms, FILES, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasReadPerms, FILES, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasReadPerms) + " OR " + getFilesAccessSql()); } @@ -481,11 +511,13 @@ public class AccessCheckerTest { // Legacy app with WRITE_EXTERNAL_STORAGE permission has access to non-media files as well. // However, they don't have global write access to secondary volume. assertWithMessage("Expected where clause SQL for Downloads collection to be") - .that(getWhereForConstrainedAccess(hasLegacyWrite, DOWNLOADS, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasLegacyWrite, DOWNLOADS, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasLegacyWrite) + " OR " + AccessChecker.getWhereForExternalPrimaryMatch()); assertWithMessage("Expected where clause SQL for FILES collection to be") - .that(getWhereForConstrainedAccess(hasLegacyWrite, FILES, true, Bundle.EMPTY)) + .that(getWhereForConstrainedAccess(hasLegacyWrite, FILES, true, + /* includedDefaultDirectoriesOptional */ Optional.empty())) .isEqualTo(getWhereForOwnerPackageMatch(hasLegacyWrite) + " OR " + AccessChecker.getWhereForExternalPrimaryMatch() + " OR " + getFilesAccessSql()); -- cgit v1.2.3-59-g8ed1b