From 8b38650ae0b4930bc1cfc172ebe88c7d3d4a1b9e Mon Sep 17 00:00:00 2001
From: Rob Pitkin <robpitkin@google.com>
Date: Mon, 12 Oct 2026 22:04:50 -0700
Subject: [PATCH] bluetooth: Fix UAF in BluetoothPairingBlueZ on synchronous cancel

PairingDelegate implementations (such as DevicePairingHandler in Ash
when a system-modal pairing dialog fails to open or is immediately
dismissed) can synchronously invoke BluetoothDevice::CancelPairing() or
BluetoothAdapter::RemovePairingDelegate() from inside delegate callbacks
like DisplayPinCode(), DisplayPasskey(), or KeysEntered().

Because BluetoothDeviceBlueZ::CancelPairing() synchronously calls
EndPairing(), which resets std::unique_ptr<BluetoothPairingBlueZ>, the
active BluetoothPairingBlueZ instance is destroyed while its method or
caller is still on the stack. This causes a use-after-free when:

1. BluetoothPairingBlueZ::DisplayPinCode() or KeysEntered() accesses
   `device_` after `pairing_delegate_` returns.
2. BluetoothAdapterBlueZ::DisplayPasskey() calls
   `pairing->KeysEntered(entered)` after `pairing->DisplayPasskey()`
   synchronously destroyed `pairing`.

This CL adds a WeakPtrFactory to BluetoothPairingBlueZ and checks weak
pointer validity after external delegate and callback invocations before
accessing member state or making follow-up calls.

Bug: 566687517
Change-Id: I4d16dff447062be9cae39cfd65ef8433ef87e0e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8477932
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Rob Pitkin <robpitkin@chromium.org>
Reviewed-by: Vincent Scheib <scheib@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1717336}
---
diff --git a/device/bluetooth/bluez/bluetooth_adapter_bluez.cc b/device/bluetooth/bluez/bluetooth_adapter_bluez.cc
index 71039738..702348f 100644
--- a/device/bluetooth/bluez/bluetooth_adapter_bluez.cc
+++ b/device/bluetooth/bluez/bluetooth_adapter_bluez.cc
@@ -1076,9 +1076,15 @@
   if (!pairing)
     return;
 
-  if (entered == 0)
+  if (entered == 0) {
+    // DisplayPasskey() notifies the pairing delegate, which may cancel pairing
+    // and destroy `pairing` before returning.
+    base::WeakPtr<BluetoothPairingBlueZ> weak_pairing = pairing->GetWeakPtr();
     pairing->DisplayPasskey(passkey);
-
+    if (!weak_pairing) {
+      return;
+    }
+  }
   pairing->KeysEntered(entered);
 }
 
