From 6f65eee85a7a248fb08eb25b3fba580332665271 Mon Sep 17 00:00:00 2001 From: va-resident Date: Tue, 22 Sep 2026 18:11:42 +0300 Subject: [PATCH 1/4] Do not treat a section's timing line as a section boundary extract_command_section() ends a section at any line starting with "------". dumpstate prints a section's timing line when THAT section finishes, and it can land in the middle of the section currently being written, so the section is truncated at an arbitrary point and the rest is silently dropped. Skip the timing line instead of returning it: it never reaches a parser as content, and the real "------ ------" boundary still ends the section. Measured over 40 bug reports carrying a dumpstate: SYSTEM PROPERTIES was truncated on 6 of them, losing 6559 properties, and 3 parsed no property at all. On one Samsung archive getprop goes from 418 to 1259 properties and from 0 to 473 ro.* ones, bringing back ro.product.model, ro.build.version.security_patch and ro.boot.verifiedbootstate. The other 34 archives are byte-identical. Fixes #938 --- src/mvt/android/modules/bugreport/base.py | 11 +++++ .../test_bugreport_section_extraction.py | 42 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 tests/android/test_bugreport_section_extraction.py diff --git a/src/mvt/android/modules/bugreport/base.py b/src/mvt/android/modules/bugreport/base.py index 25396f95..9e8954d6 100644 --- a/src/mvt/android/modules/bugreport/base.py +++ b/src/mvt/android/modules/bugreport/base.py @@ -6,6 +6,7 @@ import datetime import fnmatch import logging import os +import re from pathlib import Path from typing import List, Optional from zipfile import ZipFile @@ -13,6 +14,10 @@ from zoneinfo import ZoneInfo, ZoneInfoNotFoundError from mvt.common.module import ModuleResults, MVTModule +# `------ 0.101s was the duration of 'SOME SECTION' ------`, printed when that +# section finishes and not necessarily between two sections. +SECTION_DURATION = re.compile(r"^-{3,}\s*[0-9.]+s was the duration of", re.IGNORECASE) + class BugReportModule(MVTModule): """This class provides a base for all Android Bug Report modules.""" @@ -122,6 +127,12 @@ class BugReportModule(MVTModule): in_section = True continue if stripped.startswith("------"): + # dumpstate prints a section's timing line when that section + # finishes, which can land in the middle of the one being + # written. Treating it as a boundary truncates the section at + # an arbitrary point, silently. + if SECTION_DURATION.match(stripped): + continue break lines.append(line) return "\n".join(lines) diff --git a/tests/android/test_bugreport_section_extraction.py b/tests/android/test_bugreport_section_extraction.py new file mode 100644 index 00000000..5778f947 --- /dev/null +++ b/tests/android/test_bugreport_section_extraction.py @@ -0,0 +1,42 @@ +# Mobile Verification Toolkit (MVT) +# Copyright (c) 2021-2026 The MVT Authors. +# Use of this software is governed by the MVT License 1.1 that can be found at +# https://license.mvt.re/1.1/ +"""A section ends at the next section, not at a timing line printed inside it.""" + +from mvt.android.modules.bugreport.base import BugReportModule + +# dumpstate prints a section's duration when that section finishes, which can +# land in the middle of the section currently being written. +DUMPSTATE = """\ +------ SYSTEM PROPERTIES (getprop) ------ +[nfc.initialized]: [true] +------ 0.101s was the duration of 'DROPBOX SYSTEM SERVER CRASHES' ------ +[ro.build.version.sdk]: [30] +[ro.product.model]: [SM-A305F] +------ 0.064s was the duration of 'SYSTEM PROPERTIES' ------ +------ STORAGE INFO (df) ------ +/dev/root 2.9G +""" + + +class TestExtractCommandSection: + def test_a_foreign_timing_line_does_not_end_the_section(self): + section = BugReportModule.extract_command_section( + DUMPSTATE, "------ SYSTEM PROPERTIES" + ) + assert "[ro.product.model]: [SM-A305F]" in section + assert section.count("\n") == 2 + + def test_the_next_section_is_still_the_boundary(self): + section = BugReportModule.extract_command_section( + DUMPSTATE, "------ SYSTEM PROPERTIES" + ) + assert "STORAGE INFO" not in section + assert "/dev/root" not in section + + def test_timing_lines_are_not_returned_as_content(self): + section = BugReportModule.extract_command_section( + DUMPSTATE, "------ SYSTEM PROPERTIES" + ) + assert "was the duration of" not in section From bdbc07a3b32b7f97b24e1c25e97a21c59d09ad98 Mon Sep 17 00:00:00 2001 From: va-resident Date: Tue, 22 Sep 2026 18:13:57 +0300 Subject: [PATCH 2/4] Carry the accessibility service count the dump states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most builds never print the `installed services: {...}` block. They state installedServiceCount=N in the user's attributes line and list nothing, so the parser recorded no service at all and MVT logged "Identified a total of 0 accessibility services" — an artifact that says "no accessibility services" about a dump that said there are five. The dump pasted in #744 is itself an example: it states installedServiceCount=6 and names none. Parse the count per user and, for a user whose services the dump did not list, keep one record carrying it, raised as LOW: a count is a coverage statement, not a running service. Name the service state in the alert and split its severity, as asked on #744: a service the dump states is switched off is LOW, enabled or bound is MEDIUM, and a dump that does not state the enabled state stays MEDIUM, because "not stated" is not "not enabled". A state section the dump never printed now reads None rather than False. IOC matching is unaffected by the state: a disabled service still returns CRITICAL on a match. Measured over 163 bug reports carrying an accessibility section: MEDIUM alerts 305 -> 4, LOW 0 -> 407. The four that stay MEDIUM are the only records in the set with enabled=True. Fixes #744 --- .../artifacts/dumpsys_accessibility.py | 119 ++++++++++++++--- .../bugreport/dumpsys_accessibility.py | 17 ++- .../test_artifact_dumpsys_accessibility.py | 11 +- ...st_artifact_dumpsys_accessibility_count.py | 100 ++++++++++++++ ...st_artifact_dumpsys_accessibility_state.py | 125 ++++++++++++++++++ 5 files changed, 351 insertions(+), 21 deletions(-) create mode 100644 tests/android/test_artifact_dumpsys_accessibility_count.py create mode 100644 tests/android/test_artifact_dumpsys_accessibility_state.py diff --git a/src/mvt/android/artifacts/dumpsys_accessibility.py b/src/mvt/android/artifacts/dumpsys_accessibility.py index da46043c..1a4a05ac 100644 --- a/src/mvt/android/artifacts/dumpsys_accessibility.py +++ b/src/mvt/android/artifacts/dumpsys_accessibility.py @@ -10,8 +10,37 @@ from .artifact import AndroidArtifact class DumpsysAccessibilityArtifact(AndroidArtifact): + # One list for both record shapes — a service record and a count-only + # record must stay the same shape. + _FIELDS = ( + "user_id", + "component", + "package_name", + "service_name", + "installed", + "enabled", + "binding", + "bound", + "crashed", + "accessibility_tool", + "installed_service_count", + ) + def check_indicators(self) -> None: for result in self.results: + # A stated count with no component names is a coverage statement, + # not a service: low, but not silent. + if not result.get("component"): + self.alertstore.low( + f"The accessibility dump states " + f"{result['installed_service_count']} installed " + f"service(s) for user {result['user_id']} but does not " + f"list their component names", + "", + result, + ) + continue + if self.indicators: ioc_match = self.indicators.check_app_id(result["package_name"]) if ioc_match: @@ -20,11 +49,22 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): ) continue - self.alertstore.medium( - f'Found accessibility service: "{result["component"]}"', - "", - result, + # Installed is not enabled. A service can sit installed for years + # without ever being switched on, and that is the difference this + # channel is read for — an alert that says only "found" makes every + # device look equally exposed. A service the dump says is switched + # OFF is reported LOW, so it still reaches the analyst without + # competing with one that is actually running; a dump that does not + # state the enabled state stays MEDIUM, because "not stated" is not + # "not enabled". + message = ( + f'Found accessibility service: "{result["component"]}" ' + f"({self._describe_state(result)})" ) + if result.get("enabled") is False and not result.get("bound"): + self.alertstore.low(message, "", result) + else: + self.alertstore.medium(message, "", result) def parse(self, content: str) -> None: """ @@ -36,6 +76,11 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): self.results: list[dict[str, Any]] = [] services: dict[tuple[int | None, str], dict] = {} + seen_states: set[str] = set() + # `installedServiceCount=N` from the user's `attributes:{…}` line is on + # most builds the only statement about installed services in the dump: + # few print the `installed services: {…}` block. + installed_counts: dict[int | None, int] = {} user_id: int | None = None state: str | None = None @@ -44,6 +89,10 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): if user_match: user_id = int(user_match.group(1)) + count_match = re.search(r"installedServiceCount=(\d+)", line) + if count_match: + installed_counts[user_id] = int(count_match.group(1)) + stripped = line.strip() state_match = re.match( r"(?i)(installed|enabled|binding|bound|crashed) services\s*:\s*\{(.*)", @@ -51,6 +100,7 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): ) if state_match: state = state_match.group(1).lower() + seen_states.add(self._state_field(state)) inline = state_match.group(2) for component in re.findall( r"\{?([\w.$-]+/[\w.$-]+)(?:\s+\(A11yTool\))?\}?", inline @@ -79,24 +129,59 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): service[self._state_field(state)] = True service["accessibility_tool"] = "(A11yTool)" in stripped + # A section that was never printed is NOT the same as one printed + # empty: the first says nothing, the second says nothing is enabled. + # Defaulting every flag to False would turn "not stated" into "not + # enabled". Flags for sections this dump never printed stay None. + for (service_user, _component), service in services.items(): + for state in ("installed", "enabled", "binding", "bound", "crashed"): + if self._state_field(state) not in seen_states: + service[self._state_field(state)] = None + service["installed_service_count"] = installed_counts.get(service_user) + self.results.extend(services.values()) + # A stated count whose services were never listed would leave no trace: + # the module would log "a total of 0" about a dump that said five. + listed_users = {service_user for service_user, _component in services} + for count_user, count in installed_counts.items(): + if count_user in listed_users or count == 0: + continue + self.results.append(self._new_unlisted(count_user, count)) + + @staticmethod + def _describe_state(result: dict) -> str: + if result.get("bound"): + return "enabled and bound" + if result.get("enabled"): + return "enabled" + if result.get("enabled") is None: + return "installed, enabled state not stated" + return "installed, not enabled" + @staticmethod def _state_field(state: str) -> str: return {"binding": "binding", "bound": "bound"}.get(state, state) + @staticmethod + def _new_unlisted(user_id: int | None, count: int) -> dict: + """The dump's own count for a user whose services it did not list. + + Every other field stays unknown: the dump named no service to carry it. + """ + record: dict[str, Any] = dict.fromkeys(DumpsysAccessibilityArtifact._FIELDS) + record["user_id"] = user_id + record["installed_service_count"] = count + return record + @staticmethod def _new_service(component: str, user_id: int | None) -> dict: - package_name, service_name = component.split("/", 1) - return { - "user_id": user_id, - "component": component, - "package_name": package_name, - "service_name": service_name, - "installed": False, - "enabled": False, - "binding": False, - "bound": False, - "crashed": False, - "accessibility_tool": False, - } + record: dict[str, Any] = dict.fromkeys( + DumpsysAccessibilityArtifact._FIELDS, False + ) + record["user_id"] = user_id + record["component"] = component + record["package_name"], record["service_name"] = component.split("/", 1) + # Filled in after parsing: the count is per user. + record["installed_service_count"] = None + return record diff --git a/src/mvt/android/modules/bugreport/dumpsys_accessibility.py b/src/mvt/android/modules/bugreport/dumpsys_accessibility.py index 02d85a30..80991d37 100644 --- a/src/mvt/android/modules/bugreport/dumpsys_accessibility.py +++ b/src/mvt/android/modules/bugreport/dumpsys_accessibility.py @@ -48,9 +48,22 @@ class DumpsysAccessibility(DumpsysAccessibilityArtifact, BugReportModule): ) self.parse(content) + listed = stated = 0 for result in self.results: - self.log.info('Found accessibility service "%s"', result.get("component")) + if result.get("component"): + listed += 1 + self.log.info( + 'Found accessibility service "%s"', result.get("component") + ) + continue + # The operator gets this per user as a LOW alert from + # check_indicators(); here it only has to survive into the summary, + # so that a stated count never reads as "a total of 0". + stated += result.get("installed_service_count") or 0 self.log.info( - "Identified a total of %d accessibility services", len(self.results) + "Identified a total of %d accessibility services (%d more stated by " + "the dump without a component name)", + listed, + stated, ) diff --git a/tests/android/test_artifact_dumpsys_accessibility.py b/tests/android/test_artifact_dumpsys_accessibility.py index c727c571..f21c79ed 100644 --- a/tests/android/test_artifact_dumpsys_accessibility.py +++ b/tests/android/test_artifact_dumpsys_accessibility.py @@ -39,7 +39,10 @@ class TestDumpsysAccessibilityArtifact: assert da.results[0]["package_name"] == "com.malware.accessibility" assert da.results[0]["service_name"] == "com.malware.service.malwareservice" assert da.results[0]["enabled"] is True - assert da.results[0]["installed"] is False + # This fixture never prints an `installed services:` section, so the + # dump does not state the installed status. Reporting False would turn + # "not stated" into "not installed", so it reads None here. + assert da.results[0]["installed"] is None def test_accessibility_service_alert(self): da = DumpsysAccessibilityArtifact() @@ -84,7 +87,11 @@ User state[attributes:{id=10 assert len(da.alertstore.alerts) == 0 da.check_indicators() assert len(da.alertstore.alerts) == len(da.results) - assert da.alertstore.count(AlertLevel.MEDIUM) == 3 + # Every service in this fixture is installed and switched off + # (`enabled services:{}` is printed and empty), so the three non-IOC + # findings are LOW, not MEDIUM. The IOC match is unaffected by the + # state. + assert da.alertstore.count(AlertLevel.LOW) == 3 assert da.alertstore.count(AlertLevel.CRITICAL) == 1 critical_alert = next( alert diff --git a/tests/android/test_artifact_dumpsys_accessibility_count.py b/tests/android/test_artifact_dumpsys_accessibility_count.py new file mode 100644 index 00000000..59765073 --- /dev/null +++ b/tests/android/test_artifact_dumpsys_accessibility_count.py @@ -0,0 +1,100 @@ +# Mobile Verification Toolkit (MVT) +# Copyright (c) 2021-2026 The MVT Authors. +# Use of this software is governed by the MVT License 1.1 that can be found at +# https://license.mvt.re/1.1/ +"""The dump's own installed-service count must survive into the artifact. + +Most builds never print the `installed services: {…}` block; they state `installedServiceCount=N` in the user's `attributes:{…}` line and +list nothing. Dropping that number makes an artifact that says "no +accessibility services" about a dump that said there are five. +""" + +from mvt.android.artifacts.dumpsys_accessibility import DumpsysAccessibilityArtifact +from mvt.common.alerts import AlertLevel + +AOSP_NO_LIST = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[ + attributes:{id=0, touchExplorationEnabled=false, installedServiceCount=5} + Bound services:{} + Enabled services:{} + Binding services:{} + Crashed services:{} +""" + +ONE_UI_WITH_LIST = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, installedServiceCount=2} + installed services: { + 0 : com.example.app/com.example.app.Service + 1 : com.other.app/.Helper + } + enabled services: { + } +""" + +TWO_USERS = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, installedServiceCount=1} + installed services: { + 0 : com.example.app/com.example.app.Service + } +User state[attributes:{id=95, installedServiceCount=3} + Enabled services:{} +""" + +ZERO_COUNT = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, installedServiceCount=0} + Enabled services:{} +""" + + +def _parse(content): + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + artifact.parse(content) + return artifact + + +class TestAccessibilityInstalledServiceCount: + def test_stated_count_without_a_list_is_kept(self): + artifact = _parse(AOSP_NO_LIST) + assert len(artifact.results) == 1 + record = artifact.results[0] + assert record["installed_service_count"] == 5 + assert record["component"] is None + # Every state flag stays unknown: the dump named no service to which a + # state could belong. + assert record["installed"] is None + assert record["enabled"] is None + + def test_a_stated_count_is_reported_as_low(self): + artifact = _parse(AOSP_NO_LIST) + artifact.check_indicators() + alerts = artifact.alertstore.alerts + assert len(alerts) == 1 + # A count is a coverage statement, not a running service: it must not + # compete with a service the dump says is enabled. + assert alerts[0].level == AlertLevel.LOW + assert "does not list their component names" in alerts[0].message + assert "5 installed" in alerts[0].message + + def test_a_listed_user_carries_the_count_on_each_service(self): + artifact = _parse(ONE_UI_WITH_LIST) + assert len(artifact.results) == 2 + assert {record["installed_service_count"] for record in artifact.results} == {2} + assert all(record["component"] for record in artifact.results) + + def test_only_the_unlisted_user_gets_a_count_record(self): + artifact = _parse(TWO_USERS) + listed = [record for record in artifact.results if record["component"]] + unlisted = [record for record in artifact.results if not record["component"]] + assert [record["user_id"] for record in listed] == [0] + assert [record["user_id"] for record in unlisted] == [95] + assert unlisted[0]["installed_service_count"] == 3 + + def test_a_zero_count_adds_nothing(self): + # "Zero installed" is a negative result the empty section already + # states; a record for it would be noise. + assert _parse(ZERO_COUNT).results == [] diff --git a/tests/android/test_artifact_dumpsys_accessibility_state.py b/tests/android/test_artifact_dumpsys_accessibility_state.py new file mode 100644 index 00000000..b60368ab --- /dev/null +++ b/tests/android/test_artifact_dumpsys_accessibility_state.py @@ -0,0 +1,125 @@ +# Mobile Verification Toolkit (MVT) +# Copyright (c) 2021-2026 The MVT Authors. +# Use of this software is governed by the MVT License 1.1 that can be found at +# https://license.mvt.re/1.1/ +"""Installed is not the same as enabled, and the dump says which. + +Two things this guards: + + * a section the dump never printed must read as None ("not stated"), not as + False ("not enabled"); + * the alert must name the state, instead of firing identically on a device + where nothing is switched on and one where something is bound. +""" + +from types import SimpleNamespace + +from mvt.android.artifacts.dumpsys_accessibility import DumpsysAccessibilityArtifact +from mvt.common.alerts import AlertLevel + +from ..utils import get_artifact + +NO_STATE_SECTIONS = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, currentUser=true} + installed services: { + 0 : com.example.app/com.example.app.Service + } +""" + +ENABLED_BLOCK = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, currentUser=true} + installed services: { + 0 : com.example.app/com.example.app.Service + 1 : com.other.app/.Helper + } + enabled services: { + 0 : com.other.app/.Helper + } + bound services:{ + 0 : com.other.app/.Helper + } +""" + + +class _IndicatorsMatching: + """Minimal stand-in: matches one package id, like the STIX2 loader would.""" + + def __init__(self, package_name: str) -> None: + self.package_name = package_name + + def check_app_id(self, app_id): + if app_id != self.package_name: + return None + return SimpleNamespace( + message=f"Found a known suspicious app: {app_id}", ioc={"value": app_id} + ) + + +class TestAccessibilityServiceState: + def _parse(self, content): + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + artifact.parse(content) + return {r["component"]: r for r in artifact.results} + + def test_absent_sections_leave_the_state_unknown(self): + # None, not False: a build that does not print the sections says + # nothing about what is enabled, and that must not read as "nothing". + state = self._parse(NO_STATE_SECTIONS)[ + "com.example.app/com.example.app.Service" + ] + assert state["installed"] is True + assert state["enabled"] is None + assert state["bound"] is None + + def test_enabled_and_bound_are_attributed_per_service(self): + results = self._parse(ENABLED_BLOCK) + installed_only = results["com.example.app/com.example.app.Service"] + active = results["com.other.app/.Helper"] + assert (installed_only["enabled"], installed_only["bound"]) == (False, False) + assert (active["enabled"], active["bound"]) == (True, True) + + def test_alert_message_carries_the_state(self): + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + with open(get_artifact("android_data/dumpsys_accessibility.txt")) as handle: + artifact.parse(handle.read()) + artifact.check_indicators() + assert artifact.alertstore.alerts + assert all("installed" in alert.message for alert in artifact.alertstore.alerts) + + def test_a_switched_off_service_is_low_and_a_running_one_medium(self): + # A service the dump says is OFF still reaches the analyst, but must not + # compete with one that is actually bound. "Not stated" is not "off". + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + artifact.parse(ENABLED_BLOCK) + artifact.check_indicators() + by_level = {} + for alert in artifact.alertstore.alerts: + by_level.setdefault(alert.level, []).append(alert.message) + assert artifact.alertstore.count(AlertLevel.LOW) == 1 + assert artifact.alertstore.count(AlertLevel.MEDIUM) == 1 + assert "com.example.app" in by_level[AlertLevel.LOW][0] + assert "com.other.app" in by_level[AlertLevel.MEDIUM][0] + + def test_an_unstated_enabled_state_stays_medium(self): + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + artifact.parse(NO_STATE_SECTIONS) + artifact.check_indicators() + assert artifact.alertstore.count(AlertLevel.MEDIUM) == 1 + assert artifact.alertstore.count(AlertLevel.LOW) == 0 + + def test_a_disabled_service_is_still_matched_against_indicators(self): + # The state decides the severity of an ordinary finding, never whether + # the package is compared with the IOC feeds. + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + artifact.parse(ENABLED_BLOCK) + artifact.indicators = _IndicatorsMatching("com.example.app") + artifact.check_indicators() + assert artifact.alertstore.count(AlertLevel.CRITICAL) == 1 + assert artifact.alertstore.count(AlertLevel.LOW) == 0 From ad6caa155ff3778bbfa1a07ae8efdbacdfbdd61f Mon Sep 17 00:00:00 2001 From: va-resident Date: Fri, 25 Sep 2026 20:55:01 +0400 Subject: [PATCH 3/4] Track accessibility state sections and unnamed services per user Two issues from review: * The set of printed state sections was global, so a section printed for one user decided the flags of another. A user with an installed list and no enabled section was marked enabled=False and got a LOW "installed, not enabled" alert. Sections are now tracked per user. * A count-only record was added only when a user had no named component at all. A dump stating installedServiceCount=2 and naming one service read as complete. The count-only record is now added whenever the stated count exceeds the distinct named components for that user, and carries the difference in a new unnamed_service_count field. The module summary adds up that remainder. --- .../artifacts/dumpsys_accessibility.py | 50 ++++++++++++------- .../bugreport/dumpsys_accessibility.py | 6 +-- .../test_artifact_dumpsys_accessibility.py | 8 ++- ...st_artifact_dumpsys_accessibility_count.py | 49 ++++++++++++++++++ ...st_artifact_dumpsys_accessibility_state.py | 30 +++++++++++ 5 files changed, 121 insertions(+), 22 deletions(-) diff --git a/src/mvt/android/artifacts/dumpsys_accessibility.py b/src/mvt/android/artifacts/dumpsys_accessibility.py index 1a4a05ac..b6fe0e25 100644 --- a/src/mvt/android/artifacts/dumpsys_accessibility.py +++ b/src/mvt/android/artifacts/dumpsys_accessibility.py @@ -24,18 +24,26 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): "crashed", "accessibility_tool", "installed_service_count", + "unnamed_service_count", ) def check_indicators(self) -> None: for result in self.results: - # A stated count with no component names is a coverage statement, - # not a service: low, but not silent. + # A stated count the dump does not back with component names is a + # coverage statement, not a service: low, but not silent. if not result.get("component"): + stated = result["installed_service_count"] + unnamed = result["unnamed_service_count"] + if unnamed == stated: + detail = "does not list their component names" + else: + detail = ( + f"lists the component names of only {stated - unnamed} " + f"of them ({unnamed} unnamed)" + ) self.alertstore.low( - f"The accessibility dump states " - f"{result['installed_service_count']} installed " - f"service(s) for user {result['user_id']} but does not " - f"list their component names", + f"The accessibility dump states {stated} installed " + f"service(s) for user {result['user_id']} but {detail}", "", result, ) @@ -76,7 +84,9 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): self.results: list[dict[str, Any]] = [] services: dict[tuple[int | None, str], dict] = {} - seen_states: set[str] = set() + # Which state sections the dump printed, per user: one user's printed + # `enabled services` says nothing about another user's. + seen_states: dict[int | None, set[str]] = {} # `installedServiceCount=N` from the user's `attributes:{…}` line is on # most builds the only statement about installed services in the dump: # few print the `installed services: {…}` block. @@ -100,7 +110,7 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): ) if state_match: state = state_match.group(1).lower() - seen_states.add(self._state_field(state)) + seen_states.setdefault(user_id, set()).add(self._state_field(state)) inline = state_match.group(2) for component in re.findall( r"\{?([\w.$-]+/[\w.$-]+)(?:\s+\(A11yTool\))?\}?", inline @@ -133,21 +143,24 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): # empty: the first says nothing, the second says nothing is enabled. # Defaulting every flag to False would turn "not stated" into "not # enabled". Flags for sections this dump never printed stay None. + named: dict[int | None, int] = {} for (service_user, _component), service in services.items(): + printed = seen_states.get(service_user, set()) for state in ("installed", "enabled", "binding", "bound", "crashed"): - if self._state_field(state) not in seen_states: + if self._state_field(state) not in printed: service[self._state_field(state)] = None service["installed_service_count"] = installed_counts.get(service_user) + named[service_user] = named.get(service_user, 0) + 1 self.results.extend(services.values()) - # A stated count whose services were never listed would leave no trace: - # the module would log "a total of 0" about a dump that said five. - listed_users = {service_user for service_user, _component in services} + # A stated count the named services do not add up to would leave no + # trace of the rest: the module would log "a total of 0" about a dump + # that said five, or "a total of 1" about one that said two. for count_user, count in installed_counts.items(): - if count_user in listed_users or count == 0: - continue - self.results.append(self._new_unlisted(count_user, count)) + unnamed = count - named.get(count_user, 0) + if unnamed > 0: + self.results.append(self._new_unlisted(count_user, count, unnamed)) @staticmethod def _describe_state(result: dict) -> str: @@ -164,14 +177,16 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): return {"binding": "binding", "bound": "bound"}.get(state, state) @staticmethod - def _new_unlisted(user_id: int | None, count: int) -> dict: - """The dump's own count for a user whose services it did not list. + def _new_unlisted(user_id: int | None, count: int, unnamed: int) -> dict: + """The dump's own count for a user, and how many of those services it + did not name. Every other field stays unknown: the dump named no service to carry it. """ record: dict[str, Any] = dict.fromkeys(DumpsysAccessibilityArtifact._FIELDS) record["user_id"] = user_id record["installed_service_count"] = count + record["unnamed_service_count"] = unnamed return record @staticmethod @@ -184,4 +199,5 @@ class DumpsysAccessibilityArtifact(AndroidArtifact): record["package_name"], record["service_name"] = component.split("/", 1) # Filled in after parsing: the count is per user. record["installed_service_count"] = None + record["unnamed_service_count"] = None return record diff --git a/src/mvt/android/modules/bugreport/dumpsys_accessibility.py b/src/mvt/android/modules/bugreport/dumpsys_accessibility.py index 80991d37..fddee84a 100644 --- a/src/mvt/android/modules/bugreport/dumpsys_accessibility.py +++ b/src/mvt/android/modules/bugreport/dumpsys_accessibility.py @@ -48,7 +48,7 @@ class DumpsysAccessibility(DumpsysAccessibilityArtifact, BugReportModule): ) self.parse(content) - listed = stated = 0 + listed = unnamed = 0 for result in self.results: if result.get("component"): listed += 1 @@ -59,11 +59,11 @@ class DumpsysAccessibility(DumpsysAccessibilityArtifact, BugReportModule): # The operator gets this per user as a LOW alert from # check_indicators(); here it only has to survive into the summary, # so that a stated count never reads as "a total of 0". - stated += result.get("installed_service_count") or 0 + unnamed += result.get("unnamed_service_count") or 0 self.log.info( "Identified a total of %d accessibility services (%d more stated by " "the dump without a component name)", listed, - stated, + unnamed, ) diff --git a/tests/android/test_artifact_dumpsys_accessibility.py b/tests/android/test_artifact_dumpsys_accessibility.py index f21c79ed..6ba1a478 100644 --- a/tests/android/test_artifact_dumpsys_accessibility.py +++ b/tests/android/test_artifact_dumpsys_accessibility.py @@ -35,7 +35,9 @@ class TestDumpsysAccessibilityArtifact: assert len(da.results) == 0 da.parse(data) - assert len(da.results) == 1 + # One named service, plus one count-only record: the dump states + # `installedServiceCount=2` and names only one component. + assert len(da.results) == 2 assert da.results[0]["package_name"] == "com.malware.accessibility" assert da.results[0]["service_name"] == "com.malware.service.malwareservice" assert da.results[0]["enabled"] is True @@ -53,9 +55,11 @@ class TestDumpsysAccessibilityArtifact: da.check_indicators() - assert len(da.alertstore.alerts) == 1 + assert len(da.alertstore.alerts) == 2 assert da.alertstore.alerts[0].level == AlertLevel.MEDIUM assert da.alertstore.alerts[0].event == da.results[0] + assert da.alertstore.alerts[1].level == AlertLevel.LOW + assert da.alertstore.alerts[1].event == da.results[1] def test_same_component_is_kept_for_each_user(self): da = DumpsysAccessibilityArtifact() diff --git a/tests/android/test_artifact_dumpsys_accessibility_count.py b/tests/android/test_artifact_dumpsys_accessibility_count.py index 59765073..8c191d32 100644 --- a/tests/android/test_artifact_dumpsys_accessibility_count.py +++ b/tests/android/test_artifact_dumpsys_accessibility_count.py @@ -12,6 +12,8 @@ accessibility services" about a dump that said there are five. from mvt.android.artifacts.dumpsys_accessibility import DumpsysAccessibilityArtifact from mvt.common.alerts import AlertLevel +from ..utils import get_artifact + AOSP_NO_LIST = """\ ACCESSIBILITY MANAGER (dumpsys accessibility) User state[ @@ -43,6 +45,14 @@ User state[attributes:{id=95, installedServiceCount=3} Enabled services:{} """ +PARTIAL_TWO_USERS = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, installedServiceCount=3} + Enabled services:{{com.example.app/com.example.app.Service}} +User state[attributes:{id=10, installedServiceCount=1} + Enabled services:{{com.other.app/.Helper}} +""" + ZERO_COUNT = """\ ACCESSIBILITY MANAGER (dumpsys accessibility) User state[attributes:{id=0, installedServiceCount=0} @@ -98,3 +108,42 @@ class TestAccessibilityInstalledServiceCount: # "Zero installed" is a negative result the empty section already # states; a record for it would be noise. assert _parse(ZERO_COUNT).results == [] + + def test_a_partly_named_count_reports_the_unnamed_rest(self): + # The Android 14 fixture states `installedServiceCount=2` and names one + # enabled component. The listing is incomplete, and must not read as + # complete. + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + with open( + get_artifact("android_data/dumpsys_accessibility_v14_or_later.txt") + ) as handle: + artifact.parse(handle.read()) + unlisted = [record for record in artifact.results if not record["component"]] + assert len(unlisted) == 1 + assert unlisted[0]["installed_service_count"] == 2 + assert unlisted[0]["unnamed_service_count"] == 1 + + artifact.check_indicators() + low = [ + alert + for alert in artifact.alertstore.alerts + if alert.level == AlertLevel.LOW + ] + assert len(low) == 1 + assert "only 1 of them (1 unnamed)" in low[0].message + + def test_the_unnamed_rest_is_counted_per_user(self): + # User 0 names one of three, user 10 names its only one: the gap + # belongs to user 0 alone. + artifact = _parse(PARTIAL_TWO_USERS) + unlisted = [record for record in artifact.results if not record["component"]] + assert [ + (record["user_id"], record["unnamed_service_count"]) for record in unlisted + ] == [(0, 2)] + + def test_a_fully_named_count_adds_nothing(self): + artifact = _parse(ONE_UI_WITH_LIST) + assert all( + record["unnamed_service_count"] is None for record in artifact.results + ) diff --git a/tests/android/test_artifact_dumpsys_accessibility_state.py b/tests/android/test_artifact_dumpsys_accessibility_state.py index b60368ab..d6bbe789 100644 --- a/tests/android/test_artifact_dumpsys_accessibility_state.py +++ b/tests/android/test_artifact_dumpsys_accessibility_state.py @@ -42,6 +42,18 @@ User state[attributes:{id=0, currentUser=true} } """ +# User 0 prints only `installed services`, user 10 only `Enabled services`. +# Neither section speaks for the other user. +TWO_USERS_DIFFERENT_SECTIONS = """\ +ACCESSIBILITY MANAGER (dumpsys accessibility) +User state[attributes:{id=0, currentUser=true} + installed services: { + 0 : com.example.app/com.example.app.Service + } +User state[attributes:{id=10, currentUser=false} + Enabled services:{{com.other.app/.Helper}} +""" + class _IndicatorsMatching: """Minimal stand-in: matches one package id, like the STIX2 loader would.""" @@ -123,3 +135,21 @@ class TestAccessibilityServiceState: artifact.check_indicators() assert artifact.alertstore.count(AlertLevel.CRITICAL) == 1 assert artifact.alertstore.count(AlertLevel.LOW) == 0 + + def test_printed_sections_are_tracked_per_user(self): + artifact = DumpsysAccessibilityArtifact() + artifact.results = [] + artifact.parse(TWO_USERS_DIFFERENT_SECTIONS) + by_user = {r["user_id"]: r for r in artifact.results} + # User 0's enabled state is not stated, so it is unknown, not off. + assert (by_user[0]["installed"], by_user[0]["enabled"]) == (True, None) + # User 10's installed state is not stated either. + assert (by_user[10]["installed"], by_user[10]["enabled"]) == (None, True) + + artifact.check_indicators() + assert artifact.alertstore.count(AlertLevel.LOW) == 0 + assert artifact.alertstore.count(AlertLevel.MEDIUM) == 2 + assert not any( + "installed, not enabled" in alert.message + for alert in artifact.alertstore.alerts + ) From b1b9958f4d64b10b45603137711c7b1289ae860d Mon Sep 17 00:00:00 2001 From: va-resident Date: Tue, 22 Sep 2026 18:09:36 +0300 Subject: [PATCH 4/4] Descend into an OEM wrapper archive in check-bugreport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MIUI / HyperOS hands out a zip of app logs, ANR traces and tcpdump captures with the real bugreport--.zip nested inside. The outer archive carries none of the entry points the bug report modules read, so every module reported it found no files and check-bugreport still exited 0 with an empty result — an empty analysis that looks like a finished one. Name the entry points once in modules/bugreport/base.py, next to the _get_dumpstate_file() that tries them, and when an archive has none of them, open its zip members in memory and use the first one that does. Measured over 44 bug report collections: the 14 wrapper collections go from 0 artifacts to 260 artifacts and 586638 records; the 30 normal collections are unchanged, the descent being unreachable for them. Fixes #935 --- src/mvt/android/cmd_check_bugreport.py | 51 ++++++++++++++++- src/mvt/android/modules/bugreport/base.py | 13 ++++- tests/android/test_check_bugreport_wrapper.py | 56 +++++++++++++++++++ 3 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 tests/android/test_check_bugreport_wrapper.py diff --git a/src/mvt/android/cmd_check_bugreport.py b/src/mvt/android/cmd_check_bugreport.py index 036160d8..34dddbde 100644 --- a/src/mvt/android/cmd_check_bugreport.py +++ b/src/mvt/android/cmd_check_bugreport.py @@ -3,14 +3,19 @@ # Use of this software is governed by the MVT License 1.1 that can be found at # https://license.mvt.re/1.1/ +import fnmatch +import io import logging import os from pathlib import Path from typing import List, Optional -from zipfile import ZipFile +from zipfile import BadZipFile, ZipFile from mvt.android.artifacts.getprop import GetProp -from mvt.android.modules.bugreport.base import BugReportModule +from mvt.android.modules.bugreport.base import ( + DUMPSTATE_ENTRY_POINTS, + BugReportModule, +) from mvt.common.command import Command from mvt.common.indicators import Indicators from mvt.common.module import MVTModule @@ -88,6 +93,48 @@ class CmdAndroidCheckBugreport(Command): for file_name in self.__zip.namelist(): self.__files.append(file_name) + if not self._has_dumpstate(self.__files): + nested = self._nested_bugreport(bugreport_zip) + if nested: + self.__zip = nested + self.__files = list(nested.namelist()) + + @staticmethod + def _has_dumpstate(file_names: List[str]) -> bool: + """Whether these members carry any of the entry points the bug report + modules read (see `BugReportModule._get_dumpstate_file`).""" + return any( + fnmatch.filter(file_names, pattern) for pattern in DUMPSTATE_ENTRY_POINTS + ) + + def _nested_bugreport(self, outer: ZipFile) -> Optional[ZipFile]: + """Descend one level into an OEM wrapper archive. + + MIUI / HyperOS hands out a zip of app logs, ANR traces and tcpdump + captures with the real `bugreport--.zip` nested + inside. Without this descent the outer archive has no entry point, + every module reports it found no files, and the command still exits 0 + with an empty result. + """ + candidates = [ + name for name in outer.namelist() if name.lower().endswith(".zip") + ] + candidates.sort( + key=lambda name: ( + "bugreport" not in name.lower() and "dumpstate" not in name.lower() + ) + ) + for name in candidates: + try: + inner = ZipFile(io.BytesIO(outer.read(name))) + except (BadZipFile, OSError): + continue + if self._has_dumpstate(inner.namelist()): + log.info("Found the bug report nested inside the archive: %s", name) + return inner + inner.close() + return None + def init(self) -> None: if self.target_path: self.log.info("Checking Android bug report at path: %s", self.target_path) diff --git a/src/mvt/android/modules/bugreport/base.py b/src/mvt/android/modules/bugreport/base.py index 9e8954d6..8d4f4ace 100644 --- a/src/mvt/android/modules/bugreport/base.py +++ b/src/mvt/android/modules/bugreport/base.py @@ -18,6 +18,11 @@ from mvt.common.module import ModuleResults, MVTModule # section finishes and not necessarily between two sections. SECTION_DURATION = re.compile(r"^-{3,}\s*[0-9.]+s was the duration of", re.IGNORECASE) +# The members a bug report archive can be entered through, in the order +# _get_dumpstate_file() tries them. An archive carrying none of them is not a +# bug report at this level (see CmdAndroidCheckBugreport._has_dumpstate). +DUMPSTATE_ENTRY_POINTS = ("main_entry.txt", "dumpState_*.log", "*/dumpsys.txt") + class BugReportModule(MVTModule): """This class provides a base for all Android Bug Report modules.""" @@ -92,7 +97,9 @@ class BugReportModule(MVTModule): return data def _get_dumpstate_file(self) -> Optional[bytes]: - main = self._get_files_by_pattern("main_entry.txt") + main_entry, dumpstate_log, dumpsys_txt = DUMPSTATE_ENTRY_POINTS + + main = self._get_files_by_pattern(main_entry) if main: main_content = self._get_file_content(main[0]) try: @@ -100,11 +107,11 @@ class BugReportModule(MVTModule): except KeyError: return None - dumpstate_logs = self._get_files_by_pattern("dumpState_*.log") + dumpstate_logs = self._get_files_by_pattern(dumpstate_log) if dumpstate_logs: return self._get_file_content(dumpstate_logs[0]) - dumpsys_files = self._get_files_by_pattern("*/dumpsys.txt") + dumpsys_files = self._get_files_by_pattern(dumpsys_txt) if dumpsys_files: return self._get_file_content(dumpsys_files[0]) diff --git a/tests/android/test_check_bugreport_wrapper.py b/tests/android/test_check_bugreport_wrapper.py new file mode 100644 index 00000000..741641f2 --- /dev/null +++ b/tests/android/test_check_bugreport_wrapper.py @@ -0,0 +1,56 @@ +# Mobile Verification Toolkit (MVT) +# Copyright (c) 2021-2023 The MVT Authors. +# Use of this software is governed by the MVT License 1.1 that can be found at +# https://license.mvt.re/1.1/ +"""An OEM wrapper archive must not read as an empty bug report. + +MIUI / HyperOS hands out a zip of app logs with the real +`bugreport--.zip` nested inside. The outer archive has +none of the entry points the modules read, so every module +reported it found nothing and the command still exited 0 — an empty analysis +that looks like a finished one. +""" + +import io +import zipfile + +from mvt.android.cmd_check_bugreport import CmdAndroidCheckBugreport + +DUMPSTATE = "== dumpstate: 2026-01-01 00:00:00\nDUMP OF SERVICE package:\n" + + +def _inner_zip() -> bytes: + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as inner: + inner.writestr("main_entry.txt", "bugreport-test-2026-01-01-00-00-00.txt") + inner.writestr("bugreport-test-2026-01-01-00-00-00.txt", DUMPSTATE) + return buffer.getvalue() + + +def _wrapper_zip() -> zipfile.ZipFile: + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as outer: + outer.writestr("app_logs/hilog.txt", "unrelated OEM log\n") + outer.writestr("bugreport-test-2026-01-01-00-00-00.zip", _inner_zip()) + return zipfile.ZipFile(io.BytesIO(buffer.getvalue())) + + +class TestCheckBugreportWrapper: + def test_nested_bugreport_is_used(self, tmp_path): + cmd = CmdAndroidCheckBugreport(results_path=str(tmp_path)) + cmd.from_zip(_wrapper_zip()) + module = cmd.modules[0](results_path=str(tmp_path)) + cmd.module_init(module) + assert "main_entry.txt" in module.zip_files + + def test_plain_bugreport_is_left_alone(self, tmp_path): + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + archive.writestr("main_entry.txt", "bugreport.txt") + archive.writestr("bugreport.txt", DUMPSTATE) + archive.writestr("attachments/extra.zip", _inner_zip()) + cmd = CmdAndroidCheckBugreport(results_path=str(tmp_path)) + cmd.from_zip(zipfile.ZipFile(io.BytesIO(buffer.getvalue()))) + module = cmd.modules[0](results_path=str(tmp_path)) + cmd.module_init(module) + assert "attachments/extra.zip" in module.zip_files