// ============================================================================ // Windows-only deterministic reproducer (apply on a Windows checkout). // // This patch makes FakeGattCharacteristicWinrt::GetDescriptorsAsync() return // an IAsyncOperation whose put_Completed() returns E_FAIL for one chosen // characteristic, then drives a GATT discovery with >=2 characteristics so // the loop in BluetoothGattDiscovererWinrt::OnGetCharacteristics() continues // after callback_.Run(false) destroys |this|. // // Build & run on Windows: // autoninja -C out\asan device_unittests // out\asan\device_unittests.exe \ // --gtest_filter=*GattDiscovererPutCompletedFailureUAF* // // Expected: ASAN heap-use-after-free in // BluetoothGattDiscovererWinrt::OnGetCharacteristics() // reading the freed std::vector> backing array // on the iteration following the put_Completed failure. // ============================================================================ diff --git a/device/bluetooth/test/fake_gatt_characteristic_winrt.cc b/device/bluetooth/test/fake_gatt_characteristic_winrt.cc --- a/device/bluetooth/test/fake_gatt_characteristic_winrt.cc +++ b/device/bluetooth/test/fake_gatt_characteristic_winrt.cc @@ -21,6 +21,45 @@ #include "device/bluetooth/test/fake_gatt_value_changed_event_args_winrt.h" #include "device/bluetooth/test/fake_gatt_write_result_winrt.h" +namespace device { +namespace { + +// IAsyncOperation whose put_Completed() returns a +// failure HRESULT. Models the real-world case where the WinRT object is in a +// closed/disconnected state (RO_E_CLOSED / RPC_E_DISCONNECTED) by the time +// Chrome attaches the completion handler. +class PutCompletedFailingDescriptorsOp + : public Microsoft::WRL::RuntimeClass< + Microsoft::WRL::RuntimeClassFlags< + Microsoft::WRL::WinRt | Microsoft::WRL::InhibitRoOriginateError>, + ABI::Windows::Foundation::IAsyncOperation< + ABI::Windows::Devices::Bluetooth::GenericAttributeProfile:: + GattDescriptorsResult*>> { + public: + using Handler = ABI::Windows::Foundation::IAsyncOperationCompletedHandler< + ABI::Windows::Devices::Bluetooth::GenericAttributeProfile:: + GattDescriptorsResult*>; + IFACEMETHODIMP put_Completed(Handler*) override { + // This is what makes PostAsyncResults() return FAILED(hr) at + // bluetooth_gatt_discoverer_winrt.cc:416-426. + return E_FAIL; + } + IFACEMETHODIMP get_Completed(Handler**) override { return E_NOTIMPL; } + IFACEMETHODIMP GetResults( + ABI::Windows::Devices::Bluetooth::GenericAttributeProfile:: + IGattDescriptorsResult**) override { + return E_NOTIMPL; + } +}; + +} // namespace + +// 1-based index of the GetDescriptorsAsync() call that should hand back a +// put_Completed-failing op. 0 = disabled. +int g_fail_put_completed_on_nth_descriptors_async = 0; +} // namespace device + namespace device { namespace { @@ -193,6 +232,14 @@ HRESULT FakeGattCharacteristicWinrt::remove_ValueChanged( HRESULT FakeGattCharacteristicWinrt::GetDescriptorsAsync( IAsyncOperation** operation) { + static int call_count = 0; + ++call_count; + if (g_fail_put_completed_on_nth_descriptors_async && + call_count == g_fail_put_completed_on_nth_descriptors_async) { + *operation = Make().Detach(); + return S_OK; // GetDescriptorsAsync itself succeeds; put_Completed fails. + } + auto async_op = Make>(); base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask( FROM_HERE, diff --git a/device/bluetooth/test/fake_gatt_characteristic_winrt.h b/device/bluetooth/test/fake_gatt_characteristic_winrt.h --- a/device/bluetooth/test/fake_gatt_characteristic_winrt.h +++ b/device/bluetooth/test/fake_gatt_characteristic_winrt.h @@ -22,6 +22,9 @@ namespace device { class BluetoothTestWinrt; + +// POC hook: see .cc. +extern int g_fail_put_completed_on_nth_descriptors_async; class FakeGattDescriptorWinrt; class FakeGattCharacteristicWinrt diff --git a/device/bluetooth/test/fake_gatt_device_service_winrt.h b/device/bluetooth/test/fake_gatt_device_service_winrt.h --- a/device/bluetooth/test/fake_gatt_device_service_winrt.h +++ b/device/bluetooth/test/fake_gatt_device_service_winrt.h @@ -123,6 +123,10 @@ class FakeGattDeviceServiceWinrt void SimulateGattCharacteristic(std::string_view uuid, int proporties); + const std::vector>& + fake_characteristics() const { + return fake_characteristics_; + } void ClearBluetoothTestWinrt(); private: diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc --- a/device/bluetooth/bluetooth_device_unittest.cc +++ b/device/bluetooth/bluetooth_device_unittest.cc @@ -34,6 +34,9 @@ #if BUILDFLAG(IS_WIN) #include "device/bluetooth/bluetooth_device_winrt.h" #include "device/bluetooth/test/bluetooth_test_win.h" +#include "device/bluetooth/test/fake_bluetooth_le_device_winrt.h" +#include "device/bluetooth/test/fake_gatt_characteristic_winrt.h" +#include "device/bluetooth/test/fake_gatt_device_service_winrt.h" #endif @@ -2440,6 +2443,56 @@ TEST_P(BluetoothTestWinrt, FailedGetGattServicesAfterConnection) { } #endif // BUILDFLAG(IS_WIN) +#if BUILDFLAG(IS_WIN) +// POC for missing-return UAF in +// BluetoothGattDiscovererWinrt::OnGetCharacteristics. When PostAsyncResults +// (i.e. IAsyncOperation::put_Completed) fails, callback_.Run(false) is invoked +// without `return`, the owner synchronously deletes the discoverer, and the +// range-for continues over the freed characteristics_list backing storage. +TEST_P(BluetoothTestWinrt, GattDiscovererPutCompletedFailureUAF) { + InitWithFakeAdapter(); + StartLowEnergyDiscoverySession(); + BluetoothDevice* device = SimulateLowEnergyDevice(3); + + // Kick off connect (creates BluetoothGattDiscovererWinrt and posts + // GetGattServicesAsync). + device->CreateGattConnection( + GetGattConnectionCallback(Call::EXPECTED, Result::FAILURE)); + SimulateGattConnection(device); + + // Populate one service WITH 3 characteristics on the fake BLE device so that + // the discoverer's OnGetCharacteristics loop has multiple iterations. We + // reach into the fake directly because the public SimulateGattCharacteristic + // helper expects a BluetoothRemoteGattService* that only exists after a + // successful discovery. + FakeBluetoothLEDeviceWinrt* ble = + static_cast(device)->ble_device(); + ble->SimulateGattServicesDiscovered({kTestUUIDHeartRate}, {}); + // The above posted the GetGattServicesAsync result; before it runs we add + // characteristics to the fake service so GetCharacteristicsAsync sees them. + auto& services = const_cast< + std::vector>&>( + ble->fake_services()); + ASSERT_EQ(1u, services.size()); + for (int i = 0; i < 3; ++i) { + services[0]->SimulateGattCharacteristic(kTestUUIDHeartRateMeasurement, + /*properties=*/0); + } + + // Make the FIRST characteristic's GetDescriptorsAsync hand back an + // IAsyncOperation whose put_Completed() returns E_FAIL. The loop will then + // run callback_.Run(false) -> gatt_discoverer_.reset() -> |this| freed, + // and continue to characteristic #2: virtual call through freed ComPtr slot. + g_fail_put_completed_on_nth_descriptors_async = 1; + + // Drive discovery: OnGetGattServices -> OnServiceOpen -> OnGetCharacteristics. + // ASAN reports heap-use-after-free here. + base::RunLoop().RunUntilIdle(); + + g_fail_put_completed_on_nth_descriptors_async = 0; +} +#endif // BUILDFLAG(IS_WIN) + #if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_APPLE) || BUILDFLAG(IS_WIN) #define MAYBE_GetPrimaryServices GetPrimaryServices #else diff --git a/device/bluetooth/test/fake_bluetooth_le_device_winrt.h b/device/bluetooth/test/fake_bluetooth_le_device_winrt.h --- a/device/bluetooth/test/fake_bluetooth_le_device_winrt.h +++ b/device/bluetooth/test/fake_bluetooth_le_device_winrt.h @@ -160,6 +160,10 @@ class FakeBluetoothLEDeviceWinrt void SimulateGattCharacteristic(BluetoothRemoteGattService* service, std::string_view uuid, int properties); + const std::vector>& + fake_services() const { + return fake_services_; + } void SimulateGattDescriptor(BluetoothRemoteGattCharacteristic* characteristic, std::string_view uuid); void SimulateGattNameChange(const std::string& new_name);