search

philliphsu / ClockPlus

Alarm clock, timer, and stopwatch application for Android.
https://play.google.com/store/apps/details?id=com.philliphsu.clock2
GNU General Public License v3.0
364 stars 137 forks source link

Check & request operation missing before using RingtoneManager.getCursor() #66

Open aper-project opened 4 years ago

aper-project commented 4 years ago

Issue description

Hi, in ClockPlus v1.1.3, we found a dangerous API usage (https://github.com/philliphsu/ClockPlus/blob/master/app/src/main/java/com/philliphsu/clock2/dialogs/RingtonePickerDialog.java#L85) which requires Manifest.permission.READ_EXTERNAL_STORAGE in accordance to the Android official documentation (https://developer.android.google.cn/reference/android/media/RingtoneManager?hl=en#getCursor()).

However, it seems that it missed the “check” and “request” operation in the following call chain starting from the BaseAlertDialogFragment.onCreateDialog(android.os.Bundle) activity if permission is not granted.

CALLCHAIN:
    com.philliphsu.clock2.dialogs.BaseAlertDialogFragment.onCreateDialog(android.os.Bundle)android.app.Dialog
     com.philliphsu.clock2.dialogs.RingtonePickerDialog.createFrom(android.support.v7.app.AlertDialog$Builder)android.support.v7.app.AlertDialog
      android.media.RingtoneManager.getCursor()android.database.Cursor

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

@philliphsu Could you help me review this issue? Thx