Veracode Scan of the app results into "377:Insecure Temporary File"

[MGohil]

When we scan the app through Veracode to check for any static code vulnerabilities, it gives the "Insecure Temporary File". Following is detail and also provides remedies on how to fix this.

This error points to BitmapUtils.kt Line 461 at: https://github.com/CanHub/Android-Image-Cropper/blob/05b4d586335d6ee652f487881efd0cb54b36c43b/cropper/src/main/kotlin/com/canhub/cropper/BitmapUtils.kt#L461

Which is main reason of this issue reported by Veracode static screen.

I am not using this library directly into any native app, but using it into .NET Maui app via one of the Binding Library which originally uses this native Android library.

I have reported similar issue there too: https://github.com/jmbowman1107/ImageCropper.Maui/issues/28 but even after updating to 4.6.0 didn't resolve it.

Would you please have a look and fix this please, so we can update to latest .aar and build our .NET Maui supported binary?

Thanks, Milan G

[MGohil]

@vanniktech @Canato

I would appreciate if someone can look into this issue please? Fixing this issue is required to get our app into production. Please let me know in case you need any more details.

Thanks.

[vanniktech]

Happy to take a PR that fixes this behavior. I'm not even sure this function is actually used in your case.

[MGohil]

@vanniktech Thanks for your response, unfortunately, it will be difficult for me to fix this and raise PR as I am not a Kotlin guy. Will it be possible for you to try removing this last parameter from below method and keeping first two?

val file = File.createTempFile(
            "cropped",
            ext,
            context.getExternalFilesDir(Environment.DIRECTORY_PICTURES),
          )

I am not sure what will affect if we remove this last parameter, but the solution to above issue tells us to use createTempFile method which has only 2 parameters.

--OR--

If we want to use 3 argument version of this method, then we have to pass SANBOXED path in 3rd argument in this method. I am bit not sure as of now on this what path to pass here.

[vanniktech]

It might. I also don't know how this method is used currently. I took the library over and haven't had time yet to unravel all of those nested Utils calls.

If we want to use 3 argument version of this method, then we have to pass SANBOXED path in 3rd argument in this method. I am bit not sure as of now on this what path to pass here.

If that's supported by Android it might be the best option.

[MGohil]

@vanniktech

Just checking if is there any progress on this? May we get the above fix soon? Would you please let me know about your plan on this?

This is very crucial for our app to fix this security issue raised by Veracode scanning.

Thanks, Milan G.

[vanniktech]

Look, this project is open source. I have no obligations and neither does any other open source author. You're free to use this library as is or not. I have no plans about this. My priorities are currently elsewhere. If it is so urgent to you, you could fix it yourself and raise a PR that fixes this in a way it's not breaking anyone else and everyone could benefit from the fix.

[M66B]

The reason the scanner reports this is probably because of using getExternalFilesDir:

"There is no security enforced with these files. For example, any application holding Manifest.permission.WRITE_EXTERNAL_STORAGE can write to these files."

https://developer.android.com/reference/android/content/Context#getExternalFilesDir(java.lang.String)

The reason this workaround is being used is documented:

// We have this because of a HUAWEI path bug when we use getUriForFile

This issue can be resolved like this so that this workaround isn't needed anymore:

public class CropFileProvider extends FileProvider {
    public FileProviderEx() {
        super(R.xml.library_file_paths);
    }

Please see here why this is needed:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1978527

Some OEMs strip meta-data from the manifest. This is problematic as FileProvider was depending on meta-data to specify a resource containing the paths that should be shared.

Also, better rename library_file_paths into something like cropper_library_file_paths to prevent name clashes.

[vanniktech]

Please test out the snapshot @MGohil