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

What for is READ_EXTERNAL_STORAGE needed? #101

Closed
IzzySoft opened this issue Jan 22, 2024 · 4 comments
Closed

What for is READ_EXTERNAL_STORAGE needed? #101

IzzySoft opened this issue Jan 22, 2024 · 4 comments

Comments

@IzzySoft
Copy link

My scanner just got some improvements, and for today's release reported READ_EXTERNAL_STORAGE as sensitive permission (plus a few others which were immediately clear to me and already got added to your app's allow-list). Can you please clarify what this permission is needed for? With your app targeting Android 14 and needing at least Android 10 to run, this looks a bit confusing to a non-developer like me 😉

Thanks in advance!

@Semper-Viventem
Copy link
Owner

Hey @IzzySoft. Good catch. I added this permission almost a year ago to support database backups and restores from the external storage and this permission shouldn't be a surprise for the particular release today. But it turned out that I don't need to access the whole storage (as before) because for now the app can get read/write access for particular files from the system. I'd add that READ_EXTERNAL_STORAGE has no effect since Android 13 and we can remove it.

Thank you for noticing, you can find the manifest update in this PR: #102

@IzzySoft
Copy link
Author

this permission shouldn't be a surprise for the particular release today

No, definitely not. Which is why I wrote "My scanner just got some improvements". Those were fully enabled this weekend, and thus pop up for all updates since today – which is why today's release was the first one it was noticed for.

and we can remove it

Yay!

you can find the manifest update in this PR

(looking) Ah. Fun fact: I'd like a reference to where READ_INTERNAL_STORAGE is documented. Seeing it in apps for almost 10 years now, but I never found a reference. My guess so far was that it was erroneously deduced from READ_EXTERNAL_STORAGE, made it into some example code, and spread from there. Honestly curious if now, after all those years, I can learn its origin from you?

@Semper-Viventem
Copy link
Owner

No, definitely not. Which is why I wrote "My scanner just got some improvements". Those were fully enabled this weekend, and thus pop up for all updates since today – which is why today's release was the first one it was noticed for.

That's interesting because the permission has definitely been added in Feb 2023 in this commit (91ca8655).

My guess so far was that it was erroneously deduced from READ_EXTERNAL_STORAGE, made it into some example code, and spread from there. Honestly curious if now, after all those years, I can learn its origin from you?

Yes, I cannot find READ_INTERNAL_STORAGE permission either in the Android documentation or the AOSP repository. It seems that this permission has been mistakenly mentioned in some old documentation or examples and spilled out across all apps (even in the one of the AOSP apps, lol)

@IzzySoft
Copy link
Author

That's interesting because the permission has definitely been added in Feb 2023

At that time the additional APK checks were not yet in place, so today's release was the first APK of your app that was hit by them, because the others were not subject to the then not existing checks. Only updates (and new additions) are scanned by those. I get more than enough reports already that way. Once the reports slow down, I'll most likely check the older ones, too. But then again your previous releases won't be reported as those things are already on their allow-list 😉

this permission has been mistakenly mentioned in some old documentation or examples and spilled out across all apps

Haha! OK, so my conclusion still holds. Yes, there was one very famous piece of sample code having that mentioned. There are several more examples. I wonder I didn't put this one in there… Oof, I see I should check on those unknown ones again. Some of them sound funny, too: FOREGROUND_SERVICE_LOCATION … wait, they really did that? WTF… Added with sdk 34, wow. Lots of fun ahead updating my own reference then…

even in the one of the AOSP apps

ROFL 🙈

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