Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b76141eb authored by Oli Lan's avatar Oli Lan
Browse files

Prevent exfiltration of system files via avatar picker. 

This adds mitigations to prevent system files being exfiltrated
via the settings content provider when a content URI is provided
as a chosen user image.

The mitigations are:

1) Copy the image to a new URI rather than the existing takePictureUri
prior to cropping.

2) Only allow a system handler to respond to the CROP intent.

This is a fixed version of ag/17005706, to address b/239513606.

Bug: 187702830
Test: atest AvatarPhotoControllerTest
Change-Id: I21f1b25154dc00a305bdadb96fdf22edff31d9b8
parent f18aa11d

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

+38 −24
Original line number Diff line number Diff line
@@ -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() {

        if (mAvatarUi.canCropPhoto()) {

    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(mTakePictureUri, "image/*");

        intent.setDataAndType(pictureUri, "image/*");

        appendOutputExtra(intent, mCropPictureUri);

        appendCropExtras(intent);

        try {

            StrictMode.disableDeathOnFileUriExposure();

                mAvatarUi.startActivityForResult(intent, REQUEST_CODE_CROP_PHOTO);

            if (mAvatarUi.startSystemActivityForResult(intent, REQUEST_CODE_CROP_PHOTO)) {

                return;

            }

        } finally {

            StrictMode.enableDeathOnFileUriExposure();

        }

        } else {

            onPhotoNotCropped(mTakePictureUri);

        }

        onPhotoNotCropped(pictureUri);

    }



    private void appendOutputExtra(Intent intent, Uri pictureUri) {
@@ -320,14 +326,22 @@ class AvatarPhotoController { 
        }



        @Override

        public int getPhotoSize() {

            return mActivity.getResources()

                    .getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size);

        public boolean startSystemActivityForResult(Intent intent, int code) {

            List<ResolveInfo> resolveInfos = mActivity.getPackageManager()

                    .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() {

            return PhotoCapabilityUtils.canCropPhoto(mActivity);

        public int getPhotoSize() {

            return mActivity.getResources()

                    .getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size);

        }

    }

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

+59 −36
Original line number Diff line number Diff line
@@ -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 {

        when(mMockAvatarUi.canCropPhoto()).thenReturn(true);



    public void takePhotoIsFollowedByCrop() throws IOException {

        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 {