Merge pull request #603 from mvt-project/feature/add-suspicious-andro…

…id-setting

Add additional Android security warnings

src/mvt/android/artifacts/dumpsys_appops.py

@@ -11,6 +11,10 @@
 from .artifact import AndroidArtifact
 
 
+RISKY_PERMISSIONS = ["REQUEST_INSTALL_PACKAGES"]
+RISKY_PACKAGES = ["com.android.shell"]
+
+
 class DumpsysAppopsArtifact(AndroidArtifact):
     """
     Parser for dumpsys app ops info
@@ -45,15 +49,39 @@ def check_indicators(self) -> None:
                     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"],
+                        )
+
+                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]] = []
@@ -121,11 +149,16 @@ def parse(self, output: str) -> None:
             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:

src/mvt/android/artifacts/settings.py

@@ -16,6 +16,11 @@
         "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",

tests/android/test_artifact_dumpsys_appops.py

@@ -43,5 +43,21 @@ def test_ioc_check(self, indicator_file):
         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

tests/android_androidqf/test_dumpsysappops.py

@@ -21,4 +21,9 @@ def test_parsing(self):
         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

tests/android_bugreport/test_bugreport.py

@@ -33,7 +33,12 @@ def test_appops_module(self):
         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)