markusfisch / BinaryEye

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

Network connection may not be being verified #214

Open amazuerar opened 3 years ago

amazuerar commented 3 years ago

Dear @markusfisch!

This is the last issue 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 connection within the project since some network operations are being performed in the project. As you might know, a device may not be connected to a network. In order to get such information see https://developer.android.com/reference/android/net/ConnectivityManager. Therefore it is recommended for the app to identify whether the device has a network connection 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/reference/android/net/ConnectivityManager.NetworkCallback

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

Note: public NetworkInfo getActiveNetworkInfo () returns details about the currently active default data network. When connected, this network is the default route for outgoing connections. You should always check NetworkInfo#isConnected() before initiating network traffic. It requires Manifest.permission.ACCESS_NETWORK_STATE. However, isConnected() was deprecated in API level 29, one should instead use the ConnectivityManager.NetworkCallback API to learn about connectivity changes, to be more specific the onAvailable() method.

markusfisch commented 3 years ago

As I wrote here, I generally don't see what problem this would solve 😉

You simply make the request and either it's successful or it fails. For almost all requests, it's that simple.

If this app would make many requests at once, then I would probably check the network connection before making all these requests. But for just one request per scan, this would be overkill.

What I'm generally interested in is avoiding code bloat 😉

soshial commented 4 months ago

I wonder why this app needs network permissions in the first place? @markusfisch

markusfisch commented 4 months ago

@soshial The network permission is required for the optional forwarding feature, which can be configured in the settings.