Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
aper-project opened this issue Aug 11, 2020 · 2 comments

Comments

@aper-project
Copy link

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
Copy link
Owner

nerzhul commented Aug 11, 2020

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

@aper-project
Copy link
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants