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

Mitigation required for CVE-2020-12856 #88

Closed
jimmo opened this issue Dec 1, 2020 · 8 comments · Fixed by #206
Closed

Mitigation required for CVE-2020-12856 #88

jimmo opened this issue Dec 1, 2020 · 8 comments · Fixed by #206
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jimmo
Copy link

jimmo commented Dec 1, 2020

Herald acts as a GATT client and reads/writes characteristics and is therefore vunerable to CVE-2020-12856. This is high severity as it allows for permanent tracking of the target device, and can be chained with CVE-2020-0022 to allow RCE.

Google issued a fix for this in the Nov 2020 security bulletin, however it only applies to Android 8+.

Please see https://github.com/alwentiu/COVIDSafe-CVE-2020-12856 for more information on the CVE.

Note that this we originally discovered this in COVIDSafe (and the other derivatives from Singapore's TraceTogether, including ABTraceTogether). COVIDSafe and ABTraceTogether have implemented a workaround (see StreetPassPairingFix.kt) for older Android versions.

It's also worth noting that COVIDSafe's draft Herald integration has applied the same workaround in their modified copy of your code (rather than fixing/notifying upstream). See https://github.com/AU-COVIDSafe/mobile-android/blob/herald/app/src/main/java/au/gov/health/covidsafe/sensor/ble/ConcreteBLEReceiver.kt

@adamfowleruk adamfowleruk self-assigned this Dec 1, 2020
@adamfowleruk adamfowleruk added the enhancement New feature or request label Dec 1, 2020
@adamfowleruk
Copy link
Collaborator

Thanks for this issue. Accepted. We're in discussion with the Australian Government team about their fix for this. Once the licensing of their code is confirmed we will merge it in. We have placeholders already in ConcreteBLESensor.java.

@adamfowleruk adamfowleruk added the good first issue Good for newcomers label Dec 1, 2020
@jimmo
Copy link
Author

jimmo commented Dec 1, 2020

Thanks

It's worth noting that as well as the code workaround to prevent the silent pairing, the non-silent version where you trick the user into clicking pair is very scary too. See the linked PDF for a screenshot from an iPhone.

COVIDSafe has a prominent warning about this on the home screen. Other Herald integrators should be aware of this.

@adamfowleruk
Copy link
Collaborator

Moving this issue to the next release as we have not received the donation of code required as of yet. On a practical basis we do advise adopters to implement the mitigation. A separate documentation issue has been opened on the herald repo to cover this.

@jimmo
Copy link
Author

jimmo commented May 15, 2021

On a practical basis we do advise adopters to implement the mitigation.

Please see abopengov/contact-tracing-Android#11

@adamfowleruk adamfowleruk moved this from To do - iOS to To do - Android in Herald v2.0 Release May 22, 2021
@adamfowleruk
Copy link
Collaborator

We are confirming the StreetPass code license with the original authors (Australia DTO). Hopefully we'll be able to get this contributed as Apache-2.0 so we can just blend it into Herald and not have to worry about other projects.

@adamfowleruk
Copy link
Collaborator

Not received code donation in time for v2.0. Pushing back to v2.1.

@jimmo
Copy link
Author

jimmo commented Jun 4, 2021

This is unbelievable. Are the DTA just not responding to your request? Do you need contact details (this seems highly surprising though, I imagine you've worked closely with them).

But either way... it's not a complicated piece of code. Herald could easily re-implement the same logic. I cannot possibly imagine how you would consider releasing without this fix.

@adamfowleruk
Copy link
Collaborator

Hi Jimmo. Please be aware we are constantly evolving this repo and occasionally some features don't make it to release. It's worth remembering too that this is an Android security flaw remaining unpatched by Google.
As luck would have it, we've managed to squeeze in a fix for this written fresh, so we'll include it in our formal testing tonight, hence the change back to v2.0. This is thanks to the dedicated work of our projects volunteers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants