From 79808850064b50228087091034ce2753d8bf85a1 Mon Sep 17 00:00:00 2001
From: Chavi Weingarten <chaviw@google.com>
Date: Fri, 14 Apr 2023 19:06:27 +0000
Subject: [PATCH] Sanitize transactions in SSG

When SSG's complete, they send the Transaction to their parent which
could live in another process. This can be problematic if the parent
process has higher privilege than the child and the child added
something into that transaction that only privileged processes can call.
Therefore, sanitize transactions as they cross binder using the calling
uid and pid in case the calling process does have the correct
privileges.

Test: Coming...
Bug: 267794530
Change-Id: Ic0dd2619c22afe57c673c9484b526b841ef1ac99
---
 core/java/android/view/SurfaceControl.java                  | 6 +++---
 core/java/android/window/SurfaceSyncGroup.java              | 1 +
 core/jni/android_view_SurfaceControl.cpp                    | 6 +++---
 .../java/com/android/server/wm/WindowManagerService.java    | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java
index 99deac4c8cf47..62fdfaea13037 100644
--- a/core/java/android/view/SurfaceControl.java
+++ b/core/java/android/view/SurfaceControl.java
@@ -280,7 +280,7 @@ public final class SurfaceControl implements Parcelable {
     private static native int nativeGetLayerId(long nativeObject);
     private static native void nativeAddTransactionCommittedListener(long nativeObject,
             TransactionCommittedListener listener);
-    private static native void nativeSanitize(long transactionObject);
+    private static native void nativeSanitize(long transactionObject, int pid, int uid);
     private static native void nativeSetDestinationFrame(long transactionObj, long nativeObject,
             int l, int t, int r, int b);
     private static native void nativeSetDefaultApplyToken(IBinder token);
@@ -3960,8 +3960,8 @@ public final class SurfaceControl implements Parcelable {
         /**
          * @hide
          */
-        public void sanitize() {
-            nativeSanitize(mNativeObject);
+        public void sanitize(int pid, int uid) {
+            nativeSanitize(mNativeObject, pid, uid);
         }
 
         /**
diff --git a/core/java/android/window/SurfaceSyncGroup.java b/core/java/android/window/SurfaceSyncGroup.java
index 18405676dbd84..84a6ecb30602d 100644
--- a/core/java/android/window/SurfaceSyncGroup.java
+++ b/core/java/android/window/SurfaceSyncGroup.java
@@ -715,6 +715,7 @@ public final class SurfaceSyncGroup {
                     public void onTransactionReady(Transaction t) {
                         synchronized (mLock) {
                             if (t != null) {
+                                t.sanitize(Binder.getCallingPid(), Binder.getCallingUid());
                                 // When an older parent sync group is added due to a child syncGroup
                                 // getting added to multiple groups, we need to maintain merge order
                                 // so the older parentSyncGroup transactions are overwritten by
diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp
index 193099baad487..424925328dfd6 100644
--- a/core/jni/android_view_SurfaceControl.cpp
+++ b/core/jni/android_view_SurfaceControl.cpp
@@ -972,9 +972,9 @@ static void nativeSurfaceFlushJankData(JNIEnv* env, jclass clazz, jlong nativeOb
     SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction(ctrl);
 }
 
-static void nativeSanitize(JNIEnv* env, jclass clazz, jlong transactionObj) {
+static void nativeSanitize(JNIEnv* env, jclass clazz, jlong transactionObj, jint pid, jint uid) {
     auto transaction = reinterpret_cast<SurfaceComposerClient::Transaction*>(transactionObj);
-    transaction->sanitize();
+    transaction->sanitize(pid, uid);
 }
 
 static void nativeSetDestinationFrame(JNIEnv* env, jclass clazz, jlong transactionObj,
@@ -2268,7 +2268,7 @@ static const JNINativeMethod sSurfaceControlMethods[] = {
             (void*) nativeSetTrustedPresentationCallback },
     {"nativeClearTrustedPresentationCallback", "(JJ)V",
             (void*) nativeClearTrustedPresentationCallback },
-    {"nativeSanitize", "(J)V",
+    {"nativeSanitize", "(JII)V",
             (void*) nativeSanitize },
     {"nativeSetDestinationFrame", "(JJIIII)V",
                 (void*)nativeSetDestinationFrame },
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index f9b6fc10360df..65d17763ba412 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2675,7 +2675,7 @@ public class WindowManagerService extends IWindowManager.Stub
     void finishDrawingWindow(Session session, IWindow client,
             @Nullable SurfaceControl.Transaction postDrawTransaction, int seqId) {
         if (postDrawTransaction != null) {
-            postDrawTransaction.sanitize();
+            postDrawTransaction.sanitize(Binder.getCallingPid(), Binder.getCallingUid());
         }
 
         final long origId = Binder.clearCallingIdentity();