BluezoneGlobal / react-native-bluetooth-scan

Bluezone - Bảo vệ mình, bảo vệ cộng đồng
https://bluezone.ai
GNU General Public License v3.0
54 stars 41 forks source link

Unnecessary permissions requested #19

Open toankst opened 4 years ago

toankst commented 4 years ago

After looking through the code, I found that the app have those unnecessary permissions that are not required for the app to operate. It could potentially be a source of vulnerabilities if any other apps choose to interfere with it. image

First, WRITE_EXTERNAL_STORAGE and READ_EXTERNAL_STORAGE are currently used mostly for purpose of logging and loading configuration as I observe, you could easily remove those permissions and switch to app directory instead. I know it is a convenient to have your logs written in the external directory which could then be pulled for review. However, if any third party apps choose to interfere with it, it could easily pollute the configurations and grab the logs, which 'could' potentially a PII leakage.

Second, why ACCESS_FINE_LOCATION is needed in the first place, it is not needed for MODE_SCAN_FULL at all, the user's location is never requested. (on this build, I'll have to decompile app store version to verify)

HongThatCong commented 4 years ago

I agree with you, @KILLERST Unnecessary permissions should be removed !

NgoHuy commented 4 years ago

I submitted the issue here https://github.com/BluezoneGlobal/bluezone-app/issues/22

khanguy00 commented 4 years ago

I think that's leftover. Previously, Bluezone app saved IDs in external storage, and someone (@thaidn, maybe?) raised that issue, and the team silently removed it from the code, but the manifest hasn't updated yet.

toankst commented 4 years ago

I think that's leftover. Previously, Bluezone app saved IDs in external storage, and someone (@thaidn, maybe?) raised that issue, and the team silently removed it from the code, but the manifest hasn't updated yet.

According to my static analysis, there are still some logging and configuration methods requiring read and write permissions in version 2.0.4

Access to external drive (check & create directory and files if granted permissions): image

Calls to getAppPath: image image image image

NgoHuy commented 4 years ago

In latest code on github, the app created backup database as .bluezone.db.backup... on /sdcard/ partition. They should respect user's decision.

toankst commented 4 years ago

This is a full trace of bluezone_config.txt being accessed from ServiceTraceCovid:

image image image

khanguy00 commented 4 years ago

Okay, sounds like it's in use. Let's wait until the Bluezone team validate it.

thaidn commented 4 years ago

@KILLERST I'm not sure 100%, but it looks like the app indeed attempts to read bluezone_config.txt from external storage, but it also has a fallback mechanism when it doesn't have permission or the file doesn't exist. There are also functions that might write logs to external storage, but it looks like logging is disabled inn production built. I've run it on my test phone for days, but seen no log so far.

HongThatCong commented 4 years ago

Dùng JEB đi các đại ca, đọc code smali chi cho lòi mắt

implicit-invocation commented 4 years ago

IIRC, ACCESS_FINE_LOCATION permission is required for BLE discovery https://developer.android.com/about/versions/marshmallow/android-6.0-changes.html#behavior-hardware-id

toankst commented 4 years ago

IIRC, ACCESS_FINE_LOCATION permission is required for BLE discovery https://developer.android.com/about/versions/marshmallow/android-6.0-changes.html#behavior-hardware-id

Why would a Contact Tracing app need to call either WifiInfo.getMacAddress() or BluetoothAdapter.getAddress()?

With further digging, I found a receiver that should not exist in Contact Tracing ecosystem in the first place. https://github.com/BluezoneGlobal/react-native-bluetooth-scan/blob/master/lib/android/src/main/java/com/scan/ServiceTraceCovid.java#L96

Those 2 actions, ACTION_FOUND and ACTION_DISCOVERY_FINISHED used for usual bluetooth scan, not BLE scan. Also, there isn't any call to BluetoothAdapter.startDiscovery() in the app to start the normal scan, but there is a receiver waiting for those 2 actions and grab the data!?!

And then, those device's addresses are saved to local database, I believe the newer versions of android only broadcast temporary address, however, not all scanned devices are android. They could be a non-mobile device. If 2 devices found the same stationary smart home device, it could be safe to assume they were within a proximity. I once suggested my company that instead of tracking people's tokens, we could build a mapping of stationary objects instead. image

I am interested to hear from the team why this receiver exists in the first place.

implicit-invocation commented 4 years ago

@KILLERST I posted the incorrect link, the correct one should be https://developer.android.com/reference/android/bluetooth/le/BluetoothLeScanner#startScan(android.bluetooth.le.ScanCallback) BLE scan needs location permission since BLE beacons can be used to determine indoor location. I cannot speak for Bluezone team but the classic bluetooth scan seems to be a fallback for devices without BLE support.

thaidn commented 4 years ago

@KILLERST The Bluetooth classic scanning issue was discussed at length in the first half of https://github.com/BluezoneGlobal/documents/issues/3. I found that the scanning code is disabled: https://github.com/BluezoneGlobal/documents/issues/3#issuecomment-667746729.

toankst commented 4 years ago

@thaidn Thanks for catching that, it is indeed disabled and the whole process of normal scan would not start.

image