From 872fabf8796f3be9f54534158bf40a23732cfd7e Mon Sep 17 00:00:00 2001
From: Alvin Ji <[email protected]>
Date: Mon, 15 Jun 2026 11:07:42 -0700
Subject: [PATCH] usb: Fix protected class bypass via crafted wIndex in CLASS requests

For CLASS requests targeting DEVICE or OTHER recipients, if the target
interface cannot be identified from the wIndex field (e.g. because it
specifies an invalid interface number like 0xFF), and the device has at
least one protected interface in its active configuration, block the
request.

This prevents websites from bypassing the protected class blocklist by
sending class requests with an invalid wIndex to devices that ignore the
wIndex field and execute the request on a protected interface anyway.

Bug: 520743499
Change-Id: Ie54ffa14fdc248ae6d591bcef6ea61b08c7e0b0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7913487
Reviewed-by: Reilly Grant <[email protected]>
Commit-Queue: Alvin Ji <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1646947}
---

diff --git a/services/device/usb/mojo/device_impl.cc b/services/device/usb/mojo/device_impl.cc
index 5960f5b..b5df00a 100644
--- a/services/device/usb/mojo/device_impl.cc
+++ b/services/device/usb/mojo/device_impl.cc
@@ -199,6 +199,50 @@
   device_handle_ = nullptr;
 }
 
