search

markusfisch / BinaryEye

Yet another barcode scanner for Android
https://play.google.com/store/apps/details?id=de.markusfisch.android.binaryeye
MIT License
1.42k stars 117 forks source link

Network Type may not be being verified #213

Open amazuerar opened 3 years ago

amazuerar commented 3 years ago

Dear @markusfisch!

Once again, thanks for being interested in the issues we wanted to report as part of our research. In this case, we present the code locations that are related to a missing validation of network type within the project since some network operations are being performed in the project. As you might know, a device can have various types of network connections. For the full list of possible network types, see https://developer.android.com/reference/android/net/ConnectivityManager. Therefore it is recommended for the app to identify the type of network available before performing a network operation.

In order to address this issue we recommend you to visit:

  1. https://developer.android.com/training/basics/network-ops/managing
  2. https://developer.android.com/training/efficient-downloads/connectivity_patterns

Potential Code Location without Network type validation

https://github.com/markusfisch/BinaryEye/blob/af70d9507ba19739064949dd089de9b7112bf255/app/src/main/kotlin/de/markusfisch/android/binaryeye/net/ScanSender.kt#L83-L89

Glitchy-Tozier commented 3 years ago

@amazuerar Could it be that you posted the issue twice?

amazuerar commented 3 years ago

Hello @Glitchy-Tozier! #211, #213, #214 may look the same, however, they are referring to different scenarios. A device can be connected to a network, which happens to be a Wi-Fi network but unfortunately not having Internet connectivity. Therefore, before performing a network request which needs Internet, one should check (i) network connection, (ii) type of network, becase it can be metered, and (iii) Internet availability in such network.

Glitchy-Tozier commented 3 years ago

Ah, sorry! I just flew over it and from the title and the structure they seemed very similar 😅

markusfisch commented 3 years ago

Therefore, before performing a network request which needs Internet, one should check (i) network connection, (ii) type of network, becase it can be metered, and (iii) Internet availability in such network.

Sorry, I disagree with (i) and (iii). And I don't see (ii) apply to this app.

About (i): If a device isn't connected to a network, the connection will simply fail. This nothing to be afraid of. In fact, that's perfectly okay for a mobile device that moves between networks. The app won't crash. Nothing will happen. So I don't see any value in checking the network condition before making a request. It would just add more code for the exact same result. And more code, is always bad.

About (ii): While it would be possible to restrict the requests to specific networks, I don't think anyone would actually use such a setting in this app. A user can always just simply add or remove the address from the settings and this app won't make any requests anymore.

About (iii): As I already wrote in response to #211, you simply cannot tell if you can connect to a server without actually connecting to it. "Internet availability" simply means the device can reach "google.com". This doesn't say anything about the address I'm interested in. So, again, I don't see any value in checking this, but I can see how such a check can possibly prevent me from connecting to a private address in a private network. If I'm connected to my Wifi where I have a Raspberry Pi, I'm not interested in "Internet availablilty".