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

Download Location Vulnerability #384

Closed
anoadragon453 opened this issue Feb 2, 2018 · 10 comments
Closed

Download Location Vulnerability #384

anoadragon453 opened this issue Feb 2, 2018 · 10 comments
Milestone

Comments

@anoadragon453
Copy link

anoadragon453 commented Feb 2, 2018

Currently Yalp downloads APKs to a shared storage. This is slightly dangerous, as another app could replace the APK file before you hit Install if Yalp doesn't verify it first.

The solution is to download APKs to Yalp's internal storage, so no other random app on the user's device has access to them.

Originally reported by strncat here: https://reddit.com/comments/7ue3dg/comment/dtjqnvv

@Rikk
Copy link

Rikk commented Feb 3, 2018

  1. Doesn't increase security in rooted phones.
  2. Will be a painful experience for those needing the apk files.
  3. A phone with a malicious app installed will have much more important things to deal with, and it's not fault of YalpStore.

@anoadragon453
Copy link
Author

Those are fair arguments. Can any checking (hashes etc.) be done before install instead?

@Rikk
Copy link

Rikk commented Feb 3, 2018

I can't say about whether it is feasible feature for YalpStore, we need to hear from @yeriomin he is the developer, but I know a healthy Android by itself verifies updates when they are being installed, if the signature of the new package doesn't match the one installed it won't install and will display an error... (not for new packages, however)

@yeriomin
Copy link
Owner

@anoadragon453 @Rikk This is a valid concern. This will have to be done at some point. As an option... which disables a bunch of other options.

Downloading to internal storage would make Yalp Store lose many of its "apk downloader" features. When you download stuff you expect it to be user-accessible. Many things would have to be rewritten. Everything connected to DownloadManager at least.

a healthy Android by itself verifies updates when they are being installed

The only thing verified is the package signature which just tells the os and the user that the update comes from the same developer as the original app.

not for new packages

Yes, if the apk is replaced and the user is careless, anything can be installed.

Can any checking (hashes etc.) be done before install instead?

There is some kind of signature/hash included with the download link by the play store api. Unfortunately, I don't know how this hash is calculated, so I cannot use it to check apk integrity. If represented as a string, it has 27 characters and does not look similar to anything common. Examples:

Wh1nJGRZ51X6piOY0qRZQf-kHBc
o9zYaYCqMnj3i-uUGxy-14-fkCU

It would be nice to know what it is.

@Rikk
Copy link

Rikk commented Feb 18, 2018

Isn't it the Token that allows the user to download from the official Google servers? That's why user needs to login with Google account before downloading...

One way to check apk hash would be: download to somewhere 'inaccessible' to the user and, immediately after that, calculate the hash; but, this makes no sense for compromised systems, yet more for rooted ones. The other way is what APKMirror does: they know the trusted signatures of each developer and check if the signature of the apk file received is the same; also, not possible for us.

@yeriomin
Copy link
Owner

@Rikk

Isn't it the Token that allows the user to download from the official Google servers?

No. Google API has different tokens on almost every step of the way. I don't know the purpose of many things returned in Google API messages.

F-Droid simply downloads to internal storage. Yalp Store can do the same, at the cost of some of its useful features.

@anoadragon453
Copy link
Author

@yeriomin I'm not sure what that hash is either, perhaps the Key Hash, except it's not base64 encoded?

Could we have the option for Yalp to operate in "APK Downloader" mode and "Safe mode", with the latter being the default? Making the option to switch between the two obvious would require some UX design, but it would offer the best of both worlds, but require some work to get there.

@thestinger
Copy link

thestinger commented Feb 25, 2018

DownloadManager can be used to do a private download and saving to internal storage doesn't rule out supporting exporting the apk. An explicit export option could also use https://developer.android.com/guide/topics/providers/document-provider.html rather than needing permission to write to external storage. Rather than using external storage, apps are primarily meant to use internal storage with support for sharing and exporting data. Pervasive usage of external storage is a substantial privacy and security issue for the Android app ecosystem in general.

Yalp Store currently can't be recommended by the CopperheadOS usage guide because it opens up a trivial local privilege escalation vulnerability where an app with external storage access can replace the apk before it's installed. For the first install of an app, there's no meaningful verification. A user has no way to know the apk was replaced.

F-Droid uses signed repositories, so it provides more security for the initial install than just an HTTPS download. That's difficult to do here without figuring out how the Play Store implements it. It wouldn't be any good if it was just a hash instead of a signature though.

yeriomin added a commit that referenced this issue Mar 21, 2018
Apks can now be downloaded to internal storage. If this option is enabled, apk files are additionally hashed as the download goes and the hash is checked before attempting to install.
@yeriomin
Copy link
Owner

Added an option to download to internal storage. When it is on, the downloaded apk is hashed as it is downloaded and this hash is checked before starting installation.

@yeriomin yeriomin added this to the 0.36 milestone Mar 21, 2018
@anoadragon453
Copy link
Author

anoadragon453 commented Mar 21, 2018 via email

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

4 participants