search

nerzhul / ncsms-android

repository for the ncsms on Android
94 stars 38 forks source link

Check & request operation missing before using accountManager.getAccountsByType() #237

Open aper-project opened 4 years ago

aper-project commented 4 years ago

Issue description

Hi, in ncsms-android v1.0.0, we found a dangerous API usage (https://github.com/nerzhul/ncsms-android/blob/master/src/main/java/fr/unix_experience/owncloud_sms/activities/remote_account/RestoreMessagesActivity.java#L64) which requires Manifest.permission.GET_ACCOUNTS in accordance to the Android official documentation (https://developer.android.google.cn/reference/android/accounts/AccountManager?hl=en#getAccountsByType(java.lang.String)).

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

CALLCHAIN:
    fr.unix_experience.owncloud_sms.activities.remote_account.RestoreMessagesActivity.onCreate(android.os.Bundle)void
     android.accounts.AccountManager.getAccountsByType(java.lang.String)android.accounts.Account[]

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.

@nerzhul Could you help me review this issue? Thx

nerzhul commented 4 years ago

hello, obviously it must be fixed, if you have time :)

aper-project commented 4 years ago

Thanks for your reply, but how should we fix it?

I would recommend that we add a checkselfpermission() and a requestpermission() right before the getAccountsByType(), which is the most direct method. I wonder if there is any better strategy, such as adding them in the main activity?