diff --git a/device/bluetooth/bluez/bluetooth_pairing_bluez.cc b/device/bluetooth/bluez/bluetooth_pairing_bluez.cc
index c42d36e..bb2bba56 100644
--- a/device/bluetooth/bluez/bluetooth_pairing_bluez.cc
+++ b/device/bluetooth/bluez/bluetooth_pairing_bluez.cc
@@ -65,7 +65,13 @@
 void BluetoothPairingBlueZ::DisplayPinCode(const std::string& pincode) {
   ResetCallbacks();
   pairing_delegate_used_ = true;
+
+  base::WeakPtr<BluetoothPairingBlueZ> weak_this =
+      weak_ptr_factory_.GetWeakPtr();
   pairing_delegate_->DisplayPinCode(device_, pincode);
+  if (!weak_this) {
+    return;
+  }
 
   // If this is not an outgoing connection to the device, the pairing context
   // needs to be cleaned up again as there's no reliable indication of
@@ -108,7 +114,13 @@
 
 void BluetoothPairingBlueZ::KeysEntered(uint16_t entered) {
   pairing_delegate_used_ = true;
+
+  base::WeakPtr<BluetoothPairingBlueZ> weak_this =
+      weak_ptr_factory_.GetWeakPtr();
   pairing_delegate_->KeysEntered(device_, entered);
+  if (!weak_this) {
+    return;
+  }
 
   // If this is not an outgoing connection to the device, the pairing context
   // needs to be cleaned up again as there's no reliable indication of
@@ -169,6 +181,10 @@
   return pairing_delegate_;
 }
 
+base::WeakPtr<BluetoothPairingBlueZ> BluetoothPairingBlueZ::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void BluetoothPairingBlueZ::ResetCallbacks() {
   if (!pincode_callback_.is_null()) {
     std::move(pincode_callback_)
diff --git a/device/bluetooth/bluez/bluetooth_pairing_bluez.h b/device/bluetooth/bluez/bluetooth_pairing_bluez.h
index e6843b21..aa5221df 100644
--- a/device/bluetooth/bluez/bluetooth_pairing_bluez.h
+++ b/device/bluetooth/bluez/bluetooth_pairing_bluez.h
@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/dbus/bluetooth_agent_service_provider.h"
 
@@ -114,6 +115,8 @@
   // Returns the pairing delegate being used by this pairing object.
   device::BluetoothDevice::PairingDelegate* GetPairingDelegate() const;
 
+  base::WeakPtr<BluetoothPairingBlueZ> GetWeakPtr();
+
  private:
   // Internal method to run the current set of callbacks with canceled state
   // because a new request has arrived that supersedes them.
@@ -146,6 +149,8 @@
       passkey_callback_;
   bluez::BluetoothAgentServiceProvider::Delegate::ConfirmationCallback
       confirmation_callback_;
+
+  base::WeakPtrFactory<BluetoothPairingBlueZ> weak_ptr_factory_{this};
 };
 
 }  // namespace bluez
diff --git a/device/bluetooth/bluez/bluetooth_bluez_unittest.cc b/device/bluetooth/bluez/bluetooth_bluez_unittest.cc
index 899a93aa..3b681ca 100644
--- a/device/bluetooth/bluez/bluetooth_bluez_unittest.cc
+++ b/device/bluetooth/bluez/bluetooth_bluez_unittest.cc
@@ -3612,6 +3612,101 @@
   EXPECT_FALSE(device->IsPaired());
 }
 
