dkanada / openapk

Manage Apps on Android
GNU General Public License v3.0
67 stars 12 forks source link

Check & request operation missing before using Environment.getExternalStorageDirectory() #36

Open aper-project opened 4 years ago

aper-project commented 4 years ago

Issue description

Hi, in openapk v1.2.1, we found a dangerous API usage (https://github.com/dkanada/openapk/blob/master/app/src/main/java/com/dkanada/openapk/utils/AppPreferences.java#L27) which requires Manifest.permission.WRITE_EXTERNAL_STORAGE in accordance to the Android official documentation (https://developer.android.com/reference/android/os/Environment?hl=en#getExternalStorageDirectory()).

However, it seems that it missed the “check” and “request” operation in the following call chain starting from the AppAdapter.onBindViewHolder(AppViewHolder appViewHolder, int i) activity if permission is not granted.

CALLCHAIN:
    com.dkanada.openapk.adapters.AppAdapter$2.onClick(android.view.View)void
     com.dkanada.openapk.utils.ActionUtils.share(android.content.Context,android.content.pm.PackageInfo)boolean
      com.dkanada.openapk.utils.AppPreferences.getCustomPath()java.lang.String
       android.os.Environment.getExternalStorageDirectory()java.io.File

This may lead to a SecurityException or related functions unavailable if the user denies the storage permission but still calls the API in this chain, resulting in bad user experience.

@javiersantos @dkanada Could you help me review this issue? Thx