Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactIncorrect authorization in USB
DescriptionIncorrect authorization in USB
ComponentUSB
Bug ClassLogic Error
Tracker524520965
Fix commitb334f9d5a8d0 (chromium/src) +105/-37
CISA KEVNot listed
Creditedhongan
Disclosed2026-08-25

Changed Functions

FunctionChangeNotes
if
services/device/usb/mojo/device_impl.cc
modified
TEST_F
services/device/usb/mojo/device_impl_unittest.cc
modified

Files Changed

  • services/device/usb/mojo/device_impl.cc
  • services/device/usb/mojo/device_impl_unittest.cc
From b334f9d5a8d0a54b39b0f17aa2149cdb3fd7feab Mon Sep 17 00:00:00 2001
From: Alvin Ji <[email protected]>
Date: Tue, 07 Jul 2026 16:52:07 -0700
Subject: [PATCH] usb: Block device-level CLASS requests if any interface is protected

WebUSB blocklist enforcement for device-level CLASS requests could be
bypassed by specifying a valid, non-protected interface index in the
wIndex field. For devices that ignore wIndex and route all device-level
requests to the active interface (which might be protected), this
allowed bypassing the protection.

This change blocks all device-level CLASS requests (recipient DEVICE or
OTHER) if the device configuration contains any protected interface,
regardless of the wIndex value.

Change-Id: Ia9cbd87f346c5938695809f4c0f3bfd43cb093de
Bug: 524520965
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7963404
Reviewed-by: Matt Reynolds <[email protected]>
Commit-Queue: Alvin Ji <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1658378}
---

