Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prevent exfiltration of system files via avatar picker.

Browse Source 
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.

Bug: 187702830
Test: atest AvatarPhotoControllerTest
Change-Id: Idf1ab60878d619ee30505d71e8afe31d8b0c0ebe
This commit is contained in:
Oli Lan
2022-02-23 13:12:20 +00:00
parent e8e79f8900
commit 1325574d3a
2 changed files with 100 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
packages/SettingsLib/src/com/android/settingslib/users/AvatarPhotoController.java
View File

@@ -21,6 +21,8 @@ import android.content.ClipData;
import android.content.ContentResolver; import android.content.ContentResolver;
import android.content.Context; import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.content.pm.ActivityInfo;
import android.content.pm.PackageManager;
import android.graphics.Bitmap; import android.graphics.Bitmap;
import android.graphics.BitmapFactory; import android.graphics.BitmapFactory;
import android.graphics.Canvas; import android.graphics.Canvas;
@@ -57,9 +59,9 @@ class AvatarPhotoController {
void startActivityForResult(Intent intent, int resultCode); void startActivityForResult(Intent intent, int resultCode);
int getPhotoSize(); boolean startSystemActivityForResult(Intent intent, int resultCode);
boolean canCropPhoto(); int getPhotoSize();
} }
interface ContextInjector { interface ContextInjector {
@@ -77,6 +79,7 @@ class AvatarPhotoController {
static final int REQUEST_CODE_CROP_PHOTO = 1003; static final int REQUEST_CODE_CROP_PHOTO = 1003;
private static final String IMAGES_DIR = "multi_user"; 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 CROP_PICTURE_FILE_NAME = "CropEditUserPhoto.jpg";
private static final String TAKE_PICTURE_FILE_NAME = "TakeEditUserPhoto.jpg"; private static final String TAKE_PICTURE_FILE_NAME = "TakeEditUserPhoto.jpg";
@@ -86,6 +89,7 @@ class AvatarPhotoController {
private final ContextInjector mContextInjector; private final ContextInjector mContextInjector;
private final File mImagesDir; private final File mImagesDir;
private final Uri mPreCropPictureUri;
private final Uri mCropPictureUri; private final Uri mCropPictureUri;
private final Uri mTakePictureUri; private final Uri mTakePictureUri;
@@ -95,6 +99,8 @@ class AvatarPhotoController {
mImagesDir = new File(mContextInjector.getCacheDir(), IMAGES_DIR); mImagesDir = new File(mContextInjector.getCacheDir(), IMAGES_DIR);
mImagesDir.mkdir(); mImagesDir.mkdir();
mPreCropPictureUri = mContextInjector
.createTempImageUri(mImagesDir, PRE_CROP_PICTURE_FILE_NAME, !waiting);
mCropPictureUri = mCropPictureUri =
mContextInjector.createTempImageUri(mImagesDir, CROP_PICTURE_FILE_NAME, !waiting); mContextInjector.createTempImageUri(mImagesDir, CROP_PICTURE_FILE_NAME, !waiting);
mTakePictureUri = mTakePictureUri =
@@ -127,7 +133,7 @@ class AvatarPhotoController {
case REQUEST_CODE_TAKE_PHOTO: case REQUEST_CODE_TAKE_PHOTO:
case REQUEST_CODE_CHOOSE_PHOTO: case REQUEST_CODE_CHOOSE_PHOTO:
if (mTakePictureUri.equals(pictureUri)) { if (mTakePictureUri.equals(pictureUri)) {
cropPhoto(); cropPhoto(pictureUri);
} else { } else {
copyAndCropPhoto(pictureUri); copyAndCropPhoto(pictureUri);
} }
@@ -153,7 +159,7 @@ class AvatarPhotoController {
ThreadUtils.postOnBackgroundThread(() -> { ThreadUtils.postOnBackgroundThread(() -> {
final ContentResolver cr = mContextInjector.getContentResolver(); final ContentResolver cr = mContextInjector.getContentResolver();
try (InputStream in = cr.openInputStream(pictureUri); try (InputStream in = cr.openInputStream(pictureUri);
OutputStream out = cr.openOutputStream(mTakePictureUri)) { OutputStream out = cr.openOutputStream(mPreCropPictureUri)) {
Streams.copy(in, out); Streams.copy(in, out);
} catch (IOException e) { } catch (IOException e) {
Log.w(TAG, "Failed to copy photo", e); Log.w(TAG, "Failed to copy photo", e);
@@ -161,7 +167,7 @@ class AvatarPhotoController {
} }
ThreadUtils.postOnMainThread(() -> { ThreadUtils.postOnMainThread(() -> {
if (!mAvatarUi.isFinishing()) { if (!mAvatarUi.isFinishing()) {
cropPhoto(); cropPhoto(mPreCropPictureUri);
} }
}); });
}).get(); }).get();
@@ -170,22 +176,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) { private void appendOutputExtra(Intent intent, Uri pictureUri) {
@@ -307,14 +312,22 @@ class AvatarPhotoController {
} }
@Override @Override
public int getPhotoSize() { public boolean startSystemActivityForResult(Intent intent, int code) {
return mActivity.getResources() ActivityInfo info = intent.resolveActivityInfo(mActivity.getPackageManager(),
.getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size); 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 @Override
public boolean canCropPhoto() { public int getPhotoSize() {
return PhotoCapabilityUtils.canCropPhoto(mActivity); return mActivity.getResources()
.getDimensionPixelSize(com.android.internal.R.dimen.user_icon_size);
} }
} }

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

@@ -75,6 +75,7 @@ public class AvatarPhotoControllerTest {
public void setUp() { public void setUp() {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
when(mMockAvatarUi.getPhotoSize()).thenReturn(PHOTO_SIZE); when(mMockAvatarUi.getPhotoSize()).thenReturn(PHOTO_SIZE);
when(mMockAvatarUi.startSystemActivityForResult(any(), anyInt())).thenReturn(true);
mImagesDir = new File( mImagesDir = new File(
InstrumentationRegistry.getTargetContext().getCacheDir(), "multi_user"); InstrumentationRegistry.getTargetContext().getCacheDir(), "multi_user");
@@ -108,9 +109,7 @@ public class AvatarPhotoControllerTest {
} }
@Test @Test
public void takePhotoIsFollowedByCropWhenSupported() throws IOException { public void takePhotoIsFollowedByCrop() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
new File(mImagesDir, "file.txt").createNewFile(); new File(mImagesDir, "file.txt").createNewFile();
Intent intent = new Intent(); Intent intent = new Intent();
@@ -119,14 +118,12 @@ public class AvatarPhotoControllerTest {
mController.onActivityResult( mController.onActivityResult(
REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent);
verifyStartActivityForResult( verifyStartSystemActivityForResult(
"com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO);
} }
@Test @Test
public void takePhotoIsNotFollowedByCropWhenResultCodeNotOk() throws IOException { public void takePhotoIsNotFollowedByCropWhenResultCodeNotOk() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
new File(mImagesDir, "file.txt").createNewFile(); new File(mImagesDir, "file.txt").createNewFile();
Intent intent = new Intent(); Intent intent = new Intent();
@@ -136,12 +133,11 @@ public class AvatarPhotoControllerTest {
REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_CANCELED, intent); REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_CANCELED, intent);
verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt()); verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt());
verify(mMockAvatarUi, never()).startSystemActivityForResult(any(), anyInt());
} }
@Test @Test
public void takePhotoIsFollowedByCropWhenTakePhotoUriReturned() throws IOException { public void takePhotoIsFollowedByCropWhenTakePhotoUriReturned() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
new File(mImagesDir, "TakeEditUserPhoto.jpg").createNewFile(); new File(mImagesDir, "TakeEditUserPhoto.jpg").createNewFile();
Intent intent = new Intent(); Intent intent = new Intent();
@@ -149,13 +145,13 @@ public class AvatarPhotoControllerTest {
mController.onActivityResult( mController.onActivityResult(
REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent);
verifyStartActivityForResult( verifyStartSystemActivityForResult(
"com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO);
} }
@Test @Test
public void takePhotoIsNotFollowedByCropIntentWhenCropNotSupported() throws IOException { public void takePhotoIsNotFollowedByCropIntentWhenCropNotSupported() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(false); when(mMockAvatarUi.startSystemActivityForResult(any(), anyInt())).thenReturn(false);
File file = new File(mImagesDir, "file.txt"); File file = new File(mImagesDir, "file.txt");
saveBitmapToFile(file); saveBitmapToFile(file);
@@ -167,33 +163,11 @@ public class AvatarPhotoControllerTest {
REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent); REQUEST_CODE_TAKE_PHOTO, Activity.RESULT_OK, intent);
verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt()); verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt());
verify(mMockAvatarUi, never()).startSystemActivityForResult(any(), anyInt());
} }
@Test @Test
public void takePhotoIsFollowedByInternalCropWhenCropNotSupported() throws IOException { public void choosePhotoIsFollowedByCrop() 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_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);
}
@Test
public void choosePhotoIsFollowedByCropWhenSupported() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
new File(mImagesDir, "file.txt").createNewFile(); new File(mImagesDir, "file.txt").createNewFile();
Intent intent = new Intent(); Intent intent = new Intent();
@@ -202,14 +176,12 @@ public class AvatarPhotoControllerTest {
mController.onActivityResult( mController.onActivityResult(
REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent); REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent);
verifyStartActivityForResult( verifyStartSystemActivityForResult(
"com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); "com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO);
} }
@Test @Test
public void choosePhotoIsNotFollowedByCropWhenResultCodeNotOk() throws IOException { public void choosePhotoIsNotFollowedByCropWhenResultCodeNotOk() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
new File(mImagesDir, "file.txt").createNewFile(); new File(mImagesDir, "file.txt").createNewFile();
Intent intent = new Intent(); Intent intent = new Intent();
@@ -219,12 +191,11 @@ public class AvatarPhotoControllerTest {
REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_CANCELED, intent); REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_CANCELED, intent);
verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt()); verify(mMockAvatarUi, never()).startActivityForResult(any(), anyInt());
verify(mMockAvatarUi, never()).startSystemActivityForResult(any(), anyInt());
} }
@Test @Test
public void choosePhotoIsFollowedByCropWhenTakePhotoUriReturned() throws IOException { public void choosePhotoIsFollowedByCropWhenTakePhotoUriReturned() throws IOException {
when(mMockAvatarUi.canCropPhoto()).thenReturn(true);
new File(mImagesDir, "TakeEditUserPhoto.jpg").createNewFile(); new File(mImagesDir, "TakeEditUserPhoto.jpg").createNewFile();
Intent intent = new Intent(); Intent intent = new Intent();
@@ -232,27 +203,10 @@ public class AvatarPhotoControllerTest {
mController.onActivityResult( mController.onActivityResult(
REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent); REQUEST_CODE_CHOOSE_PHOTO, Activity.RESULT_OK, intent);
verifyStartActivityForResult( verifyStartSystemActivityForResult(
"com.android.camera.action.CROP", REQUEST_CODE_CROP_PHOTO); "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 @Test
public void cropPhotoResultIsReturnedIfResultOkAndContent() { public void cropPhotoResultIsReturnedIfResultOkAndContent() {
Intent intent = new Intent(); Intent intent = new Intent();
@@ -277,11 +231,61 @@ public class AvatarPhotoControllerTest {
verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS).times(0)).returnUriResult(mCropPhotoUri); 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);
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);
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); ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS)) verify(mMockAvatarUi, timeout(TIMEOUT_MILLIS))
.startActivityForResult(captor.capture(), eq(resultCode)); .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 { private void saveBitmapToFile(File file) throws IOException {