From 34cd08fd9ac7d8b5a7db91b62c54224f36500174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donncha=20=C3=93=20Cearbhaill?= Date: Thu, 30 Jan 2025 11:35:18 +0100 Subject: [PATCH 1/3] Add additional Android security setting to warn on --- src/mvt/android/artifacts/settings.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mvt/android/artifacts/settings.py b/src/mvt/android/artifacts/settings.py index 1ce75e8..06a261e 100644 --- a/src/mvt/android/artifacts/settings.py +++ b/src/mvt/android/artifacts/settings.py @@ -16,6 +16,11 @@ ANDROID_DANGEROUS_SETTINGS = [ "key": "package_verifier_enable", "safe_value": "1", }, + { + "description": "disabled APK package verification", + "key": "package_verifier_state", + "safe_value": "1", + }, { "description": "disabled Google Play Protect", "key": "package_verifier_user_consent", From 0962383b460aaf41480a535f2864fd14d51ad727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donncha=20=C3=93=20Cearbhaill?= Date: Thu, 30 Jan 2025 11:48:19 +0100 Subject: [PATCH 2/3] Alert on potentially suspicious permissions from ADB --- src/mvt/android/artifacts/dumpsys_appops.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mvt/android/artifacts/dumpsys_appops.py b/src/mvt/android/artifacts/dumpsys_appops.py index 12c8114..98561b4 100644 --- a/src/mvt/android/artifacts/dumpsys_appops.py +++ b/src/mvt/android/artifacts/dumpsys_appops.py @@ -55,6 +55,11 @@ class DumpsysAppopsArtifact(AndroidArtifact): result["package_name"], ) + if result["package_name"] == "com.android.shell": + self.log.warning( + "Risky package com.android.shell requested a permission" + ) + def parse(self, output: str) -> None: self.results: List[Dict[str, Any]] = [] perm = {} From 43901c96a02fea5d6f9a5c7af0e632a483468165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donncha=20=C3=93=20Cearbhaill?= Date: Thu, 30 Jan 2025 13:02:26 +0100 Subject: [PATCH 3/3] Add improved heuristic detections to AppOps module --- src/mvt/android/artifacts/dumpsys_appops.py | 50 +++++++++++++++---- tests/android/test_artifact_dumpsys_appops.py | 18 ++++++- tests/android_androidqf/test_dumpsysappops.py | 7 ++- tests/android_bugreport/test_bugreport.py | 7 ++- 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/mvt/android/artifacts/dumpsys_appops.py b/src/mvt/android/artifacts/dumpsys_appops.py index 98561b4..8323c8a 100644 --- a/src/mvt/android/artifacts/dumpsys_appops.py +++ b/src/mvt/android/artifacts/dumpsys_appops.py @@ -11,6 +11,10 @@ from mvt.common.utils import convert_datetime_to_iso from .artifact import AndroidArtifact +RISKY_PERMISSIONS = ["REQUEST_INSTALL_PACKAGES"] +RISKY_PACKAGES = ["com.android.shell"] + + class DumpsysAppopsArtifact(AndroidArtifact): """ Parser for dumpsys app ops info @@ -45,20 +49,39 @@ class DumpsysAppopsArtifact(AndroidArtifact): self.detected.append(result) continue + detected_permissions = [] for perm in result["permissions"]: if ( - perm["name"] == "REQUEST_INSTALL_PACKAGES" - and perm["access"] == "allow" + perm["name"] in RISKY_PERMISSIONS + # and perm["access"] == "allow" ): - self.log.info( - "Package %s with REQUEST_INSTALL_PACKAGES permission", - result["package_name"], - ) + detected_permissions.append(perm) + for entry in sorted(perm["entries"], key=lambda x: x["timestamp"]): + self.log.warning( + "Package '%s' had risky permission '%s' set to '%s' at %s", + result["package_name"], + perm["name"], + entry["access"], + entry["timestamp"], + ) - if result["package_name"] == "com.android.shell": - self.log.warning( - "Risky package com.android.shell requested a permission" - ) + elif result["package_name"] in RISKY_PACKAGES: + detected_permissions.append(perm) + for entry in sorted(perm["entries"], key=lambda x: x["timestamp"]): + self.log.warning( + "Risky package '%s' had '%s' permission set to '%s' at %s", + result["package_name"], + perm["name"], + entry["access"], + entry["timestamp"], + ) + + if detected_permissions: + # We clean the result to only include the risky permission, otherwise the timeline + # will be polluted with all the other irrelevant permissions + cleaned_result = result.copy() + cleaned_result["permissions"] = detected_permissions + self.detected.append(cleaned_result) def parse(self, output: str) -> None: self.results: List[Dict[str, Any]] = [] @@ -126,11 +149,16 @@ class DumpsysAppopsArtifact(AndroidArtifact): if line.startswith(" "): # Permission entry like: # Reject: [fg-s]2021-05-19 22:02:52.054 (-314d1h25m2s33ms) + access_type = line.split(":")[0].strip() + if access_type not in ["Access", "Reject"]: + # Skipping invalid access type. Some entries are not in the format we expect + continue + if entry: perm["entries"].append(entry) entry = {} - entry["access"] = line.split(":")[0].strip() + entry["access"] = access_type entry["type"] = line[line.find("[") + 1 : line.find("]")] try: diff --git a/tests/android/test_artifact_dumpsys_appops.py b/tests/android/test_artifact_dumpsys_appops.py index e713e6b..7c2edc2 100644 --- a/tests/android/test_artifact_dumpsys_appops.py +++ b/tests/android/test_artifact_dumpsys_appops.py @@ -43,5 +43,21 @@ class TestDumpsysAppopsArtifact: ind.ioc_collections[0]["app_ids"].append("com.facebook.katana") da.indicators = ind assert len(da.detected) == 0 + da.check_indicators() - assert len(da.detected) == 1 + detected_by_ioc = [ + detected for detected in da.detected if detected.get("matched_indicator") + ] + detected_by_permission_heuristic = [ + detected + for detected in da.detected + if all( + [ + perm["name"] == "REQUEST_INSTALL_PACKAGES" + for perm in detected["permissions"] + ] + ) + ] + assert len(da.detected) == 3 + assert len(detected_by_ioc) == 1 + assert len(detected_by_permission_heuristic) == 2 diff --git a/tests/android_androidqf/test_dumpsysappops.py b/tests/android_androidqf/test_dumpsysappops.py index c28c240..b0649a1 100644 --- a/tests/android_androidqf/test_dumpsysappops.py +++ b/tests/android_androidqf/test_dumpsysappops.py @@ -21,4 +21,9 @@ class TestDumpsysAppOpsModule: run_module(m) assert len(m.results) == 12 assert len(m.timeline) == 16 - assert len(m.detected) == 0 + + detected_by_ioc = [ + detected for detected in m.detected if detected.get("matched_indicator") + ] + assert len(m.detected) == 1 + assert len(detected_by_ioc) == 0 diff --git a/tests/android_bugreport/test_bugreport.py b/tests/android_bugreport/test_bugreport.py index 52b4caf..cc9afec 100644 --- a/tests/android_bugreport/test_bugreport.py +++ b/tests/android_bugreport/test_bugreport.py @@ -33,7 +33,12 @@ class TestBugreportAnalysis: m = self.launch_bug_report_module(Appops) assert len(m.results) == 12 assert len(m.timeline) == 16 - assert len(m.detected) == 0 + + detected_by_ioc = [ + detected for detected in m.detected if detected.get("matched_indicator") + ] + assert len(m.detected) == 1 # Hueristic detection for suspicious permissions + assert len(detected_by_ioc) == 0 def test_packages_module(self): m = self.launch_bug_report_module(Packages)