CVE-2024-23706 AI-generated analysis
platform/packages/modules/HealthFitness (Health Connect), change logs API: framework ChangeLogTokenRequest and service HealthConnectServiceImpl
Health Connect now rejects change-log token requests (and stored tokens) whose record-type set is empty, both in the framework ChangeLogTokenRequest constructor and in HealthConnectServiceImpl.getChangeLogToken / getChangeLogs. Previously the Builder documented that no record types meant all record types; getChangeLogToken now returns ERROR_INVALID_ARGUMENT in that case.
Root cause
Before the fix, ChangeLogTokenRequest.Builder documented that if no record type was added the request "includes all record types". In HealthConnectServiceImpl.getChangeLogToken the data permission check is mDataPermissionEnforcer.enforceRecordIdsReadPermissions(request.getRecordTypesList(), attributionSource), and getChangeLogs calls enforceRecordIdsReadPermissions on changeLogsTokenRequest.getRecordTypes() from the stored token. The fix adds empty-set rejection immediately before both calls. Taken together, this suggests an empty record-type set could get a token covering all types without matching read-permission enforcement; the internals of the enforcer and of ChangeLogsRequestHelper are not in the material. The Parcel constructor ChangeLogTokenRequest(Parcel in) does no validation, so a client-side check alone would not be enough.
How the fix works
- framework/java/android/health/connect/changelog/ChangeLogTokenRequest.java: the private constructor ChangeLogTokenRequest(Set<DataOrigin>, Set<Class<? extends Record>>) now throws IllegalArgumentException("Requested record types must not be empty") when recordTypes.isEmpty(). The Builder.addRecordType Javadoc changes from "If not set includes all record types" to "At least one record type must be set".
- service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java: getChangeLogToken: after throwExceptionIfDataSyncInProgress() and before mDataPermissionEnforcer.enforceRecordIdsReadPermissions(...), adds `if (request.getRecordTypes().isEmpty()) throw new IllegalArgumentException(...)`. This is the server-side check for requests arriving over Binder, including those deserialized through the unvalidated Parcel constructor.
- service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java: getChangeLogToken: adds a `catch (IllegalArgumentException)` block. It sets HealthConnectException.ERROR_INVALID_ARGUMENT on the logger builder, logs via Slog.e and reports the error to the callback through tryAndThrowException(callback, e, ERROR_INVALID_ARGUMENT).
- service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java: getChangeLogs: after ChangeLogsRequestHelper.getRequest(packageName, token.getToken()) resolves the stored TokenRequest, adds `if (changeLogsTokenRequest.getRecordTypes().isEmpty()) throw new IllegalArgumentException(...)` before enforceRecordIdsReadPermissions. This also rejects stored tokens with an empty record-type set.
- Tests: CTS tests that built `new ChangeLogTokenRequest.Builder().build()` now add record types (new TestUtils.getChangeLogTokenRequestForTestRecordTypes()). The host-side test testAppCanReadChangeLogsUsingDataOriginFilters in HealthConnectDeviceTest is removed. The commit message names a new test, testGetChangeLogToken_emptyRecordTypes_throwsException.
Completeness
- Both service paths that use record types from a change-log token now have the check: getChangeLogToken (request from the client) and getChangeLogs (record types read back from the stored token).
- The server-side check in getChangeLogToken sits before enforceRecordIdsReadPermissions, so an empty list never reaches the permission enforcement step.
- The framework-side check is only in the private Set-based constructor. The Parcel constructor ChangeLogTokenRequest(Parcel) is still unvalidated, and the service-side re-check covers that case.
- In the constructor the new isEmpty() check runs before Objects.requireNonNull(recordTypes), so a null recordTypes now fails in isEmpty() rather than in requireNonNull (functional nit).
- The diff adds a throw of IllegalArgumentException in getChangeLogs but no new catch block there. Whether getChangeLogs already handled IllegalArgumentException is not visible in the material.
- No other API (for example read or aggregate paths) is modified. The material does not show whether other request types use a similar empty-means-all default.
Behavior changes
- ChangeLogTokenRequest construction with no record types now throws IllegalArgumentException on the client.
- HealthConnectManager.getChangeLogToken with an empty record-type set now fails with HealthConnectException.ERROR_INVALID_ARGUMENT instead of returning a token.
- getChangeLogs with a token whose stored record-type set is empty now throws IllegalArgumentException instead of returning change logs.
- API documentation change: at least one record type is required. The commit message acknowledges that developers who requested change logs without record types will no longer be able to.
- In getChangeLogToken, the service logger now records ERROR_INVALID_ARGUMENT for this case.
Per major
- Android 14: same fix: commit 6e6896c3fd81 (cherry-pick of cd228a3e21c9, Change-Id I5dd49131fbe5eaa8bc138be03550f7fc739786d9) in android-security-14.0.0_r8. At android-security-14.0.0_r29 the code context shows the checks in the ChangeLogTokenRequest constructor and in HealthConnectServiceImpl.getChangeLogToken, including the IllegalArgumentException catch; 6/8 patched-region lines match.
- Android 15: not affected (not listed by the bulletin); at android-security-15.0.0_r17, 6/8 lines of the patched region are present.
- Android 16: not affected (not listed by the bulletin); at android-security-16.0.0_r8, 6/8 lines of the patched region are present.
- Android 17: not affected (not listed by the bulletin); at android-security-17.0.0_r1, 3/8 lines of the patched region are present, and the material does not show whether an equivalent check exists in another form.
Unknowns
- Whether getChangeLogs already caught IllegalArgumentException and mapped it to ERROR_INVALID_ARGUMENT (not in the diff).
- How enforceRecordIdsReadPermissions handles an empty list, and how ChangeLogsRequestHelper.getToken / getRequest expanded an empty record-type set when reading change logs.
- Which 2 of the 8 patched-region lines differ at android-security-14.0.0_r29, 15.0.0_r17 and 16.0.0_r8.
- Whether the Android 17 code keeps an equivalent check (3/8 patched lines present).
- Why the host-side test testAppCanReadChangeLogsUsingDataOriginFilters was removed.
- Whether other Health Connect request types use a similar empty-means-all default.