diff options
author | TreeHugger Robot <treehugger-gerrit@google.com> | 2022-09-08 17:09:07 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2022-09-08 17:09:07 +0000 |
commit | 52f3a68fcd4656390f7f90694bc186bbbe889a3f (patch) | |
tree | 9e3fafc0997306a2fcf6153d77cfdbff54e112b4 | |
parent | 8ad5a7d993bbffd31a205e3b743d2652857755f7 (diff) | |
parent | 95e397a8b82de97f9806d102f6adc519267e29c3 (diff) | |
download | base-52f3a68fcd4656390f7f90694bc186bbbe889a3f.tar.gz |
Merge "Revert "Prevent exfiltration of system files via avatar picker."" into tm-dev am: 95e397a8b8
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/19436694
Change-Id: Id0c46913c70785ecbf90b9f4d6c4a4e3b1270298
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2 files changed, 60 insertions, 96 deletions
diff --git a/packages/SettingsLib/src/com/android/settingslib/users/AvatarPhotoController.java b/packages/SettingsLib/src/com/android/settingslib/users/AvatarPhotoController.java index 63a9f0c5c7f4..adfa39e3df80 100644 --- a/packages/SettingsLib/src/com/android/settingslib/users/AvatarPhotoController.java +++ b/packages/SettingsLib/src/com/android/settingslib/users/AvatarPhotoController.java @@ -21,8 +21,6 @@ import android.content.ClipData; import android.content.ContentResolver; import android.content.Context; import android.content.Intent; -import android.content.pm.ActivityInfo; -import android.content.pm.PackageManager; import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.graphics.Canvas; @@ -59,9 +57,9 @@ class AvatarPhotoController { void startActivityForResult(Intent intent, int resultCode); - boolean startSystemActivityForResult(Intent intent, int resultCode); - int getPhotoSize(); + + boolean canCropPhoto(); } interface ContextInjector { @@ -84,7 +82,6 @@ class AvatarPhotoController { private static final long DELAY_BEFORE_CROP_MILLIS = 150; private static final String IMAGES_DIR = "multi_user"; - private static final String PRE_CROP_PICTURE_FILE_NAME = "PreCropEditUserPhoto.jpg"; private static final String CROP_PICTURE_FILE_NAME = "CropEditUserPhoto.jpg"; private static final String TAKE_PICTURE_FILE_NAME = "TakeEditUserPhoto.jpg"; @@ -94,7 +91,6 @@ class AvatarPhotoController { private final ContextInjector mContextInjector; private final File mImagesDir; - private final Uri mPreCropPictureUri; private final Uri mCropPictureUri; private final Uri mTakePictureUri; @@ -104,8 +100,6 @@ class AvatarPhotoController { mImagesDir = new File(mContextInjector.getCacheDir(), IMAGES_DIR); mImagesDir.mkdir(); - mPreCropPictureUri = mContextInjector - .createTempImageUri(mImagesDir, PRE_CROP_PICTURE_FILE_NAME, !waiting); mCropPictureUri = mContextInjector.createTempImageUri(mImagesDir, CROP_PICTURE_FILE_NAME, !waiting); mTakePictureUri = @@ -137,7 +131,7 @@ class AvatarPhotoController { return true; case REQUEST_CODE_TAKE_PHOTO: if (mTakePictureUri.equals(pictureUri)) { - cropPhoto(pictureUri); + cropPhoto(); } else { copyAndCropPhoto(pictureUri, false); } @@ -166,7 +160,7 @@ class AvatarPhotoController { ThreadUtils.postOnBackgroundThread(() -> { final ContentResolver cr = mContextInjector.getContentResolver(); try (InputStream in = cr.openInputStream(pictureUri); - OutputStream out = cr.openOutputStream(mPreCropPictureUri)) { + OutputStream out = cr.openOutputStream(mTakePictureUri)) { Streams.copy(in, out); } catch (IOException e) { Log.w(TAG, "Failed to copy photo", e); @@ -174,7 +168,7 @@ class AvatarPhotoController { } Runnable cropRunnable = () -> { if (!mAvatarUi.isFinishing()) { - cropPhoto(mPreCropPictureUri); + cropPhoto(); } }; if (delayBeforeCrop) { @@ -189,21 +183,22 @@ class AvatarPhotoController { } } - private void cropPhoto(final Uri pictureUri) { - // TODO: Use a public intent, when there is one. - Intent intent = new Intent("com.android.camera.action.CROP"); - intent.setDataAndType(pictureUri, "image/*"); - appendOutputExtra(intent, mCropPictureUri); - appendCropExtras(intent); - try { - StrictMode.disableDeathOnFileUriExposure(); - if (mAvatarUi.startSystemActivityForResult(intent, REQUEST_CODE_CROP_PHOTO)) { - return; + private void cropPhoto() { + if (mAvatarUi.canCropPhoto()) { + // TODO: Use a public intent, when there is one. + Intent intent = new Intent("com.android.camera.action.CROP"); + intent.setDataAndType(mTakePictureUri, "image/*"); + appendOutputExtra(intent, mCropPictureUri); + appendCropExtras(intent); + try { + StrictMode.disableDeathOnFileUriExposure(); + mAvatarUi.startActivityForResult(intent, REQUEST_CODE_CROP_PHOTO); + } finally { + StrictMode.enableDeathOnFileUriExposure(); } - } finally { - StrictMode.enableDeathOnFileUriExposure(); + } else { + onPhotoNotCropped(mTakePictureUri); } - onPhotoNotCropped(pictureUri); } private void appendOutputExtra(Intent intent, Uri pictureUri) { @@ -325,23 +320,15 @@ class AvatarPhotoController { } @Override - public boolean startSystemActivityForResult(Intent intent, int code) { - ActivityInfo info = intent.resolveActivityInfo(mActivity.getPackageManager(), - PackageManager.MATCH_SYSTEM_ONLY); - if (info == null) { - Log.w(TAG, "No system package activity could be found for code " + code); - return false; - } - intent.setPackage(info.packageName); - mActivity.startActivityForResult(intent, code); - return true; - } - - @Override public int getPhotoSize() { return mActivity.getResources() .getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size); } + + @Override + public boolean canCropPhoto() { + return PhotoCapabilityUtils.canCropPhoto(mActivity); + } } static class ContextInjectorImpl implements ContextInjector { diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/users/AvatarPhotoControllerTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/users/AvatarPhotoControllerTest.java index d988111c29d5..3dc2fabe051a 100644 --- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/users/AvatarPhotoControllerTest.java +++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/users/AvatarPhotoControllerTest.java @@ -34,7 +34,6 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; import android.graphics.Bitmap; -import android.graphics.BitmapFactory; import android.net.Uri; import android.provider.MediaStore; @@ -52,7 +51,6 @@ import org.mockito.MockitoAnnotations; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; @RunWith(AndroidJUnit4.class) @@ -75,7 +73,6 @@ public class AvatarPhotoControllerTest { public void setUp() { MockitoAnnotations.initMocks(this); when(mMockAvatarUi.getPhotoSize()).thenReturn(PHOTO_SIZE); - when(mMockAvatarUi.startSystemActivityForResult(any(), anyInt())).thenReturn(true); mImagesDir = new File( InstrumentationRegistry.getTargetContext().getCacheDir(), "multi_user"); @@ -113,7 +110,9 @@ public class AvatarPhotoControllerTest { } @Test - public void takePhotoIsFollowedByCrop() throws IOException { + public void takePhotoIsFollowedByCropWhenSupported() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(true); + new File(mImagesDir, "file.txt").createNewFile(); Intent intent = new Intent(); @@ -122,12 +121,14 @@ public class AvatarPhotoControllerTest { mController.onActivityResult( REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); - verifyStartSystemActivityForResult( + verifyStartActivityForResult( "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); } @Test public void takePhotoIsNotFollowedByCropWhenResultCodeNotOk() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(true); + new File(mImagesDir, "file.txt").createNewFile(); Intent intent = new Intent(); @@ -137,11 +138,12 @@ public class AvatarPhotoControllerTest { REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_CANCELED, intent); verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt()); - verify(mMockAvatarUi, never()).startSystemActivityForResult(any(), anyInt()); } @Test public void takePhotoIsFollowedByCropWhenTakePhotoUriReturned() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(true); + new File(mImagesDir, "TakeEditUserPhoto.jpg").createNewFile(); Intent intent = new Intent(); @@ -149,12 +151,14 @@ public class AvatarPhotoControllerTest { mController.onActivityResult( REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); - verifyStartSystemActivityForResult( + verifyStartActivityForResult( "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); } @Test public void choosePhotoIsFollowedByCrop() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(true); + new File(mImagesDir, "file.txt").createNewFile(); Intent intent = new Intent(); @@ -163,12 +167,14 @@ public class AvatarPhotoControllerTest { mController.onActivityResult( REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent); - verifyStartSystemActivityForResult( + verifyStartActivityForResult( "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); } @Test public void choosePhotoIsNotFollowedByCropWhenResultCodeNotOk() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(true); + new File(mImagesDir, "file.txt").createNewFile(); Intent intent = new Intent(); @@ -178,11 +184,12 @@ public class AvatarPhotoControllerTest { REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_CANCELED, intent); verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt()); - verify(mMockAvatarUi, never()).startSystemActivityForResult(any(), anyInt()); } @Test public void choosePhotoIsFollowedByCropWhenTakePhotoUriReturned() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(true); + new File(mImagesDir, "TakeEditUserPhoto.jpg").createNewFile(); Intent intent = new Intent(); @@ -190,11 +197,28 @@ public class AvatarPhotoControllerTest { mController.onActivityResult( REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent); - verifyStartSystemActivityForResult( + verifyStartActivityForResult( "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); } @Test + public void choosePhotoIsNotFollowedByCropIntentWhenCropNotSupported() throws IOException { + when(mMockAvatarUi.canCropPhoto()).thenReturn(false); + + File file = new File(mImagesDir, "file.txt"); + saveBitmapToFile(file); + + Intent intent = new Intent(); + intent.setData(Uri.parse( + "content://com.android.settingslib.test/my_cache/multi_user/file.txt")); + mController.onActivityResult( + REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent); + + verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt()); + verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS)).returnUriResult(mCropPhotoUri); + } + + @Test public void cropPhotoResultIsReturnedIfResultOkAndContent() { Intent intent = new Intent(); intent.setData(mCropPhotoUri); @@ -218,58 +242,11 @@ public class AvatarPhotoControllerTest { verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS).times(0)).returnUriResult(mCropPhotoUri); } - @Test - public void cropDoesNotUseTakePhotoUri() throws IOException { - new File(mImagesDir, "file.txt").createNewFile(); - - Intent intent = new Intent(); - intent.setData(Uri.parse( - "content://com.android.settingslib.test/my_cache/multi_user/file.txt")); - mController.onActivityResult( - REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); - - Intent startIntent = verifyStartSystemActivityForResult( - "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); - assertThat(startIntent.getData()).isNotEqualTo(mTakePhotoUri); - } - - @Test - public void internalCropUsedIfNoSystemCropperFound() throws IOException { - when(mMockAvatarUi.startSystemActivityForResult(any(), anyInt())).thenReturn(false); - - File file = new File(mImagesDir, "file.txt"); - saveBitmapToFile(file); - - Intent intent = new Intent(); - intent.setData(Uri.parse( - "content://com.android.settingslib.test/my_cache/multi_user/file.txt")); - mController.onActivityResult( - REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); - - verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS)).returnUriResult(mCropPhotoUri); - - InputStream imageStream = mContext.getContentResolver().openInputStream(mCropPhotoUri); - Bitmap bitmap = BitmapFactory.decodeStream(imageStream); - assertThat(bitmap.getWidth()).isEqualTo(PHOTO_SIZE); - assertThat(bitmap.getHeight()).isEqualTo(PHOTO_SIZE); - } - - private Intent verifyStartActivityForResult(String action, int resultCode) { + private void verifyStartActivityForResult(String action, int resultCode) { ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class); verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS)) .startActivityForResult(captor.capture(), eq(resultCode)); - Intent intent = captor.getValue(); - assertThat(intent.getAction()).isEqualTo(action); - return intent; - } - - private Intent verifyStartSystemActivityForResult(String action, int resultCode) { - ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class); - verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS)) - .startSystemActivityForResult(captor.capture(), eq(resultCode)); - Intent intent = captor.getValue(); - assertThat(intent.getAction()).isEqualTo(action); - return intent; + assertThat(captor.getValue().getAction()).isEqualTo(action); } private void saveBitmapToFile(File file) throws IOException { |