additional APK checks
A discussion on different topics raised ideas for security improvements, from which it is worth to adopt some additional checks:
-
we already check for android:debuggable
. We should also consider checking forandroid:testOnly
(must not betrue
), andandroid:usesCleartextTraffic
(true
should at least raise a warning)-
implement for manual check on inclusion -
implement for auto-check on update
-
-
we already check for debug certificates on inclusion, and do not permit those for new apps (already existing cases are rather rare and should be investigated separately => #477). This should be mentioned in the inclusion criteria. -
implement for manual check on inclusion -
implement for auto-check on update: not necessary, as a certificate change would result in the APK being rejected anyway. -
update documentation (inclusion criteria)
-
-
Sensitive permissions should trigger a manual review when encountered (they should only be permitted when reasonable in the app's context). This will be more effort to implement (we'd need another positive-list to check against cleared ones), but would be a good security measure. List below.
A special use-case of this is detecting potential self-updaters (already mentioned with our inclusion criteria), as they'd needREQUEST_INSTALL_PACKAGES
to work (which should otherwise only be needed rarely), so looking for this would serve as an "early warning system".-
implement for manual check on inclusion -
implement for auto-check on update (currently in progress) -
implement a counter-check (if a permission was explicitly allowed but is no longer requested, it should be removed from the allow-list of the app)
-
-
There are also some sensitive service intent filters (list below), which is another useful security measure to trigger "early warnings" (or maybe should even block an APK from being included), list below. Again, this needs some more efforts and per-app positives lists. -
implement for manual check on inclusion -
implement for auto-check on update -
implement a counter-check (if a service-intent was explicitly allowed but is no longer requested, it should be removed from the allow-list of the app)
-
-
final task: update our documentation to add the new security measures to the existing ones when in place
Flags
android:usesCleartextTraffic
The application has
android:usesCleartextTraffic
set totrue
, which allows it to access resources that do not use encryption, a situation that could be exploited by an attacker to perform MitM attacks and compromise the confidentiality and integrity of the application.
And from the linked page at developer.android.com:
The key reason for avoiding cleartext traffic is the lack of confidentiality, authenticity, and protections against tampering. A network attacker can eavesdrop on transmitted data and also modify it without being detected.
So this should be avoided wherever possible – but can be OK if it's intended for local resources (hard to have a proper certificate for a server in your home network), e.g. media or WebDAV servers.
android:testOnly
Indicates whether this application is only for testing purposes. For example, it might expose functionality or data outside of itself that can cause a security hole, but be useful for testing. This kind of APK only installs through
adb
. You can't publish it to Google Play.
Never encountered this here, but according to the description it should not be possible to install such an app via an F-Droid client, so it would not make any sense adding it to the repo.
android:debuggable
Not to be confused with a debug key (an app can be declared android:debuggable
and signed with a release key, and vice versa).
This flag is a bit controversial. On one hand, such an app might open doors to things it should rather not (except in the developers' environments) – on the other hand, with newer Android versions this is the only way to convince adb backup
to include app data as well. So it's unclear if it should be handled as a risk or as a feature…
Sensitive permissions as per suggestion
All of those are already considered on app inclusion. Ticked check-marks indicate permissions already included with scans on updates as well:
-
ACCESS_BACKGROUND_LOCATION
-
ACCESS_COARSE_LOCATION
-
ACCESS_FINE_LOCATION
-
BLUETOOTH_SCAN
-
CAMERA
-
MANAGE_EXTERNAL_STORAGE
-
NEARBY_WIFI_DEVICES
-
PROCESS_OUTGOING_CALLS
-
QUERY_ALL_PACKAGES
-
READ_CALL_LOG
-
READ_CONTACTS
-
READ_EXTERNAL_STORAGE
(note this can be granted implicitly when an app just requestsWRITE_EXTERNAL_STORAGE
– in which case it won't be part of theAndroidManifest.xml
, but Android would handle it as if it would.aapt dump badging
reports it then asuses-implied-permission
, but our parser could not see it. Update: fixed with ce52f37e handling implied permissions, still needs to be implemented withApkUpdater
) -
READ_MEDIA_AUDIO
-
READ_MEDIA_IMAGES
-
READ_MEDIA_VIDEO
-
READ_PHONE_NUMBERS
-
READ_PHONE_STATE
-
READ_SMS
-
RECEIVE_MMS
-
RECEIVE_SMS
-
RECEIVE_WAP_PUSH
-
RECORD_AUDIO
-
REQUEST_INSTALL_PACKAGES
-
SEND_SMS
-
WRITE_CALL_LOG
-
WRITE_CONTACTS
-
SYSTEM_ALERT_WINDOW
Additions:
-
REQUEST_DELETE_PACKAGES
-
dealt with via implied permissionsWRITE_EXTERNAL_STORAGE
(as this impliesREAD_EXTERNAL_STORAGE
without the latter showing up in the permission list, see above)
According to AOSP source on aapt
, the only other implied-permission
is READ_CALL_LOG
when READ_CONTACTS
is requested and targetSdk < 16
– plus the same for the corresponding WRITE_*
. So those two need to be implemented separately in ManifestCheck
, based on the app's targetSdk
– and maybe also adding the implied READ_EXTERNAL_STORAGE
at that place so we don't have to list/check those WRITE_
permissions separately. Should have another impliedPermissions[]
for them as well then to make it transparent. => implied permissions are dealt with in ManifestCheck
via ce52f37e
Service intent filters
android.accessibilityservice.AccessibilityService
android.net.VpnService
android.view.InputMethod
These service intents must be reasonable in the app's context.