CVE-2024-40660 AI-generated analysis
platform/frameworks/native, SurfaceFlinger transaction handling (services/surfaceflinger/SurfaceFlinger.cpp, libs/gui ISurfaceComposer)
Makes SurfaceFlinger::setTransactionState sanitize the DisplayState entries in place (iterate by reference over a non-const Vector<DisplayState>) so that permission-stripping applies to the display states actually used, instead of to discarded copies.
Root cause
In SurfaceFlinger::setTransactionState the per-caller permissions computed by LayerStatePermissions::getTransactionPermissions were applied to display states with `for (DisplayState display : displays) { display.sanitize(permissions); }`. The loop variable is a by-value copy and the `displays` parameter was `const Vector<DisplayState>&`, so sanitize() changed a temporary and the original, unsanitized DisplayState entries stayed in the vector. Composer states were handled correctly (`for (auto& composerState : states)`); only the display loop was wrong. As the new test shows, a caller without the required permission could therefore have display-level changes such as a setDisplayProjection orientation change applied.
How the fix works
- services/surfaceflinger/SurfaceFlinger.cpp:SurfaceFlinger::setTransactionState: the loop becomes `for (DisplayState& display : displays)`, so DisplayState::sanitize(permissions) changes the actual elements of the vector.
- libs/gui/include/gui/ISurfaceComposer.h:ISurfaceComposer::setTransactionState and services/surfaceflinger/SurfaceFlinger.h: the `displays` parameter changes from `const Vector<DisplayState>&` to `Vector<DisplayState>&` so that in-place sanitization is allowed. The same signature change is made in libs/gui/ISurfaceComposer.cpp:BpSurfaceComposer::setTransactionState, where the proxy still only reads and serializes the vector.
- libs/gui/SurfaceComposerClient.cpp:SurfaceComposerClient::doUncacheBufferTransaction: a temporary `{}` can no longer bind to a non-const reference, so the code now creates a named empty `Vector<DisplayState> displayStates` and passes it.
- Test doubles updated to the new signature: libs/gui/tests/Surface_test.cpp:FakeSurfaceComposer::setTransactionState, plus the forwarding wrappers in services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h and services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h.
- services/surfaceflinger/tests/Credentials_test.cpp: the new DisplayTransactionPermissionTest checks that a setDisplayProjection transaction from AID_BIN leaves the display orientation unchanged, while the same transaction from AID_SYSTEM changes it.
Completeness
- The fix covers the single DisplayState sanitization loop in SurfaceFlinger::setTransactionState, which is the only place in the material where display states are sanitized. The ComposerState loop was already by reference and is untouched.
- This commit does not change DisplayState::sanitize itself (which fields are cleared for which permissions), and the material does not show its body. What exactly is now blocked is only shown for the projection/orientation case in the test.
- The Binder server side (BnSurfaceComposer::onTransact deserializing into the Vector passed to setTransactionState) is not shown. The fix depends on that path passing a mutable vector, which the non-const signature now enforces at compile time.
Behavior changes
- DisplayState::sanitize now strips display-level transaction changes from callers that lack the relevant transaction permission. The transaction otherwise goes through: there is no new error code, and the test expects it to 'fail silently'.
- ISurfaceComposer::setTransactionState signature change: `displays` is now a non-const lvalue reference, so callers can no longer pass a braced temporary. Any out-of-tree implementer or caller of this libgui interface must be updated.
Per major
- Android 14: same fix: commit 064ce6e3235b in android-security-14.0.0_r12 (Change-Id I500ff9a5...). The same diff landed again as 24bddbdd6d19 (same Change-Id) and as eb7cf7e9df61 (Change-Id Ia86633ba...), both in android-security-14.0.0_r14. At android-security-14.0.0_r29, 7/8 lines of the patched region are present, and the context shows the by-reference loop and the non-const signature.
- Android 15: same fix: commit 95185d7fe46c in android-security-15.0.0_r2. The functional diff is identical, plus a test-only lambda capture change (`[=, this]`) in Credentials_test.cpp:CaptureLayersTest. At android-security-15.0.0_r17, 7/8 lines of the patched region are present.
- Android 16: not affected (not listed by the bulletin); 3/8 lines of the patched region present at android-security-16.0.0_r8.
- Android 17: not affected (not listed by the bulletin); 3/8 lines of the patched region present at android-security-17.0.0_r1.
Unknowns
- Body of DisplayState::sanitize, and which DisplayState fields and permissions it enforces (not in the material).
- Why Android 16/17 are not listed by the bulletin, and what the equivalent code looks like there (only partial line matches are reported).
- Which public AOSP 14 release tags carry the fix. Per index search, android-14.0.0_r75 still shows the const signature, but the index may be incomplete or stale.
- Exactly which callers can reach setTransactionState with display states, given the header comment mentioning ACCESS_SURFACE_FLINGER.