From 3cfba8ec65b79c2c4446585059c6cda0dfd8ac5f Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <[email protected]>
Date: Wed, 08 Apr 2026 12:42:58 -0700
Subject: [PATCH] android_webview: Fix race condition and potential UAF in AwPacProcessor

A race condition in AwPacProcessor allowed asynchronous Android network
callbacks to execute after the underlying native object was destroyed,
potentially leading to a Use-After-Free (UAF).

This CL addresses the issue by:

1. Removing the 'final' modifier from 'mNativePacProcessor' to allow it
   to be cleared.
2. Setting 'mNativePacProcessor' to 0 in 'destroy()' before
   unregistering callbacks and destroying the native object.
3. Adding checks to ensure JNI calls are only made if
   'mNativePacProcessor' is non-zero.
4. Adding a test case to verify that 'destroy()' is safe and idempotent.

Fixed: 499238195
Change-Id: I963a9671b6deb78dac29c2310e9b15b04eac8719
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7736909
Reviewed-by: Richard (Torne) Coles <[email protected]>
Commit-Queue: Andrew Paseltiner <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1611728}
---

diff --git a/android_webview/java/src/org/chromium/android_webview/AwPacProcessor.java b/android_webview/java/src/org/chromium/android_webview/AwPacProcessor.java
index 09d84bb..999c05b90 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwPacProcessor.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwPacProcessor.java
@@ -29,7 +29,8 @@
 // TODO(amalova): remove UsedByReflection
 @UsedByReflection("Android")
 public class AwPacProcessor {
-    private final long mNativePacProcessor;
+    // 0 if it's already been destroyed.
+    private long mNativePacProcessor;
     private Network mNetwork;
     private ConnectivityManager.NetworkCallback mNetworkCallback;
 
@@ -65,6 +66,7 @@
     }
 
     public void setNetworkAndLinkAddresses(long networkHandle, List<String> addresses) {
+        if (mNativePacProcessor == 0) return;
         AwPacProcessorJni.get()
                 .setNetworkAndLinkAddresses(mNativePacProcessor, networkHandle, addresses);
     }
@@ -97,17 +99,22 @@
     // The calling code must not call any methods after it called destroy().
     @UsedByReflection("Android")
     public void destroy() {
+        if (mNativePacProcessor == 0) return;
+        long nativePacProcessor = mNativePacProcessor;
+        mNativePacProcessor = 0;
         unregisterNetworkCallback();
-        AwPacProcessorJni.get().destroyNative(mNativePacProcessor);
+        AwPacProcessorJni.get().destroyNative(nativePacProcessor);
     }
 
     @UsedByReflection("Android")
     public boolean setProxyScript(String script) {
+        if (mNativePacProcessor == 0) return false;
         return AwPacProcessorJni.get().setProxyScript(mNativePacProcessor, script);
     }
 
     @UsedByReflection("Android")
     public String makeProxyRequest(String url) {
+        if (mNativePacProcessor == 0) return null;
         return AwPacProcessorJni.get().makeProxyRequest(mNativePacProcessor, url);
     }
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java
index 1e2ccf9..758b4b6 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java
@@ -68,4 +68,20 @@
         mProcessor.setNetwork(null);
         Assert.assertEquals(proxyResultNetworkIsNotSet, mProcessor.makeProxyRequest(M_TEST_URL));
     }
+
+    @Test
+    @SmallTest
+    public void testDestroyIsIdempotentAndSafe() throws Throwable {
+        Assert.assertTrue(mProcessor.setProxyScript(PAC_SCRIPT));
+
+        mProcessor.destroy();
+
+        // Subsequent calls should fail gracefully and not crash.
+        Assert.assertFalse(mProcessor.setProxyScript(PAC_SCRIPT));
+        Assert.assertNull(mProcessor.makeProxyRequest(M_TEST_URL));
+        mProcessor.setNetworkAndLinkAddresses(42, List.of("1.2.3.4"));
+
+        // Calling destroy again should be safe.
+        mProcessor.destroy();
+    }
 }

diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java
index 1e2ccf9..758b4b6 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java
@@ -68,4 +68,20 @@
         mProcessor.setNetwork(null);
         Assert.assertEquals(proxyResultNetworkIsNotSet, mProcessor.makeProxyRequest(M_TEST_URL));
     }
+
+    @Test
+    @SmallTest
+    public void testDestroyIsIdempotentAndSafe() throws Throwable {
+        Assert.assertTrue(mProcessor.setProxyScript(PAC_SCRIPT));
+
+        mProcessor.destroy();
+
+        // Subsequent calls should fail gracefully and not crash.
+        Assert.assertFalse(mProcessor.setProxyScript(PAC_SCRIPT));
+        Assert.assertNull(mProcessor.makeProxyRequest(M_TEST_URL));
+        mProcessor.setNetworkAndLinkAddresses(42, List.of("1.2.3.4"));
+
+        // Calling destroy again should be safe.
+        mProcessor.destroy();
+    }
 }