From 842a412840fbab2332a27d646a167177b110abf4 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 7 Jan 2022 18:23:06 -0800 Subject: DO NOT MERGE: SurfaceFlinger: Add Transaction#sanitize Various elements of the Transaction interface require a permission in order to apply. In particular the setTrustedOverlay and setInputWindowInfo fields. These permission checks are implemented by checking the PID and the UID of the process which sent the transaction. Unfortunately widespread use of transaction merging makes this inadequate. At the moment IWindowSession#finishDrawing seems to be the only boundary on which transactions move from client to system processes, and so we expose a sanitize method and use it from there to resolve the situation in an easily backportable way. Moving forward it likely make sense to move security sensitive interfaces off of Transaction. Most of the things behind permissions currently are not truly security sensitive, more of just a request not to use them. It was also considered to sanitize transactions at all process boundaries through writeToParcel, however this could be disruptive as previously permissioned processes (WM and SysUI) could freely exchange transactions. As the change needs to be backportable the lowest risk option was chosen. Bug: 213644870 Test: Existing tests pass Change-Id: I424f45bc30ea8e56e4c4493203ee0749eabf239c (cherry picked from commit de6d7b467e572d384f2bc1bc788259340ebe2f93) --- libs/gui/SurfaceComposerClient.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'libs/gui/SurfaceComposerClient.cpp') diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 6ffd25d227..3f1277ed57 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -527,6 +527,13 @@ SurfaceComposerClient::Transaction::Transaction(const Transaction& other) mListenerCallbacks = other.mListenerCallbacks; } +void SurfaceComposerClient::Transaction::sanitize() { + for (auto & [handle, composerState] : mComposerStates) { + composerState.state.sanitize(0 /* permissionMask */); + } + mInputWindowCommands.clear(); +} + std::unique_ptr SurfaceComposerClient::Transaction::createFromParcel(const Parcel* parcel) { auto transaction = std::make_unique(); @@ -611,7 +618,6 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel if (composerState.read(*parcel) == BAD_VALUE) { return BAD_VALUE; } - composerStates[surfaceControlHandle] = composerState; } -- cgit v1.2.3-59-g8ed1b