+const mojom::UsbInterfaceInfo* DeviceImpl::FindInterface(
+    const mojom::UsbConfigurationInfo* config,
+    uint8_t interface_number) const {
+  auto it = std::ranges::find(config->interfaces, interface_number,
+                              &mojom::UsbInterfaceInfo::interface_number);
+  return it == config->interfaces.end() ? nullptr : it->get();
+}
+
+std::optional<uint8_t> DeviceImpl::FindBlockedClass(
+    const mojom::UsbInterfaceInfo* interface) const {
+  if (!base::FeatureList::IsEnabled(
+          features::kWebUsbProtectedClassControlTransferBlock)) {
+    return std::nullopt;
+  }
+  for (const auto& alternate : interface->alternates) {
+    if (blocked_interface_classes_.contains(alternate->class_code)) {
+      return alternate->class_code;
+    }
+  }
+  return std::nullopt;
+}
+
+bool DeviceImpl::HasProtectedInterface(
+    const mojom::UsbConfigurationInfo* config) const {
+  for (const auto& interface : config->interfaces) {
+    if (FindBlockedClass(interface.get())) {
+      return true;
+    }
+  }
+  return false;
+}
+
+bool DeviceImpl::AllowAndLog(WebUsbControlTransferPermissionOutcome outcome) {
+  base::UmaHistogramEnumeration("WebUsb.ControlTransferPermissionOutcome",
+                                outcome);
+  return true;
+}
+
+bool DeviceImpl::BlockAndLog(WebUsbControlTransferPermissionOutcome outcome) {
+  base::UmaHistogramEnumeration("WebUsb.ControlTransferPermissionOutcome",
+                                outcome);
+  return false;
+}
+
 bool DeviceImpl::HasControlTransferPermission(
     UsbTransferDirection direction,
     UsbControlTransferType type,
@@ -207,6 +251,15 @@
     uint16_t index) {
   DCHECK(device_handle_);
 
+  const mojom::UsbConfigurationInfo* config = device_->GetActiveConfiguration();
+  if (!config) {
+    return BlockAndLog(
+        WebUsbControlTransferPermissionOutcome::kError_NoConfiguration);
+  }
+
+  // ==========================================
+  // 1. STANDARD Requests
+  // ==========================================
   if (type == UsbControlTransferType::STANDARD) {
     if (base::FeatureList::IsEnabled(
             features::kWebUsbEnforceStandardRequestAllowlist)) {
@@ -221,102 +274,119 @@
            request == kUsbRequestGetConfiguration ||
            request == kUsbRequestGetInterface ||
            request == kUsbRequestSynchFrame)) {
-        base::UmaHistogramEnumeration(
-            "WebUsb.ControlTransferPermissionOutcome",
-            WebUsbControlTransferPermissionOutcome::kAllowed);
-        return true;
-      } else {
-        base::UmaHistogramEnumeration(
-            "WebUsb.ControlTransferPermissionOutcome",
-            WebUsbControlTransferPermissionOutcome::kBlocked);
-        return false;
+        return AllowAndLog(WebUsbControlTransferPermissionOutcome::kAllowed);
       }
+      return BlockAndLog(WebUsbControlTransferPermissionOutcome::kBlocked);
+    }
+
+    // Legacy fallback behavior.
+    if (recipient == UsbControlTransferRecipient::DEVICE ||
+        recipient == UsbControlTransferRecipient::OTHER) {
+      return AllowAndLog(WebUsbControlTransferPermissionOutcome::kAllowed);
+    }
+
+    // Fall through case: allowlist is disabled, and recipient is
+    // INTERFACE/ENDPOINT. We must validate the interface.
+    const mojom::UsbInterfaceInfo* interface = nullptr;
+    if (recipient == UsbControlTransferRecipient::ENDPOINT) {
+      interface = device_handle_->FindInterfaceByEndpoint(index & 0xff);
+    } else if (recipient == UsbControlTransferRecipient::INTERFACE) {
+      interface = FindInterface(config, index & 0xff);
+    }
+
+    if (interface) {
+      auto blocked_class = FindBlockedClass(interface);
+      if (blocked_class) {
+        LogBlockedControlTransfer(*blocked_class, direction, type);
+        return BlockAndLog(WebUsbControlTransferPermissionOutcome::kBlocked);
+      }
+      return AllowAndLog(WebUsbControlTransferPermissionOutcome::kAllowed);
+    }
+
+    return BlockAndLog(
+        WebUsbControlTransferPermissionOutcome::kError_InterfaceNotFound);
+  }
+
+  // ==========================================
+  // 2. CLASS Requests
+  // ==========================================
+  if (type == UsbControlTransferType::CLASS) {
+    const mojom::UsbInterfaceInfo* interface = nullptr;
+    if (recipient == UsbControlTransferRecipient::ENDPOINT) {
+      interface = device_handle_->FindInterfaceByEndpoint(index & 0xff);
     } else {
-      // Legacy fallback behavior.
-      if (recipient == UsbControlTransferRecipient::DEVICE ||
-          recipient == UsbControlTransferRecipient::OTHER) {
-        base::UmaHistogramEnumeration(
-            "WebUsb.ControlTransferPermissionOutcome",
-            WebUsbControlTransferPermissionOutcome::kAllowed);
-        return true;
+      // For CLASS requests, we assume index identifies the interface for all
+      // other recipients (INTERFACE, DEVICE, OTHER).
+      interface = FindInterface(config, index & 0xff);
+    }
+
+    // Block if the targeted interface is protected.
+    if (interface) {
+      auto blocked_class = FindBlockedClass(interface);
+      if (blocked_class) {
+        LogBlockedControlTransfer(*blocked_class, direction, type);
+        return BlockAndLog(WebUsbControlTransferPermissionOutcome::kBlocked);
       }
     }
-  }
 
-  const mojom::UsbConfigurationInfo* config = device_->GetActiveConfiguration();
-  if (!config) {
-    base::UmaHistogramEnumeration(
-        "WebUsb.ControlTransferPermissionOutcome",
-        WebUsbControlTransferPermissionOutcome::kError_NoConfiguration);
-    return false;
-  }
-
-  // Identify the interface targeted by this request.
-  const mojom::UsbInterfaceInfo* interface = nullptr;
-  if (recipient == UsbControlTransferRecipient::ENDPOINT) {
-    // For the ENDPOINT recipient, the low byte of `index` is the endpoint
-    // address. We look up the interface that owns this endpoint.
-    interface = device_handle_->FindInterfaceByEndpoint(index & 0xff);
-  } else if (recipient == UsbControlTransferRecipient::INTERFACE ||
-             type == UsbControlTransferType::CLASS) {
-    // For the INTERFACE recipient, `index` identifies the target interface.
-    // For CLASS requests to DEVICE/OTHER recipients, `index` is often used to
-    // identify an interface as defined in class-specific specs (e.g. HID, Mass
-    // Storage, Audio). For VENDOR requests, `index` is manufacturer-defined and
-    // highly variable (e.g. AOA string index), so it is not treated as an
-    // interface ID.
-    auto interface_it =
-        std::ranges::find(config->interfaces, index & 0xff,
-                          &mojom::UsbInterfaceInfo::interface_number);
-    if (interface_it != config->interfaces.end()) {
-      interface = interface_it->get();
+    // For requests explicitly targeting an INTERFACE or ENDPOINT, the interface
+    // must actually exist in the current configuration.
+    if (recipient == UsbControlTransferRecipient::INTERFACE ||

diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc
index 8cd82e8b..7df57bd 100644
--- a/services/device/usb/mojo/device_impl_unittest.cc
+++ b/services/device/usb/mojo/device_impl_unittest.cc
@@ -1482,6 +1482,124 @@
   EXPECT_CALL(mock_handle(), Close());
 }
 
+TEST_F(USBDeviceImplTest,
+       ClassControlTransferToDeviceWithProtectedInterfaceBypass) {
+  // Block interface class 2.
+  mojo::Remote<mojom::UsbDevice> device =
+      GetMockDeviceProxyWithBlockedInterfaces(base::span_from_ref(uint8_t{2}));
+
+  EXPECT_CALL(mock_device(), OpenInternal(_));
+
+  {
+    base::test::TestFuture<mojom::UsbOpenDeviceResultPtr> future;
+    device->Open(future.GetCallback());
+    EXPECT_TRUE(future.Get()->is_success());
+  }
+
+  // Interface 1 has class 2 (blocked).
+  AddMockConfig(ConfigBuilder(/*configuration_value=*/1)
+                    .AddInterface(/*interface_number=*/1,
+                                  /*alternate_setting=*/0,
+                                  /*class_code=*/2, /*subclass_code=*/0,
+                                  /*protocol_code=*/0)
+                    .Build());
+
+  EXPECT_CALL(mock_handle(), SetConfigurationInternal(1, _));
+
+  {
+    base::test::TestFuture<bool> future;
+    device->SetConfiguration(1, future.GetCallback());
+    EXPECT_TRUE(future.Get());
+  }
+
+  {
+    // 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;
+    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::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();
+  }
+
+  EXPECT_CALL(mock_handle(), Close());
+}
+
+TEST_F(USBDeviceImplTest,
+       ClassControlTransferToDeviceWithoutProtectedInterface) {
+  // Block interface class 2 (but device won't have it).
+  mojo::Remote<mojom::UsbDevice> device =
+      GetMockDeviceProxyWithBlockedInterfaces(base::span_from_ref(uint8_t{2}));
+
+  EXPECT_CALL(mock_device(), OpenInternal(_));
+
+  {
+    base::test::TestFuture<mojom::UsbOpenDeviceResultPtr> future;
+    device->Open(future.GetCallback());
+    EXPECT_TRUE(future.Get()->is_success());
+  }
+
+  // Interface 1 has class 3 (NOT blocked).
+  AddMockConfig(ConfigBuilder(/*configuration_value=*/1)
+                    .AddInterface(/*interface_number=*/1,
+                                  /*alternate_setting=*/0,
+                                  /*class_code=*/3, /*subclass_code=*/0,
+                                  /*protocol_code=*/0)
+                    .Build());
+
+  EXPECT_CALL(mock_handle(), SetConfigurationInternal(1, _));
+
+  {
+    base::test::TestFuture<bool> future;
+    device->SetConfiguration(1, future.GetCallback());
+    EXPECT_TRUE(future.Get());
+  }
+
+  {
+    // A CLASS request to the DEVICE with index 0xFF (not matching any
+    // interface) should be ALLOWED because the device has no protected
+    // interfaces.
+    std::vector<uint8_t> fake_data = {1, 2, 3};
+    AddMockInboundData(fake_data);
+
+    EXPECT_CALL(mock_handle(),
+                ControlTransferInternal(UsbTransferDirection::INBOUND,
+                                        UsbControlTransferType::CLASS,
+                                        UsbControlTransferRecipient::DEVICE, 5,
+                                        6, 0xFF, _, 0, _));
+
+    auto params = mojom::UsbControlTransferParams::New();
+    params->type = UsbControlTransferType::CLASS;
+    params->recipient = UsbControlTransferRecipient::DEVICE;
+    params->request = 5;
+    params->value = 6;
+    params->index = 0xFF;  // Does not exist
+
+    base::RunLoop loop;
+    device->ControlTransferIn(
+        std::move(params), static_cast<uint32_t>(fake_data.size()), 0,
+        base::BindOnce(&ExpectTransferInAndThen,
+                       mojom::UsbTransferStatus::COMPLETED, fake_data,
+                       loop.QuitClosure()));
+    loop.Run();
+  }
+
+  EXPECT_CALL(mock_handle(), Close());
+}
+
 TEST_F(USBDeviceImplTest, ControlTransferProtectedClassBlockDisabled) {
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndDisableFeature(