From 3b696bd61c35756ebc1e79b0d8864b8fef5190c9 Mon Sep 17 00:00:00 2001
From: Ashley Newson <ashleynewson@chromium.org>
Date: Mon, 15 Jun 2026 15:05:35 -0700
Subject: [PATCH] [js_sandbox] Fix JsSandboxMessagePort thread safety

Teardown of a message port in the sandbox (due to closing the isolate)
could theoretically race with incoming messages from the app-side that
were dispatched before the app received the close signal. This could
theoretically lead to calls being directed to a destroyed native object.

Additionally, the constructor leaked `this` when setting the internal
message port client, so an incoming message could theoretically race
with returning from the constructor, resulting in undefined behavior.

This change adds Java synchronization around use of the native object
and avoids leakage of `this`.

Fixed: 523721871
Change-Id: Ifc5af234259de086bf0db416d5a8db684c1ee56d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7942016
Reviewed-by: Abhijith Nair <abhijithnair@chromium.org>
Commit-Queue: Ashley Newson <ashleynewson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1647108}
---

diff --git a/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java b/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java
index 873514b..01ac1f20 100644
--- a/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java
+++ b/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java
@@ -13,27 +13,42 @@
 
 import org.chromium.build.annotations.NullMarked;
 
+import javax.annotation.concurrent.GuardedBy;
+
 @JNINamespace("android_webview")
 @NullMarked
 public class JsSandboxMessagePort {
-    private final long mNativeJsSandboxMessagePort;
+    private final Object mLock = new Object();
     private final MessagePortInternal mMessagePortInternal;
 
+    @GuardedBy("mLock")
+    private long mNativeJsSandboxMessagePort;
+
     @CalledByNative
     public static JsSandboxMessagePort create(
             MessagePortInternal messagePortInternal, long nativeSandboxMessagePort) {
         JsSandboxMessagePort jsSandboxMessagePort =
                 new JsSandboxMessagePort(messagePortInternal, nativeSandboxMessagePort);
+        // Setup the client after construction to avoid undefined behavior from leaking `this` to
+        // the MessagePortInternal, which could otherwise be invoked from another thread before all
+        // fields have been initialized.
+        jsSandboxMessagePort.setupClient();
         return jsSandboxMessagePort;
     }
 
     private JsSandboxMessagePort(
             MessagePortInternal messagePortInternal, long nativeJsSandboxMessagePort) {
-        mNativeJsSandboxMessagePort = nativeJsSandboxMessagePort;
+        mMessagePortInternal = messagePortInternal;
+        synchronized (mLock) {
+            mNativeJsSandboxMessagePort = nativeJsSandboxMessagePort;
+        }
+    }
+
+    private void setupClient() {
         // TODO(b/435619571):
         //  Memory allocation checks should ideally happen before arbitrary size data is received
         //  (via FDs), as this could easily exhaust memory before the MessagePortClient is invoked.
-        messagePortInternal.setClient(
+        mMessagePortInternal.setClient(
                 new MessagePortInternal.MessagePortClient() {
                     @Override
                     public void onString(String string) {
@@ -41,28 +56,36 @@
                         // UTF-8, it may end up being more or less. As such, this is somewhat more
                         // of a best-effort estimate than a concrete value.
                         long size = (long) string.length() * Character.BYTES;
-                        if (!JsSandboxMessagePortJni.get()
-                                .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) {
-                            return;
+                        synchronized (mLock) {
+                            if (mNativeJsSandboxMessagePort == 0) {
+                                return;
+                            }
+                            if (!JsSandboxMessagePortJni.get()
+                                    .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) {
+                                return;
+                            }
+                            JsSandboxMessagePortJni.get()
+                                    .handleString(mNativeJsSandboxMessagePort, string, size);
                         }
-
-                        JsSandboxMessagePortJni.get()
-                                .handleString(mNativeJsSandboxMessagePort, string, size);
                     }
 
                     @Override
                     public void onArrayBuffer(byte[] arrayBuffer) {
                         long size = arrayBuffer.length;
-                        if (!JsSandboxMessagePortJni.get()
-                                .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) {
-                            return;
+                        synchronized (mLock) {
+                            if (mNativeJsSandboxMessagePort == 0) {
+                                return;
+                            }
+                            if (!JsSandboxMessagePortJni.get()
+                                    .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) {
+                                return;
+                            }
+                            JsSandboxMessagePortJni.get()
+                                    .handleArrayBuffer(
+                                            mNativeJsSandboxMessagePort, arrayBuffer, size);
                         }
-
-                        JsSandboxMessagePortJni.get()
-                                .handleArrayBuffer(mNativeJsSandboxMessagePort, arrayBuffer, size);
                     }
                 });
-        mMessagePortInternal = messagePortInternal;
     }
 
     // Called by isolate thread
@@ -80,6 +103,9 @@
     // Called by isolate thread
     @CalledByNative
     void close() {
+        synchronized (mLock) {
+            mNativeJsSandboxMessagePort = 0;
+        }
         mMessagePortInternal.close();
     }