diff --git a/services/device/usb/mojo/device_impl.cc b/services/device/usb/mojo/device_impl.cc
index b5df00a..9b3492bd 100644
--- a/services/device/usb/mojo/device_impl.cc
+++ b/services/device/usb/mojo/device_impl.cc
@@ -311,39 +311,34 @@
   // 2. CLASS Requests
   // ==========================================
   if (type == UsbControlTransferType::CLASS) {
-    const mojom::UsbInterfaceInfo* interface = nullptr;
-    if (recipient == UsbControlTransferRecipient::ENDPOINT) {
-      interface = device_handle_->FindInterfaceByEndpoint(index & 0xff);
-    } else {
-      // For CLASS requests, we assume index identifies the interface for all
-      // other recipients (INTERFACE, DEVICE, OTHER).
-      interface = FindInterface(config, index & 0xff);
-    }
+    if (recipient == UsbControlTransferRecipient::INTERFACE ||
+        recipient == UsbControlTransferRecipient::ENDPOINT) {
+      const mojom::UsbInterfaceInfo* interface = nullptr;
+      if (recipient == UsbControlTransferRecipient::ENDPOINT) {
+        interface = device_handle_->FindInterfaceByEndpoint(index & 0xff);
+      } else {
+        interface = FindInterface(config, index & 0xff);
+      }
 
-    // Block if the targeted interface is protected.
-    if (interface) {
+      if (!interface) {
+        return BlockAndLog(
+            WebUsbControlTransferPermissionOutcome::kError_InterfaceNotFound);
+      }
+
       auto blocked_class = FindBlockedClass(interface);
       if (blocked_class) {
         LogBlockedControlTransfer(*blocked_class, direction, type);
         return BlockAndLog(WebUsbControlTransferPermissionOutcome::kBlocked);
       }
+
+      return AllowAndLog(WebUsbControlTransferPermissionOutcome::kAllowed);
     }
 
-    // For requests explicitly targeting an INTERFACE or ENDPOINT, the interface
-    // must actually exist in the current configuration.
-    if (recipient == UsbControlTransferRecipient::INTERFACE ||
-        recipient == UsbControlTransferRecipient::ENDPOINT) {
-      return interface ? AllowAndLog(
-                             WebUsbControlTransferPermissionOutcome::kAllowed)
-                       : BlockAndLog(WebUsbControlTransferPermissionOutcome::
-                                         kError_InterfaceNotFound);
-    }
-
-    // For DEVICE and OTHER recipients, if we could not identify the target
-    // interface, we must block it if the device has any protected interfaces.
-    // This prevents bypassing the blocklist by specifying an invalid interface
-    // number (e.g. 0xFF) on a device that ignores the wIndex field.
-    if (!interface && HasProtectedInterface(config)) {
+    // For DEVICE and OTHER recipients, we block the request if the device
+    // has any protected interfaces. This prevents bypassing the blocklist by
+    // specifying a non-protected interface number (or an invalid one) on a
+    // device that ignores the wIndex field.
+    if (HasProtectedInterface(config)) {
       return BlockAndLog(WebUsbControlTransferPermissionOutcome::kBlocked);
     }
 
diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc
index 7df57bd..cea062d 100644
--- a/services/device/usb/mojo/device_impl_unittest.cc
+++ b/services/device/usb/mojo/device_impl_unittest.cc
@@ -1482,8 +1482,7 @@
   EXPECT_CALL(mock_handle(), Close());
 }
 
-TEST_F(USBDeviceImplTest,
-       ClassControlTransferToDeviceWithProtectedInterfaceBypass) {
+TEST_F(USBDeviceImplTest, ClassControlTransferBlockedIfProtected) {
   // Block interface class 2.
   mojo::Remote<mojom::UsbDevice> device =
       GetMockDeviceProxyWithBlockedInterfaces(base::span_from_ref(uint8_t{2}));
@@ -1496,8 +1495,13 @@
     EXPECT_TRUE(future.Get()->is_success());
   }
 
+  // Interface 0 has class 3 (allowed).
   // Interface 1 has class 2 (blocked).
   AddMockConfig(ConfigBuilder(/*configuration_value=*/1)
+                    .AddInterface(/*interface_number=*/0,
+                                  /*alternate_setting=*/0,
+                                  /*class_code=*/3, /*subclass_code=*/0,
+                                  /*protocol_code=*/0)
                     .AddInterface(/*interface_number=*/1,
                                   /*alternate_setting=*/0,
                                   /*class_code=*/2, /*subclass_code=*/0,
@@ -1512,10 +1516,9 @@
     EXPECT_TRUE(future.Get());
   }
 
+  // Recipient: DEVICE
+  // Case 1: Invalid index (0xFF).
   {
-    // A CLASS request to the DEVICE with index 0xFF (not matching any
-    // interface) should be BLOCKED because the device has a protected interface
-    // (interface 1).
     auto params = mojom::UsbControlTransferParams::New();
     params->type = UsbControlTransferType::CLASS;
     params->recipient = UsbControlTransferRecipient::DEVICE;
@@ -1526,13 +1529,83 @@
     std::vector<uint8_t> fake_data = {1, 2, 3};
     AddMockInboundData(fake_data);
 
-    base::RunLoop loop;
-    device->ControlTransferIn(
-        std::move(params), static_cast<uint32_t>(fake_data.size()), 0,
-        base::BindOnce(&ExpectTransferInAndThen,
-                       mojom::UsbTransferStatus::PERMISSION_DENIED,
-                       std::vector<uint8_t>(), loop.QuitClosure()));
-    loop.Run();
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
+  }
+
+  // Recipient: DEVICE
+  // Case 2: Valid index pointing to non-protected interface (0).
+  {
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
+    params->recipient = UsbControlTransferRecipient::DEVICE;
+    params->request = 5;
+    params->value = 6;
+    params->index = 0;  // Valid interface 0 (non-protected)
+
+    std::vector<uint8_t> fake_data = {1, 2, 3};
+    AddMockInboundData(fake_data);
+
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
+  }
+
+  // Recipient: OTHER
+  // Case 1: Invalid index (0xFF).
+  {
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
+    params->recipient = UsbControlTransferRecipient::OTHER;
+    params->request = 5;
+    params->value = 6;
+    params->index = 0xFF;  // Does not exist
+
+    std::vector<uint8_t> fake_data = {1, 2, 3};
+    AddMockInboundData(fake_data);
+
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
+  }
+
+  // Recipient: OTHER
+  // Case 2: Valid index pointing to non-protected interface (0).
+  {
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc
index 7df57bd..cea062d 100644
--- a/services/device/usb/mojo/device_impl_unittest.cc
+++ b/services/device/usb/mojo/device_impl_unittest.cc
@@ -1482,8 +1482,7 @@
   EXPECT_CALL(mock_handle(), Close());
 }
 
-TEST_F(USBDeviceImplTest,
-       ClassControlTransferToDeviceWithProtectedInterfaceBypass) {
+TEST_F(USBDeviceImplTest, ClassControlTransferBlockedIfProtected) {
   // Block interface class 2.
   mojo::Remote<mojom::UsbDevice> device =
       GetMockDeviceProxyWithBlockedInterfaces(base::span_from_ref(uint8_t{2}));
@@ -1496,8 +1495,13 @@
     EXPECT_TRUE(future.Get()->is_success());
   }
 
+  // Interface 0 has class 3 (allowed).
   // Interface 1 has class 2 (blocked).
   AddMockConfig(ConfigBuilder(/*configuration_value=*/1)
+                    .AddInterface(/*interface_number=*/0,
+                                  /*alternate_setting=*/0,
+                                  /*class_code=*/3, /*subclass_code=*/0,
+                                  /*protocol_code=*/0)
                     .AddInterface(/*interface_number=*/1,
                                   /*alternate_setting=*/0,
                                   /*class_code=*/2, /*subclass_code=*/0,
@@ -1512,10 +1516,9 @@
     EXPECT_TRUE(future.Get());
   }
 
+  // Recipient: DEVICE
+  // Case 1: Invalid index (0xFF).
   {
-    // A CLASS request to the DEVICE with index 0xFF (not matching any
-    // interface) should be BLOCKED because the device has a protected interface
-    // (interface 1).
     auto params = mojom::UsbControlTransferParams::New();
     params->type = UsbControlTransferType::CLASS;
     params->recipient = UsbControlTransferRecipient::DEVICE;
@@ -1526,13 +1529,83 @@
     std::vector<uint8_t> fake_data = {1, 2, 3};
     AddMockInboundData(fake_data);
 
-    base::RunLoop loop;
-    device->ControlTransferIn(
-        std::move(params), static_cast<uint32_t>(fake_data.size()), 0,
-        base::BindOnce(&ExpectTransferInAndThen,
-                       mojom::UsbTransferStatus::PERMISSION_DENIED,
-                       std::vector<uint8_t>(), loop.QuitClosure()));
-    loop.Run();
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
+  }
+
+  // Recipient: DEVICE
+  // Case 2: Valid index pointing to non-protected interface (0).
+  {
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
+    params->recipient = UsbControlTransferRecipient::DEVICE;
+    params->request = 5;
+    params->value = 6;
+    params->index = 0;  // Valid interface 0 (non-protected)
+
+    std::vector<uint8_t> fake_data = {1, 2, 3};
+    AddMockInboundData(fake_data);
+
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
+  }
+
+  // Recipient: OTHER
+  // Case 1: Invalid index (0xFF).
+  {
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
+    params->recipient = UsbControlTransferRecipient::OTHER;
+    params->request = 5;
+    params->value = 6;
+    params->index = 0xFF;  // Does not exist
+
+    std::vector<uint8_t> fake_data = {1, 2, 3};
+    AddMockInboundData(fake_data);
+
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
+  }
+
+  // Recipient: OTHER
+  // Case 2: Valid index pointing to non-protected interface (0).
+  {
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
+    params->recipient = UsbControlTransferRecipient::OTHER;
+    params->request = 5;
+    params->value = 6;
+    params->index = 0;  // Valid interface 0 (non-protected)
+
+    std::vector<uint8_t> fake_data = {1, 2, 3};
+    AddMockInboundData(fake_data);
+
+    base::test::TestFuture<mojom::UsbTransferStatus, base::span<const uint8_t>>
+        transfer_future;
+    device->ControlTransferIn(std::move(params),
+                              static_cast<uint32_t>(fake_data.size()), 0,
+                              transfer_future.GetCallback());
+    EXPECT_EQ(mojom::UsbTransferStatus::PERMISSION_DENIED,
+              transfer_future.Get<0>());
+    EXPECT_TRUE(transfer_future.Get<1>().empty());
   }
 
   EXPECT_CALL(mock_handle(), Close());
Loading diff…

Original Bug Report

reported by [email protected]

Incomplete fix of crbug **520743499** (fix `872fabf8796f3`, landed 2026-06-15).

Steps to reproduce the problem

There are two repro paths. Path A (unit test) is deterministic and needs no hardware — it is the recommended proof. Path B is the real-world web attack on physical hardware.


Add the test below to services/device/usb/mojo/device_impl_unittest.cc. It mirrors the harness the fix itself uses (the fix only tested index = 0xFF).

Device shape: a composite device whose active configuration has

  • interface 0 = class 0xFF (vendor-specific — NOT on the protected blocklist)
  • interface 1 = class 0x03 (HID — protected/blocklisted)

Call:

// CLASS request, recipient = DEVICE, wIndex low byte = 0 (valid, non-protected interface 0)
bool allowed = device_impl_->HasControlTransferPermission(
    UsbTransferDirection::INBOUND,
    UsbControlTransferType::CLASS,
    UsbControlTransferRecipient::DEVICE,
    /*request=*/0x01,        // e.g. HID GET_REPORT
    /*value=*/0x0100,
    /*index=*/0x0000);       // resolves to interface 0 (not protected)

Expected (correct) result: false (blocked) — the device has a protected interface and the request to recipient DEVICE cannot be safely routed by wIndex.

Actual result on 151.0.7896.0: true (allowed). FindInterface(config, 0) returns the non-protected interface 0, FindBlockedClass returns nullopt, the explicit INTERFACE/ENDPOINT branch is skipped, and !interface is false so the new HasProtectedInterface guard is skipped — control falls through to return AllowAndLog(kAllowed).

Contrast: the same call with index = 0x00FF (invalid interface) correctly returns false — that is the only case the 872fabf8 fix and its tests cover.


Path B — Live web reproduction (physical composite device)

Preconditions:

  1. A composite USB device whose active configuration exposes BOTH a non-protected interface (e.g. vendor/CDC, interface 0) AND a protected-class interface (e.g. HID, interface 1), and whose firmware services the class request on the control endpoint without strictly validating that wIndex names interface 1 (common — e.g. WinUSB forces wIndex to the interface number; many HID stacks act on SET_REPORT/GET_REPORT regardless of the wIndex interface field).
  2. The user grants the origin access to this device once (normal WebUSB grant).

Page script:

const dev = (await navigator.usb.getDevices())[0]
         || await navigator.usb.requestDevice({ filters: [] });
await dev.open();
await dev.selectConfiguration(1);
// NOTE: we do NOT claimInterface() the protected interface — a DEVICE-recipient
// control transfer requires no interface claim in Blink.

// CLASS request to recipient 'device', wIndex naming the NON-protected interface 0.
const res = await dev.controlTransferIn({
  requestType: 'class',
  recipient:   'device',
  request:     0x01,      // e.g. HID GET_REPORT
  value:       0x0100,
  index:       0x0000     // low byte = interface 0 (valid, non-protected)
}, 64);
console.log('reached device, status =', res.status);

Expected (correct): the transfer is blocked by the protected-class policy (it would reach a HID interface). Actual on 151.0.7896.0: the transfer is delivered to the device; firmware that ignores/misinterprets wIndex executes it on the protected HID interface.

A control-OUT variant (controlTransferOut, e.g. HID SET_REPORT, request 0x09) lets the page write to the protected interface the same way.

Problem Description

WebUSB enforces a “protected interface class” blocklist so that a web origin which the user has granted access to a USB device still cannot send control transfers that reach sensitive interface classes (HID, mass-storage, smart-card, audio/video). The enforcement point is DeviceImpl::HasControlTransferPermission in the device service.

Commit 872fabf8796f3 (“usb: Fix protected class bypass via crafted wIndex in CLASS requests”, Bug 520743499, landed 2026-06-15) fixed one bypass: a CLASS request to recipient DEVICE/OTHER with an invalid wIndex (e.g. 0xFF) on a device that ignores wIndex. The fix added if (!interface && HasProtectedInterface(config)) block;.

That guard is gated on !interface — it only fires when wIndex resolves to no interface. But the fix’s own threat model is a device that ignores wIndex, which ignores a valid wIndex just the same. So a CLASS request to recipient DEVICE with wIndex naming a valid non-protected interface (e.g. 0) on a composite device that also exposes a protected interface skips the guard and is allowed, reaching the protected interface. The protections are enabled by default; DEVICE/OTHER recipients require no interface claim. This is an incomplete fix with a distinct trigger (valid wIndex vs the fixed 0xFF case) and no test coverage — a fresh, non-duplicate bypass of the same security boundary.

Summary

Incomplete fix of crbug 520743499 (fix 872fabf8796f3, landed 2026-06-15).

Additional Data

Category: Security
Chrome Channel: Canary
Regression: N/A \

View on issue tracker