+TEST_P(BluetoothBlueZTestP, PairingCancelledSynchronouslyAtDisplayPinCode) {
+  fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
+
+  GetAdapter();
+  DiscoverDevices();
+
+  BluetoothDevice* device = adapter_->GetDevice(
+      bluez::FakeBluetoothDeviceClient::kDisplayPinCodeAddress);
+  ASSERT_TRUE(device != nullptr);
+  ASSERT_FALSE(device->IsPaired());
+
+  StrictMock<MockPairingDelegate> pairing_delegate;
+  // Note: Synchronous cancel
+  // The delegate cancels pairing from inside its callout. CancelPairing()
+  // synchronously ends pairing, destroying the BluetoothPairingBlueZ while
+  // the method that called the delegate is still on the stack. That method
+  // must check its WeakPtr before touching members, or it reads freed memory.
+  EXPECT_CALL(pairing_delegate, DisplayPinCode(_, "123456"))
+      .WillOnce([](BluetoothDevice* device, const std::string& pincode) {
+        device->CancelPairing();
+      });
+
+  base::test::TestFuture<std::optional<BluetoothDevice::ConnectErrorCode>>
+      connect_future;
+  PerformConnect(device, &pairing_delegate, connect_future.GetCallback());
+  EXPECT_EQ(BluetoothDevice::ERROR_AUTH_CANCELED, connect_future.Get());
+  EXPECT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnecting());
+  EXPECT_FALSE(device->IsPaired());
+}
+
+TEST_P(BluetoothBlueZTestP, PairingCancelledSynchronouslyAtDisplayPasskey) {
+  fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
+
+  GetAdapter();
+  DiscoverDevices();
+
+  BluetoothDevice* device = adapter_->GetDevice(
+      bluez::FakeBluetoothDeviceClient::kDisplayPasskeyAddress);
+  ASSERT_TRUE(device != nullptr);
+  ASSERT_FALSE(device->IsPaired());
+
+  StrictMock<MockPairingDelegate> pairing_delegate;
+  // See "Note: Synchronous cancel" above. Cancelling inside DisplayPasskey()
+  // means BluetoothAdapterBlueZ::DisplayPasskey() must not go on to call
+  // KeysEntered() on the destroyed pairing.
+  EXPECT_CALL(pairing_delegate, DisplayPasskey(_, 123456U))
+      .WillOnce([](BluetoothDevice* device, uint32_t passkey) {
+        device->CancelPairing();
+      });
+  EXPECT_CALL(pairing_delegate, KeysEntered(_, _)).Times(0);
+
+  base::test::TestFuture<std::optional<BluetoothDevice::ConnectErrorCode>>
+      connect_future;
+  PerformConnect(device, &pairing_delegate, connect_future.GetCallback());
+  EXPECT_EQ(BluetoothDevice::ERROR_AUTH_CANCELED, connect_future.Get());
+  EXPECT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnecting());
+  EXPECT_FALSE(device->IsPaired());
+}
+
+TEST_P(BluetoothBlueZTestP, PairingCancelledSynchronouslyAtKeysEntered) {
+  fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
+
+  GetAdapter();
+  DiscoverDevices();
+
+  BluetoothDevice* device = adapter_->GetDevice(
+      bluez::FakeBluetoothDeviceClient::kDisplayPasskeyAddress);
+  ASSERT_TRUE(device != nullptr);
+  ASSERT_FALSE(device->IsPaired());
+
+  StrictMock<MockPairingDelegate> pairing_delegate;
+  EXPECT_CALL(pairing_delegate, DisplayPasskey(_, 123456U)).Times(1);
+  // See "Note: Synchronous cancel" above. Cancelling inside the final
+  // KeysEntered() means BluetoothPairingBlueZ::KeysEntered() must not read
+  // `device_` afterwards.
+  testing::InSequence seq;
+  for (int i = 0; i < 7; ++i) {
+    EXPECT_CALL(pairing_delegate, KeysEntered(_, i));
+  }
+  EXPECT_CALL(pairing_delegate, KeysEntered(_, 7))
+      .WillOnce([](BluetoothDevice* device, uint32_t entered) {
+        device->CancelPairing();
+      });
+
+  base::test::TestFuture<std::optional<BluetoothDevice::ConnectErrorCode>>
+      connect_future;
+  PerformConnect(device, &pairing_delegate, connect_future.GetCallback());
+  EXPECT_EQ(BluetoothDevice::ERROR_AUTH_CANCELED, connect_future.Get());
+  EXPECT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnecting());
+  EXPECT_FALSE(device->IsPaired());
+}
+
 TEST_P(BluetoothBlueZTestP, PairingCancelledInFlight) {
   fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
 

diff --git a/device/bluetooth/bluez/bluetooth_bluez_unittest.cc b/device/bluetooth/bluez/bluetooth_bluez_unittest.cc
index 899a93aa..3b681ca 100644
--- a/device/bluetooth/bluez/bluetooth_bluez_unittest.cc
+++ b/device/bluetooth/bluez/bluetooth_bluez_unittest.cc
@@ -3612,6 +3612,101 @@
   EXPECT_FALSE(device->IsPaired());
 }
 
