From c54faf3e691b3d952f649756578eab6f8a5d3208 Mon Sep 17 00:00:00 2001
From: Robert Carr <racarr@google.com>
Date: Sat, 22 Jan 2022 16:49:31 -0800
Subject: [PATCH] DO NOT MERGE: WM: Call 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: I8d4a0ebe0cdfaed7ff1ad862724d49a14ed04478
(cherry picked from commit cdf139841567a625608b94339fa49cb895f6a69f)
---
 core/java/android/view/SurfaceControl.java             | 10 +++++++++-
 core/jni/android_view_SurfaceControl.cpp               |  7 +++++++
 .../com/android/server/wm/WindowManagerService.java    |  4 ++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java
index 4dc9807aca5d0..1e290920b00b8 100644
--- a/core/java/android/view/SurfaceControl.java
+++ b/core/java/android/view/SurfaceControl.java
@@ -239,6 +239,7 @@ public final class SurfaceControl implements Parcelable {
     private static native int nativeGetGPUContextPriority();
     private static native void nativeSetTransformHint(long nativeObject, int transformHint);
     private static native int nativeGetTransformHint(long nativeObject);
+    private static native void nativeSanitize(long transactionObject);
 
     @Nullable
     @GuardedBy("mLock")
@@ -3439,7 +3440,14 @@ public final class SurfaceControl implements Parcelable {
             return this;
         }
 
-         /**
+        /**
+         * @hide
+         */
+        public void sanitize() {
+            nativeSanitize(mNativeObject);
+        }
+
+        /**
          * Merge the other transaction into this transaction, clearing the
          * other transaction as if it had been applied.
          *
diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp
index 7f99e2998b13d..4cb258dc0bbde 100644
--- a/core/jni/android_view_SurfaceControl.cpp
+++ b/core/jni/android_view_SurfaceControl.cpp
@@ -875,6 +875,11 @@ static void nativeSetDropInputMode(JNIEnv* env, jclass clazz, jlong transactionO
     transaction->setDropInputMode(ctrl, static_cast<gui::DropInputMode>(mode));
 }
 
+static void nativeSanitize(JNIEnv* env, jclass clazz, jlong transactionObj) {
+    auto transaction = reinterpret_cast<SurfaceComposerClient::Transaction*>(transactionObj);
+    transaction->sanitize();
+}
+
 static jlongArray nativeGetPhysicalDisplayIds(JNIEnv* env, jclass clazz) {
     const auto displayIds = SurfaceComposerClient::getPhysicalDisplayIds();
     jlongArray array = env->NewLongArray(displayIds.size());
@@ -2003,6 +2008,8 @@ static const JNINativeMethod sSurfaceControlMethods[] = {
             (void*)nativeSetTrustedOverlay },
     {"nativeSetDropInputMode", "(JJI)V",
             (void*)nativeSetDropInputMode},
+    {"nativeSanitize", "(J)V",
+            (void*) nativeSanitize }
         // clang-format on
 };
 
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index fac953910e927..b69182f41b76f 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2643,6 +2643,10 @@ public class WindowManagerService extends IWindowManager.Stub
 
     void finishDrawingWindow(Session session, IWindow client,
             @Nullable SurfaceControl.Transaction postDrawTransaction) {
+        if (postDrawTransaction != null) {
+            postDrawTransaction.sanitize();
+        }
+
         final long origId = Binder.clearCallingIdentity();
         try {
             synchronized (mGlobalLock) {