diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc index 0461cd32c0bf9..07a6ec977f852 100644 --- a/services/device/usb/mojo/device_impl_unittest.cc +++ b/services/device/usb/mojo/device_impl_unittest.cc @@ -860,6 +860,123 @@ TEST_F(USBDeviceImplTest, ClaimProtectedInterface) { EXPECT_CALL(mock_handle(), Close()); } +// POC: SetInterfaceAlternateSetting bypasses blocked_interface_classes_. +// +// A compromised renderer holding a WebUSB chooser grant can send +// SetInterfaceAlternateSetting() over the device::mojom::UsbDevice pipe to +// drive a SET_INTERFACE control transfer to a protected-class interface, +// even though: +// * ClaimInterface() on that interface is rejected with kProtectedClass +// * The wire-equivalent ControlTransferOut(STANDARD, INTERFACE, 0x0B, ...) +// is rejected with PERMISSION_DENIED by HasControlTransferPermission() +// +// On the usbfs backend (Linux/ChromeOS/Android), the handle layer also lacks +// a claimed-interface check, so the call falls through to +// ioctl(fd, USBDEVFS_SETINTERFACE, ...) and the kernel auto-claims the +// protected interface for Chrome's fd before issuing SET_INTERFACE. +// +// This test PASSES because it asserts the buggy behaviour: the call to the +// underlying device handle is NOT gated. A fixed DeviceImpl would cause the +// EXPECT_CALL(SetInterfaceAlternateSettingInternal).Times(1) below to fail. +TEST_F(USBDeviceImplTest, SetInterfaceAlternateSettingProtectedClassBypass) { + constexpr uint8_t kHidClass = 0x03; // WebUSB protected class. + constexpr uint8_t kProtectedIface = 1; + constexpr uint8_t kProtectedAlt = 1; + + mojo::Remote device = + GetMockDeviceProxyWithBlockedInterfaces( + base::span_from_ref(uint8_t{kHidClass})); + + EXPECT_CALL(mock_device(), OpenInternal(_)); + { + base::test::TestFuture future; + device->Open(future.GetCallback()); + EXPECT_TRUE(future.Get()->is_success()); + } + + // Composite device: interface 0 is vendor-specific (0xFF), interface 1 is + // HID (protected) with two alternate settings. + AddMockConfig(ConfigBuilder(/*configuration_value=*/1) + .AddInterface(/*interface_number=*/0, + /*alternate_setting=*/0, + /*class_code=*/0xFF, /*subclass_code=*/0, + /*protocol_code=*/0) + .AddInterface(/*interface_number=*/kProtectedIface, + /*alternate_setting=*/0, + /*class_code=*/kHidClass, /*subclass_code=*/0, + /*protocol_code=*/0) + .AddInterface(/*interface_number=*/kProtectedIface, + /*alternate_setting=*/kProtectedAlt, + /*class_code=*/kHidClass, /*subclass_code=*/0, + /*protocol_code=*/0) + .Build()); + + EXPECT_CALL(mock_handle(), SetConfigurationInternal(1, _)); + { + base::RunLoop loop; + device->SetConfiguration( + 1, base::BindOnce(&ExpectResultAndThen, true, loop.QuitClosure())); + loop.Run(); + } + + // Sanity check: ClaimInterface on the protected interface IS blocked at the + // DeviceImpl layer and never reaches the handle. + EXPECT_CALL(mock_handle(), ClaimInterfaceInternal(kProtectedIface, _)) + .Times(0); + { + base::RunLoop loop; + device->ClaimInterface( + kProtectedIface, + base::BindLambdaForTesting([&](mojom::UsbClaimInterfaceResult result) { + EXPECT_EQ(result, mojom::UsbClaimInterfaceResult::kProtectedClass); + loop.Quit(); + })); + loop.Run(); + } + + // Sanity check: the wire-equivalent SET_INTERFACE control transfer + // (bmRequestType=0x01, bRequest=0x0B, wValue=alt, wIndex=iface) IS blocked + // by HasControlTransferPermission() and never reaches the handle. + EXPECT_CALL(mock_handle(), ControlTransferInternal(_, _, _, _, _, _, _, _, _)) + .Times(0); + { + auto params = mojom::UsbControlTransferParams::New(); + params->type = UsbControlTransferType::STANDARD; + params->recipient = UsbControlTransferRecipient::INTERFACE; + params->request = 0x0B; // SET_INTERFACE + params->value = kProtectedAlt; // wValue = alternate setting + params->index = kProtectedIface; // wIndex = interface number + base::RunLoop loop; + device->ControlTransferOut( + std::move(params), {}, /*timeout=*/0, + base::BindOnce(&ExpectTransferStatusAndThen, + mojom::UsbTransferStatus::PERMISSION_DENIED, + loop.QuitClosure())); + loop.Run(); + } + + // *** VULNERABILITY *** + // SetInterfaceAlternateSetting on the SAME protected interface is NOT + // blocked: the call reaches the underlying device handle. On usbfs this + // results in ioctl(fd, USBDEVFS_SETINTERFACE, {iface=1, alt=1}) being + // issued without the interface ever having been claimed by Chrome. + EXPECT_CALL(mock_handle(), SetInterfaceAlternateSettingInternal( + kProtectedIface, kProtectedAlt, _)) + .Times(1); + { + base::RunLoop loop; + // The mock handle returns success (interface/alt exist in the config), + // mirroring usbfs behaviour where the kernel auto-claims and accepts. + device->SetInterfaceAlternateSetting( + kProtectedIface, kProtectedAlt, + base::BindOnce(&ExpectResultAndThen, /*expected_result=*/true, + loop.QuitClosure())); + loop.Run(); + } + + EXPECT_CALL(mock_handle(), Close()); +} + TEST_F(USBDeviceImplTest, SetInterfaceAlternateSetting) { mojo::Remote device = GetMockDeviceProxy();