+TEST_P(BluetoothBlueZTestP, PairingCancelledSynchronouslyAtDisplayPinCode) {
+  fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
+
+  GetAdapter();
+  DiscoverDevices();
+
+  BluetoothDevice* device = adapter_->GetDevice(
+      bluez::FakeBluetoothDeviceClient::kDisplayPinCodeAddress);
+  ASSERT_TRUE(device != nullptr);
+  ASSERT_FALSE(device->IsPaired());
+
+  StrictMock<MockPairingDelegate> pairing_delegate;
+  // Note: Synchronous cancel
+  // The delegate cancels pairing from inside its callout. CancelPairing()
+  // synchronously ends pairing, destroying the BluetoothPairingBlueZ while
+  // the method that called the delegate is still on the stack. That method
+  // must check its WeakPtr before touching members, or it reads freed memory.
+  EXPECT_CALL(pairing_delegate, DisplayPinCode(_, "123456"))
+      .WillOnce([](BluetoothDevice* device, const std::string& pincode) {
+        device->CancelPairing();
+      });
+
+  base::test::TestFuture<std::optional<BluetoothDevice::ConnectErrorCode>>
+      connect_future;
+  PerformConnect(device, &pairing_delegate, connect_future.GetCallback());
+  EXPECT_EQ(BluetoothDevice::ERROR_AUTH_CANCELED, connect_future.Get());
+  EXPECT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnecting());
+  EXPECT_FALSE(device->IsPaired());
+}
+
+TEST_P(BluetoothBlueZTestP, PairingCancelledSynchronouslyAtDisplayPasskey) {
+  fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
+
+  GetAdapter();
+  DiscoverDevices();
+
+  BluetoothDevice* device = adapter_->GetDevice(
+      bluez::FakeBluetoothDeviceClient::kDisplayPasskeyAddress);
+  ASSERT_TRUE(device != nullptr);
+  ASSERT_FALSE(device->IsPaired());
+
+  StrictMock<MockPairingDelegate> pairing_delegate;
+  // See "Note: Synchronous cancel" above. Cancelling inside DisplayPasskey()
+  // means BluetoothAdapterBlueZ::DisplayPasskey() must not go on to call
+  // KeysEntered() on the destroyed pairing.
+  EXPECT_CALL(pairing_delegate, DisplayPasskey(_, 123456U))
+      .WillOnce([](BluetoothDevice* device, uint32_t passkey) {
+        device->CancelPairing();
+      });
+  EXPECT_CALL(pairing_delegate, KeysEntered(_, _)).Times(0);
+
+  base::test::TestFuture<std::optional<BluetoothDevice::ConnectErrorCode>>
+      connect_future;
+  PerformConnect(device, &pairing_delegate, connect_future.GetCallback());
+  EXPECT_EQ(BluetoothDevice::ERROR_AUTH_CANCELED, connect_future.Get());
+  EXPECT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnecting());
+  EXPECT_FALSE(device->IsPaired());
+}
+
+TEST_P(BluetoothBlueZTestP, PairingCancelledSynchronouslyAtKeysEntered) {
+  fake_bluetooth_device_client_->SetSimulationIntervalMs(10);
+
+  GetAdapter();
+  DiscoverDevices();
+
+  BluetoothDevice* device = adapter_->GetDevice(
+      bluez::FakeBluetoothDeviceClient::kDisplayPasskeyAddress);
+  ASSERT_TRUE(device != nullptr);
+  ASSERT_FALSE(device->IsPaired());
+
+  StrictMock<MockPairingDelegate> pairing_delegate;
+  EXPECT_CALL(pairing_delegate, DisplayPasskey(_, 123456U)).Times(1);
+  // See "Note: Synchronous cancel" above. Cancelling inside the final
+  // KeysEntered() means BluetoothPairingBlueZ::KeysEntered() must not read
+  // `device_` afterwards.
+  testing::InSequence seq;
+  for (int i = 0; i < 7; ++i) {
+    EXPECT_CALL(pairing_delegate, KeysEntered(_, i));
+  }
+  EXPECT_CALL(pairing_delegate, KeysEntered(_, 7))
+      .WillOnce([](BluetoothDevice* device, uint32_t entered) {
+        device->CancelPairing();
+      });
+
+  base::test::TestFuture<std::optional<BluetoothDevice::ConnectErrorCode>>
+      connect_future;
+  PerformConnect(device, &pairing_delegate, connect_future.GetCallback());
+  EXPECT_EQ(BluetoothDevice::ERROR_AUTH_CANCELED, connect_future.Get());
+  EXPECT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnecting());
+  EXPECT_FALSE(device->IsPaired());
+}
+
 TEST_P(BluetoothBlueZTestP, PairingCancelledInFlight) {
   fake_bluetooth_device_client_->SetSimulationIntervalMs(10);