From ad6caa155ff3778bbfa1a07ae8efdbacdfbdd61f Mon Sep 17 00:00:00 2001 From: va-resident Date: Fri, 25 Sep 2026 20:55:01 +0400 Subject: [PATCH] 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 + )