Question on permissions

[IzzySoft]

My scanner just got a few additional checks included lately, and reported on today's update:

! repo/org.joinmastodon.android.moshinda_104.apk declares risky permission(s): android.permission.CAMERA android.permission.READ_EXTERNAL_STORAGE*

I dug a little through the code and see CAMERA can be explained by the fact that one can "shoot" photos to include with a toot directly from within the app (hope I got that right?). That leaves the READ_EXTERNAL_STORAGE*. The asterisk at its end indicates Android grants that "implicitly", as the app requests WRITE_EXTERNAL_STORAGE. May I ask what that's needed for? And whether you might in fact need READ_INTERNAL_STORAGE as well – or not?

Thanks in advance for clarification!

[FineFindus]

From a quick search through the code, WRITE_EXTERNAL_STORAGE is required for downloading files. When viewing media attachments, e.g. a post with images, the images can be downloaded directly to the downloads directory (without using a thirdparty services like in other social networks), see the corresponding code below https://github.com/LucasGGamerM/moshidon/blob/a1a4c59b8363b88f6ba82fa7085172e0b2ca48e5/mastodon/src/main/java/org/joinmastodon/android/ui/photoviewer/PhotoViewer.java#L571-L573

[IzzySoft]

From a quick search through the code, WRITE_EXTERNAL_STORAGE is required for downloading files.

Thanks for digging that up, @FineFindus! I'm not 100% sure (can one ever be sure with all the exceptions ans shenigans this permission and file-system-handling have?) – but maybe using a file picker (and SAF) would be a good idea there? Then on first download, that would open a pop up and ask where to store downloaded files.

Not saying this has to be done, just thinking aloud.

I see line 573 would probably need the READ permission if I'm not mistaken. So if the current handling is being kept, maybe that should be requested explicitly then – and I'd add that to the allow-list with the explanation "needed to handle downloads" (or something alike – suggestions welcome).

[FineFindus]

but maybe using a file picker (and SAF) would be a good idea there? Then on first download, that would open a pop up and ask where to store downloaded files.

That's probably possible, but it might be a good idea to ask upstream (the official app), since they wrote the code and are more familiar with the quirks and how it should work.

I see line 573 would probably need the READ permission if I'm not mistaken. So if the current handling is being kept, maybe that should be requested explicitly then – and I'd add that to the allow-list with the explanation "needed to handle downloads" (or something alike – suggestions welcome).

I'm not sure, the Android docs don't mention that it needs permission, so I would assume no.

[IzzySoft]

it might be a good idea to ask upstream

Agreed. Who will do that?

the Android docs don't mention

They neither mention the implicit grant, but still it's happening.

For clarification from my side: I'm not saying those permissions must be removed. If they are justified as needed, I'm fine with an explanation/reason to show along them. From what I've learned since I started reporting those, on Android 5-9 it's quite a mess of exceptions and combinations on when you need those "compatibility permissions" and when not. So if it's clear they are needed, an explanation like "needed for compatibility reasons with Android 9 and below" would do.

[FineFindus]

Sorry for the late reply, I've been a bit busy lately.

Agreed. Who will do that?

I'm happy to let you do it :)

Agreed. Who will do it?

Fair point. I tested it on my test device running Android 13 (or 14?), it works when I manually remove the permission. There is also this stackoverflow post which suggests that the permission is required on earlier versions of Android.

[IzzySoft]

Sorry for the late reply, I've been a bit busy lately.

No prob – so was/am I (currently working hard to finish the current batch of improvements for my repo – apart from also having a dayjob)…

I'm happy to let you do it :)

Haha, nice joke :rofl:

I tested it on my test device running Android 13 (or 14?)

No prob removing it for 13+. The trouble with that one lies before. Storage access is quite a mess up to 9 at least (I've learned a thing or two about that the last 2 weeks, since I'm sending out those reports).

So let me make an "educated guess": Moshidon needs storage access so you can attach a photo from your gallery? OK, then I give up, and simply add it to the allow-list. The app supports Android 6+ – and with 9 that permission is still needed to access media directories. After 9, the fun starts on the other end – then you'll need MANAGE_EXTERNAL_STORAGE if you want to access the Downloads/ :see_no_evil:

Google says: local storage bad for business. Cloud is much better, so we can protect your files… </s>

[IzzySoft]

Oof, now there's an additional one which might turn out to be problematic: What was android.permission.REQUEST_INSTALL_PACKAGES added for? I honestly hope you didn't add some self-updater, as that would violate the inclusion policy of F-Droid as well as the one of IzzyOnDroid. @LucasGGamerM?

[LucasGGamerM]

This one is for the little dohicky with the browser selection dialog. I agree we can just remove it

[LucasGGamerM]

Though how do you believe I should deal with this? Should I make a new release? What do you think?

[IzzySoft]

A new release would clear it out, yes (I'm just catching up with the backlog which has built up while I was on a short vacation – seems REQUEST_INSTALL_PACKAGES was already removed with versionCode: 105?). Report for 105 just reads:

! repo/org.joinmastodon.android.moshinda_105.apk declares sensitive permission(s): android.permission.READ_EXTERNAL_STORAGE*
! repo/org.joinmastodon.android.moshinda_105.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

So it's READ_EXTERNAL_STORAGE (being implicitly granted due to ẀRITE_EXTERNAL_STORAGEbeing requested) and theDEPENDENCY_INFO_BLOCK`, which can be avoided easily:

android {
    dependenciesInfo {
        // Disables dependency metadata when building APKs.
        includeInApk = false
        // Disables dependency metadata when building Android App Bundles.
        includeInBundle = false
    }
}

For some background: that BLOB is supposed to be just a binary representation of your app's dependency tree. But as it's encrypted with a public key belonging to Google, only Google can read it – and nobody else can even verify what it really contains.

[LucasGGamerM]

Anyhow, will v105 land on fdroid if I don't make these changes immediately?

[IzzySoft]

F-Droid? Since when do they check this? I've been preaching for years to integrate the scanner with their build system, to no avail. And wouldn't it be for the IzzyOnDroid repo having some overlap with theirs, many things would have passed into theirs unnoticed, only to be found by some other scan a year later or so.

But could you meanwhile let me know about the storage permission? I guess it's for picking media to attach to posts?

[IzzySoft]

@LucasGGamerM may I ask for the status here? Just got the report again for the new release:

! repo/org.joinmastodon.android.moshinda_106.apk declares sensitive permission(s): android.permission.READ_EXTERNAL_STORAGE*
! repo/org.joinmastodon.android.moshinda_106.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)