nic-delhi / AarogyaSetu_Android

Aarogya Setu Android app native code
https://www.aarogyasetu.gov.in/
Other
2.89k stars 1.94k forks source link

Do Check for Mock Location #13

Closed dorky-handler closed 4 years ago

dorky-handler commented 4 years ago

Since there isn't any method called to check whether mock location is enabled or not, hackers/miscreants can use any mock location app to hide their real location and report infection from this fake location. Since the program does not have any checks for Temporary phone numbers , A user could register/login using a temporary phone number hide his location and report a fake covid infection case. adding this simple permission check will prevent this

public static boolean isMockSettingsON(Context context) { // returns true if mock location enabled, false if not enabled. if (Settings.Secure.getString(context.getContentResolver(), Settings.Secure.ALLOW_MOCK_LOCATION).equals("0")) return false; else return true; }

0xSumitBanik commented 4 years ago

Good one. This needs to be highlighted.

ckcr4lyf commented 4 years ago

Can't you remove the "MOCK LOCATION" property when faking on a rooted device?

tachyons commented 4 years ago

Haven't checked implementation details, but a hackers/miscreants can still send arbitrary location by altering the code,right?. Since api endpoint remains the same

dorky-handler commented 4 years ago

on /nic/goi/aarogyasetu/location/RetrieveLocationService

Location should be filtered using 
boolean isMock = false;
if (android.os.Build.VERSION.SDK_INT >= 18) {
    isMock = location.isFromMockProvider();
} else {
    isMock = !Settings.Secure.getString(context.getContentResolver(), Settings.Secure.ALLOW_MOCK_LOCATION).equals("0");
}

or using it's kotlin equivalent

dorky-handler commented 4 years ago

Haven't checked implementation details, but a hackers/miscreants can still send arbitrary location by altering the code,right?. Since api endpoint remains the same

Yes,they can by using dex2jar and editing&recompiling unless tampering detection is implemented using Certificate/signature verification,which I think they've already added

Animesh-Ghosh commented 4 years ago

@AhasHDas you can format your code using Markdown. Like so:

// returns true if mock location enabled, false if not enabled.
public static boolean isMockSettingsON(Context context) {
    if (Settings.Secure.getString(context.getContentResolver(), Settings.Secure.ALLOW_MOCK_LOCATION).equals("0"))
        return false;
    else
        return true;
}
dorky-handler commented 4 years ago

@AhasHDas you can format your code using Markdown. Like so:

// returns true if mock location enabled, false if not enabled.
public static boolean isMockSettingsON(Context context) {
    if (Settings.Secure.getString(context.getContentResolver(), Settings.Secure.ALLOW_MOCK_LOCATION).equals("0"))
        return false;
    else
        return true;
}

duly noted...

arpanbag001 commented 4 years ago

What's the problem with rooted devices? It's not a banking app! Why would anyone spoof location on such an app?

And given your scenario, even without using mock location, users can submit fake covid19 cases in their own location, and I think this is the issue we should address first. Users should not be able to submit covid19 case, without any proof, which should be verified before being accepted.

So why exactly do you think users of rooted devices do not deserve protection?

I own several devices, which are rooted as I use them for development and stuff, and blocking rooted devices will simply make me not use the app. And I'm sure, many many other users will agree. So, I think it's not logical to block rooted devices on every single app, even when it's unnecessary.

criticic commented 4 years ago

Why would anyone spoof location on such an app?

By spoofing locations, a user might go near a verified covid patient also, come back, or go anywhere, as far as I know the app is supposed to also keep track of people who have been in contact with covid patients, this could add great deal of fake information, which is not desirable and might cause panic among people.

arpanbag001 commented 4 years ago

Why would anyone spoof location on such an app?

By spoofing locations, a user might go near a verified covid patient also, come back, or go anywhere, as far as I know the app is supposed to also keep track of people who have been in contact with covid patients, this could add great deal of fake information, which is not desirable and might cause panic among people.

User can disable location instead, or temporarily uninstall the app while visiting verified covid patient.

From my perspective, this app solely depends on the goodwill of people. If they want to cheat the system, there's probably no way to prevent that.

criticic commented 4 years ago

@arpanbag001 I meant, a miscreant (sitting at home) using a location spoofer to go near covid patients knowingly or unknowingly, and going to different places for what so ever reason.

arpanbag001 commented 4 years ago

@arpanbag001 I meant, a miscreant (sitting at home) using a location spoofer to go near covid patients knowingly or unknowingly, and going to different places for what so ever reason.

If he is sitting at home, and not actually going near covid patients, what's the problem?

Also, as someone already mentioned, if someone is determined, he or she can decompile the apk, get the api endpoints, and hit them using his own auth tokens, but with fake gps coordinates!

My point being, preventing rooted users from using the app will do more harm (as many people won't be able to use the app, risking themselves and everyone near them), than good.

sureshvgs commented 4 years ago

on /nic/goi/aarogyasetu/location/RetrieveLocationService

LocationClient.setMockMode(false) instead of mFusedLocationClient.setMockMode(false);

sureshvgs commented 4 years ago

you may not need root access always. Just enabling developer option is more than enough. there we can select location mock option with any location faking apps. Also most apps fail or carelessly exclude checking mac address or further screening related to mocking or faking actions.

sureshvgs commented 4 years ago

@arpanbag001 I meant, a miscreant (sitting at home) using a location spoofer to go near covid patients knowingly or unknowingly, and going to different places for what so ever reason.

If he is sitting at home, and not actually going near covid patients, what's the problem?

Also, as someone already mentioned, if someone is determined, he or she can decompile the apk, get the api endpoints, and hit them using his own auth tokens, but with fake gps coordinates!

My point being, preventing rooted users from using the app will do more harm (as many people won't be able to use the app, risking themselves and everyone near them), than good.

Highlighted

dorky-handler commented 4 years ago

@arpanbag001 I meant, a miscreant (sitting at home) using a location spoofer to go near covid patients knowingly or unknowingly, and going to different places for what so ever reason.

If he is sitting at home, and not actually going near covid patients, what's the problem?

Also, as someone already mentioned, if someone is determined, he or she can decompile the apk, get the api endpoints, and hit them using his own auth tokens, but with fake gps coordinates!

My point being, preventing rooted users from using the app will do more harm (as many people won't be able to use the app, risking themselves and everyone near them), than good.

I don't know if you have noticed or not. States ruled by some parties are trying to polish their image by proclaiming lesser number of infections like in the case of an infection reported in Mahe. If you want a state to lose its "image" or just to create panic among a locality someone could misuse this feature to flag a large number of infections in a locality thus by making it a red zone ,they could profit in someway. I have many more reasons/example situations how this can be exploited but I think I've already proved my point.

dorky-handler commented 4 years ago

you may not need root access always. Just enabling developer option is more than enough. there we can select location mock option with any location faking apps. Also most apps fail or carelessly exclude checking mac address or further screening related to mocking or faking actions.

Thats why we are checking for mock location permission instead of root checks as commented by me above. We can also filter mock location out from API 18 or above

arpanbag001 commented 4 years ago

@arpanbag001 I meant, a miscreant (sitting at home) using a location spoofer to go near covid patients knowingly or unknowingly, and going to different places for what so ever reason.

If he is sitting at home, and not actually going near covid patients, what's the problem?

Also, as someone already mentioned, if someone is determined, he or she can decompile the apk, get the api endpoints, and hit them using his own auth tokens, but with fake gps coordinates!

My point being, preventing rooted users from using the app will do more harm (as many people won't be able to use the app, risking themselves and everyone near them), than good.

I don't know if you have noticed or not. States ruled by some parties are trying to polish their image by proclaiming lesser number of infections like in the case of an infection reported in Mahe. If you want a state to lose its "image" or just to create panic among a locality someone could misuse this feature to flag a large number of infections in a locality thus by making it a red zone ,they could profit in someway. I have many more reasons/example situations how this can be exploited but I think I've already proved my point.

I think that's why it's important to verify someone's report, before accepting the person as covid19 positive.

Even without using mock location, anyone can use extra phone numbers to sign up, then report (falsely) being positive and create panic in the locality, which won't be a good experience.

By implementing some kind of verification mechanism, we can solve the issue altogether!

And regarding mock location, UTS (Unreserved Ticketing System - Indian Railway) uses some kind of location tracking which couldn't be fooled using rooted or unrooted (developer options) mock location apps. The app runs perfectly on rooted devices, yet location detection was bulletproof. I think trying to implement something like that would be a better way to go.

dorky-handler commented 4 years ago

@arpanbag001 I meant, a miscreant (sitting at home) using a location spoofer to go near covid patients knowingly or unknowingly, and going to different places for what so ever reason.

If he is sitting at home, and not actually going near covid patients, what's the problem? Also, as someone already mentioned, if someone is determined, he or she can decompile the apk, get the api endpoints, and hit them using his own auth tokens, but with fake gps coordinates! My point being, preventing rooted users from using the app will do more harm (as many people won't be able to use the app, risking themselves and everyone near them), than good.

I don't know if you have noticed or not. States ruled by some parties are trying to polish their image by proclaiming lesser number of infections like in the case of an infection reported in Mahe. If you want a state to lose its "image" or just to create panic among a locality someone could misuse this feature to flag a large number of infections in a locality thus by making it a red zone ,they could profit in someway. I have many more reasons/example situations how this can be exploited but I think I've already proved my point.

I think that's why it's important to verify someone's report, before accepting the person as covid19 positive.

Even without using mock location, anyone can use extra phone numbers to sign up, then report (falsely) being positive and create panic in the locality, which won't be a good experience.

By implementing some kind of verification mechanism, we can solve the issue altogether!

And regarding mock location, UTS (Unreserved Ticketing System - Indian Railway) uses some kind of location tracking which couldn't be fooled using rooted or unrooted (developer options) mock location apps. The app runs perfectly on rooted devices, yet location detection was bulletproof. I think trying to implement something like that would be a better way to go.

That's exactly what the code I quoted above does.It has nothing to do with rooted device.FYI mock location works perfectly even without root via enabling developer options

SarangKulkarni commented 4 years ago

Location data is by definition client side data and in this case there is no option but to trust it for the functionality of the app. Whatever client-side checks are implemented in the app for stopping mock location etc., a malicious actor could bypass them in their own build of the app and continue to remain malicious.

It might be better to attempt to implement this intelligence on the server side.

ckcr4lyf commented 4 years ago

The best way to solve false report problem is needing a confirmed diagnosis to report yourself. When you do test positive, testing centers should be able to provide you with a nonce (or "one time password") that you need to use to report yourself.

This way trolls cannot report, and only authorized medical professionals can generate these nonces

dorky-handler commented 4 years ago

Location data is by definition client side data and in this case there is no option but to trust it for the functionality of the app. Whatever client-side checks are implemented in the app for stopping mock location etc., a malicious actor could bypass them in their own build of the app and continue to remain malicious.

It might be better to attempt to implement this intelligence on the server side.

No it does not has to be left at the mercy of client,if we add a certificate/signature validation every time the user opens the app,in that case the app does not work if it's inside is tinkered.

dorky-handler commented 4 years ago

I'm glad that someone had added the necessary code to filter out mock locations,should I close this now?

SarangKulkarni commented 4 years ago

Location data is by definition client side data and in this case there is no option but to trust it for the functionality of the app. Whatever client-side checks are implemented in the app for stopping mock location etc., a malicious actor could bypass them in their own build of the app and continue to remain malicious. It might be better to attempt to implement this intelligence on the server side.

No it does not has to be left at the mercy of client,if we add a certificate/signature validation every time the user opens the app,in that case the app does not work if it's inside is tinkered.

Where will this validation happen? Also could you please elaborate on what will be signed and by whom and how will the signature be validated?

dorky-handler commented 4 years ago

Location data is by definition client side data and in this case there is no option but to trust it for the functionality of the app. Whatever client-side checks are implemented in the app for stopping mock location etc., a malicious actor could bypass them in their own build of the app and continue to remain malicious. It might be better to attempt to implement this intelligence on the server side.

No it does not has to be left at the mercy of client,if we add a certificate/signature validation every time the user opens the app,in that case the app does not work if it's inside is tinkered.

Where will this validation happen? Also could you please elaborate on what will be signed and by whom and how will the signature be validated?

I'm talking about client side validation,The developer ought to sign application with their private key/certificate before the app can be installed on user devices.This app signature will be broken if the .apk is altered in any way.Thus by embedding the signature(developer certificate signature) inside a string in the app and by obfuscating it(using DexGuard) we can do a check everytime the app starts to check if the signature matches(from server side perhaps?) and then by verifying the installer which was used to install the app , app tampering/reverse engineering can effectively be prevented.Also by writing some code in C/C++ too could prevent this aforementioned tampering of the app as NDK can't be easily decompiled.
Besides how can the client location be validated from server side?using ip address?if so you know that's useless.

tachyons commented 4 years ago

how can the client location be validated from server side?

It can't. It can only detect patterns of client location, like number of location changes in given time period, distance between them, etc. But there are still high chances of false negatives

SarangKulkarni commented 4 years ago

The developer ought to sign application with their private key/certificate before the app can be installed on user devices.This app signature will be broken if the .apk is altered in any way.Thus by embedding the signature(developer certificate signature) inside a string in the app and by obfuscating it(using DexGuard) we can do a check everytime the app starts to check if the signature matches(from server side perhaps?) and then by verifying the installer which was used to install the app , app tampering/reverse engineering can effectively be prevented.Also by writing some code in C/C++ too could prevent this aforementioned tampering of the app as NDK can't be easily decompiled. Besides how can the client location be validated from server side?using ip address?if so you know that's useless.

The app has been open sourced. There is no need for anyone to edit APKs / reverse engineer anything when the whole source is available.

The aim is to have auditable source which can be verified as the exact codebase from which the distributed APKs are built. (See #36, #186). Anything that tries obfuscation etc. is not a good idea. We should aim for robustness by design, not by obfuscation. Also, we need to preserve the ability of users to build usable APKs from source.

krunalg commented 4 years ago

You can close this once following PR is merged https://github.com/nic-delhi/AarogyaSetu_Android/pull/199

dorky-handler commented 4 years ago

The developer ought to sign application with their private key/certificate before the app can be installed on user devices.This app signature will be broken if the .apk is altered in any way.Thus by embedding the signature(developer certificate signature) inside a string in the app and by obfuscating it(using DexGuard) we can do a check everytime the app starts to check if the signature matches(from server side perhaps?) and then by verifying the installer which was used to install the app , app tampering/reverse engineering can effectively be prevented.Also by writing some code in C/C++ too could prevent this aforementioned tampering of the app as NDK can't be easily decompiled. Besides how can the client location be validated from server side?using ip address?if so you know that's useless.

The app has been open sourced. There is no need for anyone to edit APKs / reverse engineer anything when the whole source is available.

The aim is to have auditable source which can be verified as the exact codebase from which the distributed APKs are built. (See #36, #186). Anything that tries obfuscation etc. is not a good idea. We should aim for robustness by design, not by obfuscation. Also, we need to preserve the ability of users to build usable APKs from source.

Hey man I just wanted to share a possible exploit I found and I'm glad that it's fixed now. So,I'm closing this thread now...