Merge "Prevent exfiltration of system files via avatar picker." into tm-dev am: 5489aa365f

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/19754655

Change-Id: Ia0cefeb6938b14976f34559f4b1d3e4cda5c029a
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

packages/SettingsLib/src/com/android/settingslib/users/AvatarPhotoController.java

@@ -21,6 +21,8 @@ import android.content.ClipData;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
+import android.content.pm.PackageManager;
+import android.content.pm.ResolveInfo;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.graphics.Canvas;
@@ -46,6 +48,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.List;
 import java.util.concurrent.ExecutionException;
 
 class AvatarPhotoController {
@@ -57,9 +60,9 @@ class AvatarPhotoController {
 
         void startActivityForResult(Intent intent, int resultCode);
 
-        int getPhotoSize();
+        boolean startSystemActivityForResult(Intent intent, int resultCode);
 
-        boolean canCropPhoto();
+        int getPhotoSize();
     }
 
     interface ContextInjector {
@@ -82,6 +85,7 @@ 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";
 
@@ -91,6 +95,7 @@ class AvatarPhotoController {
     private final ContextInjector mContextInjector;
 
     private final File mImagesDir;
+    private final Uri mPreCropPictureUri;
     private final Uri mCropPictureUri;
     private final Uri mTakePictureUri;
 
@@ -100,6 +105,8 @@ 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 =
@@ -131,7 +138,7 @@ class AvatarPhotoController {
                 return true;
             case REQUEST_CODE_TAKE_PHOTO:
                 if (mTakePictureUri.equals(pictureUri)) {
-                    cropPhoto();
+                    cropPhoto(pictureUri);
                 } else {
                     copyAndCropPhoto(pictureUri, false);
                 }
@@ -160,7 +167,7 @@ class AvatarPhotoController {
             ThreadUtils.postOnBackgroundThread(() -> {
                 final ContentResolver cr = mContextInjector.getContentResolver();
                 try (InputStream in = cr.openInputStream(pictureUri);
-                     OutputStream out = cr.openOutputStream(mTakePictureUri)) {
+                        OutputStream out = cr.openOutputStream(mPreCropPictureUri)) {
                     Streams.copy(in, out);
                 } catch (IOException e) {
                     Log.w(TAG, "Failed to copy photo", e);
@@ -168,7 +175,7 @@ class AvatarPhotoController {
                 }
                 Runnable cropRunnable = () -> {
                     if (!mAvatarUi.isFinishing()) {
-                        cropPhoto();
+                        cropPhoto(mPreCropPictureUri);
                     }
                 };
                 if (delayBeforeCrop) {
@@ -183,22 +190,21 @@ class AvatarPhotoController {
         }
     }
 
-    private void cropPhoto() {
+    private void cropPhoto(final Uri pictureUri) {
-        if (mAvatarUi.canCropPhoto()) {
+        // TODO: Use a public intent, when there is one.
-            // TODO: Use a public intent, when there is one.
+        Intent intent = new Intent("com.android.camera.action.CROP");
-            Intent intent = new Intent("com.android.camera.action.CROP");
+        intent.setDataAndType(pictureUri, "image/*");
-            intent.setDataAndType(mTakePictureUri, "image/*");
+        appendOutputExtra(intent, mCropPictureUri);
-            appendOutputExtra(intent, mCropPictureUri);
+        appendCropExtras(intent);
-            appendCropExtras(intent);
+        try {
-            try {
+            StrictMode.disableDeathOnFileUriExposure();
-                StrictMode.disableDeathOnFileUriExposure();
+            if (mAvatarUi.startSystemActivityForResult(intent, REQUEST_CODE_CROP_PHOTO)) {
-                mAvatarUi.startActivityForResult(intent, REQUEST_CODE_CROP_PHOTO);
+                return;
-            } finally {
-                StrictMode.enableDeathOnFileUriExposure();
             }
-        } else {
+        } finally {
-            onPhotoNotCropped(mTakePictureUri);
+            StrictMode.enableDeathOnFileUriExposure();
         }
+        onPhotoNotCropped(pictureUri);
     }
 
     private void appendOutputExtra(Intent intent, Uri pictureUri) {
@@ -320,14 +326,22 @@ class AvatarPhotoController {
         }
 
         @Override
-        public int getPhotoSize() {
+        public boolean startSystemActivityForResult(Intent intent, int code) {
-            return mActivity.getResources()
+            List<ResolveInfo> resolveInfos = mActivity.getPackageManager()
-                    .getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size);
+                    .queryIntentActivities(intent, PackageManager.MATCH_SYSTEM_ONLY);
+            if (resolveInfos.isEmpty()) {
+                Log.w(TAG, "No system package activity could be found for code " + code);
+                return false;
+            }
+            intent.setPackage(resolveInfos.get(0).activityInfo.packageName);
+            mActivity.startActivityForResult(intent, code);
+            return true;
         }
 
         @Override
-        public boolean canCropPhoto() {
+        public int getPhotoSize() {
-            return PhotoCapabilityUtils.canCropPhoto(mActivity);
+            return mActivity.getResources()
+                    .getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size);
         }
     }
 

packages/SettingsLib/tests/integ/src/com/android/settingslib/users/AvatarPhotoControllerTest.java

@@ -34,6 +34,7 @@ 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;
 
@@ -51,6 +52,7 @@ 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)
@@ -73,6 +75,7 @@ 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");
@@ -110,9 +113,7 @@ public class AvatarPhotoControllerTest {
     }
 
     @Test
-    public void takePhotoIsFollowedByCropWhenSupported() throws IOException {
+    public void takePhotoIsFollowedByCrop() throws IOException {
-        when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
-
         new File(mImagesDir, "file.txt").createNewFile();
 
         Intent intent = new Intent();
@@ -121,14 +122,12 @@ public class AvatarPhotoControllerTest {
         mController.onActivityResult(
                 REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent);
 
-        verifyStartActivityForResult(
+        verifyStartSystemActivityForResult(
                 "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();
@@ -138,12 +137,11 @@ 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();
@@ -151,14 +149,12 @@ public class AvatarPhotoControllerTest {
         mController.onActivityResult(
                 REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent);
 
-        verifyStartActivityForResult(
+        verifyStartSystemActivityForResult(
                 "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();
@@ -167,14 +163,12 @@ public class AvatarPhotoControllerTest {
         mController.onActivityResult(
                 REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent);
 
-        verifyStartActivityForResult(
+        verifyStartSystemActivityForResult(
                 "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();
@@ -184,12 +178,11 @@ 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();
@@ -197,27 +190,10 @@ public class AvatarPhotoControllerTest {
         mController.onActivityResult(
                 REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent);
 
-        verifyStartActivityForResult(
+        verifyStartSystemActivityForResult(
                 "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();
@@ -242,11 +218,58 @@ public class AvatarPhotoControllerTest {
         verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS).times(0)).returnUriResult(mCropPhotoUri);
     }
 
-    private void verifyStartActivityForResult(String action, int resultCode) {
+    @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) {
         ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
         verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS))
                 .startActivityForResult(captor.capture(), eq(resultCode));
-        assertThat(captor.getValue().getAction()).isEqualTo(action);
+        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;
     }
 
     private void saveBitmapToFile(File file) throws IOException {