CVE-2026-79003
Overview
Background
- HID collections
- a HID report descriptor defines a tree of collections. Top-level collections (TLCs) can contain child collections, each with a usage page and usage (for example Generic Desktop/Keyboard or FIDO) and the input, output and feature reports defined inside it.
- WebHID protected reports
- Chrome hides reports belonging to sensitive usages: “always protected” usages such as keyboards (
IsAlwaysProtected), FIDO security keys (unlessIsFidoAllowedForOrigin), and blocklist entries inHidBlocklist. Their IDs are recorded inprotected_input_report_ids,protected_output_report_idsandprotected_feature_report_ids. - `HidService::RemoveProtectedReports`
- the browser-side filter that removes protected reports from a
HidDeviceInfobefore it is exposed to a page, and drops collections left with no reports. - `kWebHidRecursiveFiltering`
- the new feature flag, enabled by default, that gates the recursive logic. The old flat logic is kept as a fallback, which acts as a kill switch.
Root Cause Analysis
All of WebHID’s report-protection decisions looked only at top-level collections. In RemoveProtectedReports, is_fido was computed from collection->usage->usage_page of the TLC. CheckBlocklistEntry in hid_blocklist.cc matched usage-based rules only against each TLC’s usage. In hid_connection.cc, the connection-side checks used FindCollectionWithReport(...)->usage with IsAlwaysProtected and a FIDO usage-page comparison on the collection returned, without looking at its descendants. A device that nested a Keyboard or FIDO collection inside a benign TLC (the tests use a vendor TLC) therefore had its sensitive reports judged by the benign parent’s usage and exposed. This violated the invariant that a report is protected if any collection containing it has a protected usage. In addition, HidService::FinishRequestDevice returned the chooser’s devices to the page without filtering.
The fix adds FilterCollectionReports, which recurses into children, decides FIDO and always-protected status per report against the original (unfiltered) collection subtree through HasReportInCollectionWithUsagePage and HasReportInAlwaysProtectedCollection, and prunes empty children. It also makes CheckBlocklistEntry recurse into collection->children, switches the connection-side checks to the same subtree-aware helpers, and runs RemoveProtectedReports in FinishRequestDevice, dropping devices with no remaining collections.
Attack Path
- Nested descriptor
a HID device presents a report descriptor in which a Keyboard or FIDO collection is nested inside a benign top-level collection. The tests use
VendorWithNestedKeyboard()andVendorWithNestedFido(), with akPageVendorTLC containing akGenericDesktopKeyboardorkPageFidochild. - User grants access
a web page calls
navigator.hid.requestDevice()and the user selects the device. Before the patch,FinishRequestDevicereturned the device info unfiltered, andRemoveProtectedReportsevaluated only the vendor TLC, so the nested reports stayed visible. - Connection-side checks pass
when the page opens the device and sends or receives reports, the
hid_connection.ccprotection checks consult the TLC usage returned byFindCollectionWithReport, which is vendor-defined and not always protected, so the nested keyboard or FIDO report IDs are allowed. - Sensitive I/O
the page can receive keyboard input reports or exchange FIDO reports with the security key, bypassing the protections WebHID applies to such devices. Concrete keystroke capture or a CTAP misuse scenario is
(inferred).
Impact Assessment
(inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
forcontent/browser/hid/hid_service.cc |
modified | |
ifcontent/browser/hid/hid_service.cc |
modified | |
ifservices/device/hid/hid_connection.cc |
modified | |
ifservices/device/public/cpp/hid/hid_blocklist.cc |
modified | |
forservices/device/public/cpp/hid/hid_blocklist.cc |
modified |
Files Changed
content/browser/BUILD.gncontent/browser/hid/hid_service.cccontent/browser/hid/hid_service_unittest.ccservices/device/hid/hid_connection.ccservices/device/hid/hid_connection_unittest.ccservices/device/hid/hid_device_info_unittest.ccservices/device/public/cpp/device_features.ccservices/device/public/cpp/device_features.hservices/device/public/cpp/hid/hid_blocklist.ccservices/device/public/cpp/hid/hid_blocklist_unittest.ccservices/device/public/cpp/hid/hid_report_utils.ccservices/device/public/cpp/hid/hid_report_utils.hservices/device/public/cpp/test/test_report_descriptors.ccservices/device/public/cpp/test/test_report_descriptors.h
Audit Directions
- Top-level-only reasoning over tree-structured descriptorssearch for other users of
collectionsorusage->usage_pagethat ignorechildren, such as WebHID permission auto-grant,IsKnownSecurityKey, chooser display or usage filters inrequestDevice({filters}), policy allowlists keyed on usage, and the platform-specificHidServiceWin/Mac/Linuxreport-ID mapping. - Precedence between the FIDO exemption and other protectionsin
FilterCollectionReports,(is_fido && is_fido_allowed)short-circuits the always-protected and blocklist terms. Check whether a keyboard or blocklisted report nested under a FIDO collection, or sharing its report ID, can be exposed to a FIDO-privileged origin. - Feature-flag fallback and shared report IDsthe old flat path remains when
kWebHidRecursiveFilteringis disabled. Also verify that devices reusing the same report ID across sibling collections (report IDs are device-wide) cannot have a protected ID re-enabled by a benign sibling.
Patch
diff --git a/content/browser/hid/hid_service.cc b/content/browser/hid/hid_service.cc
index 9b5e15f..fe153f8 100644
--- a/content/browser/hid/hid_service.cc
+++ b/content/browser/hid/hid_service.cc
@@ -25,6 +25,7 @@
#include "mojo/public/cpp/bindings/message.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "services/device/public/cpp/device_features.h"
+#include "services/device/public/cpp/hid/hid_report_utils.h"
#include "services/network/public/mojom/permissions_policy/permissions_policy_feature.mojom.h"
namespace content {
@@ -206,6 +207,100 @@
std::move(receiver));
}
+namespace {
+
+// Recursively filters reports from the collection tree. Returns true if the
+// collection is now empty (has no reports and no active children) and should
+// be pruned by its parent.
+bool FilterCollectionReports(
+ device::mojom::HidCollectionInfo& collection,
+ const device::mojom::HidCollectionInfo& original_collection,
+ const std::optional<std::vector<uint8_t>>& protected_input_report_ids,
+ const std::optional<std::vector<uint8_t>>& protected_output_report_ids,
+ const std::optional<std::vector<uint8_t>>& protected_feature_report_ids,
+ bool is_fido_allowed) {
+ // Recursively filter children first.
+ std::vector<device::mojom::HidCollectionInfoPtr> children;
+ CHECK_EQ(collection.children.size(), original_collection.children.size());
+ for (size_t i = 0; i < collection.children.size(); ++i) {
+ if (!FilterCollectionReports(
+ *collection.children[i], *original_collection.children[i],
+ protected_input_report_ids, protected_output_report_ids,
+ protected_feature_report_ids, is_fido_allowed)) {
+ children.push_back(std::move(collection.children[i]));
+ }
+ }
+ collection.children = std::move(children);
+
+ // Filter input reports.
+ std::vector<device::mojom::HidReportDescriptionPtr> input_reports;
+ for (auto& report : collection.input_reports) {
+ const bool is_fido = device::HasReportInCollectionWithUsagePage(
+ original_collection, report->report_id, device::HidReportType::kInput,
+ device::mojom::kPageFido);
+ const bool is_always_protected =
+ device::HasReportInAlwaysProtectedCollection(
+ original_collection, report->report_id,
+ device::HidReportType::kInput);
+ if ((is_fido && is_fido_allowed) ||
+ (!is_always_protected &&
+ (!protected_input_report_ids.has_value() ||
+ !std::ranges::contains(*protected_input_report_ids,
+ report->report_id)))) {
+ input_reports.push_back(std::move(report));
+ }
+ }
+ collection.input_reports = std::move(input_reports);
+
+ // Filter output reports.
+ std::vector<device::mojom::HidReportDescriptionPtr> output_reports;
+ for (auto& report : collection.output_reports) {
+ const bool is_fido = device::HasReportInCollectionWithUsagePage(
+ original_collection, report->report_id, device::HidReportType::kOutput,
+ device::mojom::kPageFido);
+ const bool is_always_protected =
+ device::HasReportInAlwaysProtectedCollection(
+ original_collection, report->report_id,
+ device::HidReportType::kOutput);
+ if ((is_fido && is_fido_allowed) ||
+ (!is_always_protected &&
+ (!protected_output_report_ids.has_value() ||
+ !std::ranges::contains(*protected_output_report_ids,
+ report->report_id)))) {
+ output_reports.push_back(std::move(report));
+ }
+ }
+ collection.output_reports = std::move(output_reports);
+
+ // Filter feature reports.
+ std::vector<device::mojom::HidReportDescriptionPtr> feature_reports;
+ for (auto& report : collection.feature_reports) {
+ const bool is_fido = device::HasReportInCollectionWithUsagePage(
+ original_collection, report->report_id, device::HidReportType::kFeature,
+ device::mojom::kPageFido);
+ const bool is_always_protected =
+ device::HasReportInAlwaysProtectedCollection(
+ original_collection, report->report_id,
+ device::HidReportType::kFeature);
+ if ((is_fido && is_fido_allowed) ||
+ (!is_always_protected &&
+ (!protected_feature_report_ids.has_value() ||
+ !std::ranges::contains(*protected_feature_report_ids,
+ report->report_id)))) {
+ feature_reports.push_back(std::move(report));
+ }
+ }
+ collection.feature_reports = std::move(feature_reports);
+
+ // Return true if this collection is now empty and should be pruned by its
+ // parent.
+ return collection.input_reports.empty() &&
+ collection.output_reports.empty() &&
+ collection.feature_reports.empty() && collection.children.empty();
+}
+
+} // namespace
+
// static
void HidService::RemoveProtectedReports(device::mojom::HidDeviceInfo& device,
bool is_known_security_key,
@@ -219,44 +314,60 @@
return;
}
#endif // !BUILDFLAG(IS_ANDROID)
+
std::vector<device::mojom::HidCollectionInfoPtr> collections;
- for (auto& collection : device.collections) {
- const bool is_fido =
- collection->usage->usage_page == device::mojom::kPageFido;
- std::vector<device::mojom::HidReportDescriptionPtr> input_reports;
- for (auto& report : collection->input_reports) {
- if ((is_fido && is_fido_allowed) ||
- !device.protected_input_report_ids.has_value() ||
- !std::ranges::contains(*device.protected_input_report_ids,
- report->report_id)) {
- input_reports.push_back(std::move(report));
+ if (base::FeatureList::IsEnabled(features::kWebHidRecursiveFiltering)) {
+ // Clone to preserve original state for queries.
+ auto original_device = device.Clone();
+ for (size_t i = 0; i < device.collections.size(); ++i) {
+ if (!FilterCollectionReports(
+ *device.collections[i], *original_device->collections[i],
+ original_device->protected_input_report_ids,
+ original_device->protected_output_report_ids,
+ original_device->protected_feature_report_ids, is_fido_allowed)) {
+ collections.push_back(std::move(device.collections[i]));
}
}
- std::vector<device::mojom::HidReportDescriptionPtr> output_reports;
- for (auto& report : collection->output_reports) {
- if ((is_fido && is_fido_allowed) ||
- !device.protected_output_report_ids.has_value() ||
- !std::ranges::contains(*device.protected_output_report_ids,
- report->report_id)) {
- output_reports.push_back(std::move(report));
+ } else {
+ // Fall back to old flat filtering.
+ for (auto& collection : device.collections) {
+ const bool is_fido =
+ collection->usage->usage_page == device::mojom::kPageFido;
+ std::vector<device::mojom::HidReportDescriptionPtr> input_reports;
+ for (auto& report : collection->input_reports) {
+ if ((is_fido && is_fido_allowed) ||
+ !device.protected_input_report_ids.has_value() ||
+ !std::ranges::contains(*device.protected_input_report_ids,
+ report->report_id)) {
+ input_reports.push_back(std::move(report));
+ }
}
- }
- std::vector<device::mojom::HidReportDescriptionPtr> feature_reports;
- for (auto& report : collection->feature_reports) {
- if ((is_fido && is_fido_allowed) ||
- !device.protected_feature_report_ids.has_value() ||
- !std::ranges::contains(*device.protected_feature_report_ids,
- report->report_id)) {
- feature_reports.push_back(std::move(report));
+ std::vector<device::mojom::HidReportDescriptionPtr> output_reports;
+ for (auto& report : collection->output_reports) {
+ if ((is_fido && is_fido_allowed) ||
+ !device.protected_output_report_ids.has_value() ||
+ !std::ranges::contains(*device.protected_output_report_ids,
+ report->report_id)) {
+ output_reports.push_back(std::move(report));
+ }
}
- }
- // Only keep the collection if it has at least one report.
- if (!input_reports.empty() || !output_reports.empty() ||
- !feature_reports.empty()) {
- collection->input_reports = std::move(input_reports);
- collection->output_reports = std::move(output_reports);
- collection->feature_reports = std::move(feature_reports);
- collections.push_back(std::move(collection));
+ std::vector<device::mojom::HidReportDescriptionPtr> feature_reports;
+ for (auto& report : collection->feature_reports) {
+ if ((is_fido && is_fido_allowed) ||
+ !device.protected_feature_report_ids.has_value() ||
+ !std::ranges::contains(*device.protected_feature_report_ids,
+ report->report_id)) {
+ feature_reports.push_back(std::move(report));
+ }
+ }
+ // Only keep the collection if it has at least one report.
+ if (!input_reports.empty() || !output_reports.empty() ||
+ !feature_reports.empty()) {
+ collection->input_reports = std::move(input_reports);
+ collection->output_reports = std::move(output_reports);
+ collection->feature_reports = std::move(feature_reports);
+ collections.push_back(std::move(collection));
+ }
}
}
device.collections = std::move(collections);
@@ -577,7 +688,25 @@
void HidService::FinishRequestDevice(
RequestDeviceCallback callback,
std::vector<device::mojom::HidDeviceInfoPtr> devices) {
- std::move(callback).Run(std::move(devices));
+ auto* browser_context = GetBrowserContext();
+ auto* delegate = GetContentClient()->browser()->GetHidDelegate();
+ if (!browser_context || !delegate) {
+ std::move(callback).Run(std::vector<device::mojom::HidDeviceInfoPtr>());
+ return;
+ }
+
+ bool is_fido_allowed =
+ delegate->IsFidoAllowedForOrigin(browser_context, origin_);
+ std::vector<device::mojom::HidDeviceInfoPtr> result;
+ for (auto& device : devices) {
+ RemoveProtectedReports(
+ *device, delegate->IsKnownSecurityKey(browser_context, *device),
+ is_fido_allowed);
+ if (!device->collections.empty()) {
+ result.push_back(std::move(device));
+ }
+ }
+ std::move(callback).Run(std::move(result));
}
void HidService::FinishConnect(
diff --git a/services/device/hid/hid_connection.cc b/services/device/hid/hid_connection.cc
index 6a8afc9..7fa17b8f 100644
--- a/services/device/hid/hid_connection.cc
+++ b/services/device/hid/hid_connection.cc
@@ -8,6 +8,7 @@
#include "base/memory/ref_counted_memory.h"
#include "components/device_event_log/device_event_log.h"
+#include "services/device/public/cpp/device_features.h"
#include "services/device/public/cpp/hid/hid_blocklist.h"
#include "services/device/public/cpp/hid/hid_report_type.h"
#include "services/device/public/cpp/hid/hid_report_utils.h"
@@ -169,9 +170,15 @@
}
auto* collection_info =
FindCollectionWithReport(device, report_id, report_type);
- if (collection_info &&
- collection_info->usage->usage_page == mojom::kPageFido) {
- return false;
+ if (collection_info) {
+ if (base::FeatureList::IsEnabled(features::kWebHidRecursiveFiltering)) {
+ if (HasReportInCollectionWithUsagePage(
+ *collection_info, report_id, report_type, mojom::kPageFido)) {
+ return false;
+ }
+ } else if (collection_info->usage->usage_page == mojom::kPageFido) {
+ return false;
+ }
}
}
@@ -202,7 +209,12 @@
auto* collection_info =
FindCollectionWithReport(device, report_id, report_type);
if (collection_info) {
- return IsAlwaysProtected(*collection_info->usage, report_type);
+ if (base::FeatureList::IsEnabled(features::kWebHidRecursiveFiltering)) {
+ return HasReportInAlwaysProtectedCollection(*collection_info, report_id,
+ report_type);
+ } else {
+ return IsAlwaysProtected(*collection_info->usage, report_type);
+ }
}
return HasAlwaysProtectedCollectionFor(report_type);
diff --git a/services/device/public/cpp/device_features.cc b/services/device/public/cpp/device_features.cc
index 938c6dd..8ab64b7 100644
--- a/services/device/public/cpp/device_features.cc
+++ b/services/device/public/cpp/device_features.cc
@@ -127,6 +127,10 @@
base::FEATURE_ENABLED_BY_DEFAULT);
#endif // !BUILDFLAG(IS_ANDROID)
+// Enables recursive filtering of nested HID collections to prevent WebHID
+// security bypasses (e.g., nested keyboards or FIDO keys).
+BASE_FEATURE(kWebHidRecursiveFiltering, base::FEATURE_ENABLED_BY_DEFAULT);
+
const base::FeatureParam<device::mojom::LocationProviderManagerMode>::Option
location_provider_manager_mode_options[] = {
{device::mojom::LocationProviderManagerMode::kNetworkOnly,
diff --git a/services/device/public/cpp/device_features.h b/services/device/public/cpp/device_features.h
index ff95d09..96e36de7 100644
--- a/services/device/public/cpp/device_features.h
+++ b/services/device/public/cpp/device_features.h
@@ -57,6 +57,8 @@
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kSecurityKeyHidInterfacesAreFido);
#endif // !BUILDFLAG(IS_ANDROID)
+DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kWebHidRecursiveFiltering);
+
extern const DEVICE_FEATURES_EXPORT
base::FeatureParam<device::mojom::LocationProviderManagerMode>
kLocationProviderManagerParam;
diff --git a/services/device/public/cpp/hid/hid_blocklist.cc b/services/device/public/cpp/hid/hid_blocklist.cc
index d95caeae..83c3d293 100644
--- a/services/device/public/cpp/hid/hid_blocklist.cc
+++ b/services/device/public/cpp/hid/hid_blocklist.cc
@@ -140,6 +140,20 @@
}
}
+bool MatchesUsage(const HidBlocklist::Entry& entry,
+ const mojom::HidCollectionInfo& collection) {
+ if (!entry.has_usage_page) {
+ return true;
+ }
+ if (entry.usage_page != collection.usage->usage_page) {
+ return false;
+ }
+ if (!entry.has_usage) {
+ return true;
+ }
+ return entry.usage == collection.usage->usage;
+}
+
// Iterates over |collections| to find reports of type |report_type| that should
// be protected according to the blocklist rule |entry|. |vendor_id| and
// |product_id| are the vendor and product IDs of the device with these reports.
@@ -166,18 +180,21 @@
}
for (const auto& collection : collections) {
- if (entry.has_usage_page) {
- if (entry.usage_page != collection->usage->usage_page)
- continue;
-
- if (entry.has_usage && entry.usage != collection->usage->usage)
- continue;
+ if (MatchesUsage(entry, *collection)) {
+ const auto& reports = GetReportsForType(report_type, *collection);
+ for (const auto& report : reports) {
+ if (!entry.has_report_id || entry.report_id == report->report_id) {
+ protected_ids.insert(report->report_id);
+ }
+ }
}
-
- const auto& reports = GetReportsForType(report_type, *collection);
- for (const auto& report : reports) {
- if (!entry.has_report_id || entry.report_id == report->report_id)
- protected_ids.insert(report->report_id);
+ // A report defined in a child collection is also part of the enclosing
+ // top-level collection. Evaluate usage-based rules against child
+ // collections as well so reports defined in protected child collections
+ // are not missed.
+ if (base::FeatureList::IsEnabled(features::kWebHidRecursiveFiltering)) {
+ CheckBlocklistEntry(entry, report_type, vendor_id, product_id,
+ collection->children, protected_ids);
}
}
}
diff --git a/services/device/public/cpp/hid/hid_report_utils.cc b/services/device/public/cpp/hid/hid_report_utils.cc
index fcb588a..b153a35 100644
--- a/services/device/public/cpp/hid/hid_report_utils.cc
+++ b/services/device/public/cpp/hid/hid_report_utils.cc
@@ -23,6 +23,14 @@
}
}
+bool CollectionContainsReport(const mojom::HidCollectionInfo& collection,
+ uint8_t report_id,
+ HidReportType report_type) {
+ return std::ranges::contains(ReportsForType(collection, report_type),
+ report_id,
+ &mojom::HidReportDescription::report_id);
+}
+
} // namespace
bool IsAlwaysProtected(const mojom::HidUsageAndPage& hid_usage_and_page,
@@ -113,4 +121,41 @@
return find_it->get();
}
+bool HasReportInAlwaysProtectedCollection(
+ const mojom::HidCollectionInfo& collection,
+ uint8_t report_id,
+ HidReportType report_type) {
+ if (!CollectionContainsReport(collection, report_id, report_type)) {
+ return false;
+ }
+ if (collection.collection_type == mojom::kHIDCollectionTypeApplication &&
+ IsAlwaysProtected(*collection.usage, report_type)) {
+ return true;
+ }
+ return std::ranges::any_of(collection.children, [report_id, report_type](
... (file diff truncated)
Regression Test / PoC
diff --git a/content/browser/hid/hid_service_unittest.cc b/content/browser/hid/hid_service_unittest.cc
index a3bcd5b..8c2ba32 100644
--- a/content/browser/hid/hid_service_unittest.cc
+++ b/content/browser/hid/hid_service_unittest.cc
@@ -200,6 +200,18 @@
device::TestReportDescriptors::FidoU2fHid());
}
+ device::mojom::HidDeviceInfoPtr CreateNestedFidoDevice() {
+ return device::CreateDeviceFromReportDescriptor(
+ /*vendor_id=*/0x1234, /*product_id=*/0xabcd,
+ device::TestReportDescriptors::VendorWithNestedFido());
+ }
+
+ device::mojom::HidDeviceInfoPtr CreateNestedKeyboardDevice() {
+ return device::CreateDeviceFromReportDescriptor(
+ /*vendor_id=*/0x1234, /*product_id=*/0xabcd,
+ device::TestReportDescriptors::VendorWithNestedKeyboard());
+ }
+
device::mojom::HidDeviceInfoPtr CreateTitanFidoDevice() {
return device::CreateDeviceFromReportDescriptor(
kVendorGoogle, kProductTitan,
@@ -353,7 +365,8 @@
public:
void SetUp() override {
scoped_feature_list_.InitWithFeatures(
- /*enabled_features=*/{features::kSecurityKeyHidInterfacesAreFido},
+ /*enabled_features=*/{features::kSecurityKeyHidInterfacesAreFido,
+ features::kWebHidRecursiveFiltering},
/*disabled_features=*/{});
}
@@ -1376,6 +1389,147 @@
}
}
+TEST_P(HidServiceFidoTest, NestedFidoDeviceAllowedWithPrivilegedOrigin) {
+ auto service_creation_type = std::get<0>(GetParam());
+ const auto& service = GetService(service_creation_type);
+ const bool is_fido_allowed = std::get<1>(GetParam());
+
+ url::Origin origin = url::Origin::Create(GURL(kTestUrl));
+ EXPECT_CALL(hid_delegate(), IsFidoAllowedForOrigin(_, origin))
+ .WillRepeatedly(Return(is_fido_allowed));
+ EXPECT_CALL(hid_delegate(), HasDevicePermission).WillRepeatedly(Return(true));
+
+ TestFuture<std::vector<device::mojom::HidDeviceInfoPtr>> get_devices_future;
+ service->GetDevices(get_devices_future.GetCallback());
+ EXPECT_TRUE(get_devices_future.Get().empty());
+
+ auto device_info = CreateNestedFidoDevice();
+ ASSERT_EQ(device_info->collections.size(), 1u);
+ EXPECT_EQ(device_info->collections[0]->usage->usage_page,
+ device::mojom::kPageVendor);
+ ASSERT_EQ(device_info->collections[0]->children.size(), 1u);
+ EXPECT_EQ(device_info->collections[0]->children[0]->usage->usage_page,
+ device::mojom::kPageFido);
+
+ TestFuture<device::mojom::HidDeviceInfoPtr> device_added_future;
+ if (is_fido_allowed) {
+ EXPECT_CALL(hid_manager_client(), DeviceAdded)
+ .WillOnce(InvokeFuture(device_added_future));
+ } else {
+ EXPECT_CALL(hid_manager_client(), DeviceAdded).Times(0);
+ }
+ ConnectDevice(*device_info);
+ if (is_fido_allowed) {
+ const auto& d = *device_added_future.Get();
+ ASSERT_EQ(d.collections.size(), 1u);
+ ASSERT_EQ(d.collections[0]->children.size(), 1u);
+ EXPECT_EQ(d.collections[0]->children[0]->input_reports.size(), 1u);
+ EXPECT_EQ(d.collections[0]->children[0]->output_reports.size(), 1u);
+ } else {
+ FlushHidServicePipe(service_);
+ }
+
+ TestFuture<device::mojom::HidDeviceInfoPtr> device_changed_future;
+ EXPECT_CALL(hid_manager_client(), DeviceChanged)
+ .WillOnce(InvokeFuture(device_changed_future));
+
+ auto joystick = device::mojom::HidCollectionInfo::New();
+ joystick->usage = device::mojom::HidUsageAndPage::New(
+ device::mojom::kGenericDesktopJoystick,
+ device::mojom::kPageGenericDesktop);
+ joystick->collection_type = device::mojom::kHIDCollectionTypeApplication;
+ joystick->feature_reports.push_back(
+ device::mojom::HidReportDescription::New());
+
+ auto updated_device_info = device_info.Clone();
+ updated_device_info->collections.push_back(std::move(joystick));
+ UpdateDevice(*updated_device_info);
+ const auto& changed_d = *device_changed_future.Get();
+ if (is_fido_allowed) {
+ ASSERT_EQ(changed_d.collections.size(), 2u);
+ EXPECT_EQ(changed_d.collections[0]->usage->usage_page,
+ device::mojom::kPageVendor);
+ ASSERT_EQ(changed_d.collections[0]->children.size(), 1u);
+ EXPECT_EQ(changed_d.collections[1]->usage->usage_page,
+ device::mojom::kPageGenericDesktop);
+ EXPECT_EQ(changed_d.collections[1]->usage->usage,
+ device::mojom::kGenericDesktopJoystick);
+ } else {
+ ASSERT_EQ(changed_d.collections.size(), 1u);
+ EXPECT_EQ(changed_d.collections[0]->usage->usage_page,
+ device::mojom::kPageGenericDesktop);
+ EXPECT_EQ(changed_d.collections[0]->usage->usage,
+ device::mojom::kGenericDesktopJoystick);
+ }
+
+ TestFuture<device::mojom::HidDeviceInfoPtr> device_removed_future;
+ EXPECT_CALL(hid_manager_client(), DeviceRemoved)
+ .WillOnce(InvokeFuture(device_removed_future));
+ DisconnectDevice(*updated_device_info);
+ const auto& removed_d = *device_removed_future.Get();
+ if (is_fido_allowed) {
+ EXPECT_EQ(removed_d.collections.size(), 2u);
+ } else {
+ EXPECT_EQ(removed_d.collections.size(), 1u);
+ }
+}
+
+TEST_P(HidServiceTest, NestedKeyboardDeviceBlocked) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(features::kWebHidRecursiveFiltering);
+
+ auto service_creation_type = GetParam();
+ GetService(service_creation_type);
+
+ // Set up global expectations for the delegate.
+ EXPECT_CALL(hid_delegate(), HasDevicePermission).WillRepeatedly(Return(true));
+
+ auto device_info = CreateNestedKeyboardDevice();
+ ASSERT_EQ(device_info->collections.size(), 1u);
+ EXPECT_EQ(device_info->collections[0]->usage->usage_page,
+ device::mojom::kPageVendor);
+ ASSERT_EQ(device_info->collections[0]->children.size(), 1u);
+ EXPECT_EQ(device_info->collections[0]->children[0]->usage->usage_page,
+ device::mojom::kPageGenericDesktop);
+ EXPECT_EQ(device_info->collections[0]->children[0]->usage->usage,
+ device::mojom::kGenericDesktopKeyboard);
+
+ EXPECT_CALL(hid_manager_client(), DeviceAdded).Times(0);
+ ConnectDevice(*device_info);
+ FlushHidServicePipe(service_);
+
+ base::RunLoop device_changed_loop;
+ EXPECT_CALL(hid_manager_client(), DeviceChanged).WillOnce([&](auto d) {
+ EXPECT_EQ(d->collections.size(), 1u);
+ EXPECT_EQ(d->collections[0]->usage->usage_page,
+ device::mojom::kPageGenericDesktop);
+ EXPECT_EQ(d->collections[0]->usage->usage,
+ device::mojom::kGenericDesktopJoystick);
+ device_changed_loop.Quit();
+ });
+
+ auto joystick = device::mojom::HidCollectionInfo::New();
+ joystick->usage = device::mojom::HidUsageAndPage::New(
+ device::mojom::kGenericDesktopJoystick,
+ device::mojom::kPageGenericDesktop);
+ joystick->collection_type = device::mojom::kHIDCollectionTypeApplication;
+ joystick->feature_reports.push_back(
+ device::mojom::HidReportDescription::New());
+
+ auto updated_device_info = device_info.Clone();
+ updated_device_info->collections.push_back(std::move(joystick));
+ UpdateDevice(*updated_device_info);
+ device_changed_loop.Run();
+
+ base::RunLoop device_removed_loop;
+ EXPECT_CALL(hid_manager_client(), DeviceRemoved).WillOnce([&](auto d) {
+ EXPECT_EQ(d->collections.size(), 1u);
+ device_removed_loop.Quit();
+ });
+ DisconnectDevice(*updated_device_info);
+ device_removed_loop.Run();
+}
+
INSTANTIATE_TEST_SUITE_P(
HidServiceTests,
HidServiceTest,
diff --git a/services/device/hid/hid_connection_unittest.cc b/services/device/hid/hid_connection_unittest.cc
index 614ee02..f1ebd3d 100644
--- a/services/device/hid/hid_connection_unittest.cc
+++ b/services/device/hid/hid_connection_unittest.cc
@@ -279,7 +279,10 @@
class HidConnectionProtectedReportTest : public testing::Test,
HidConnection::Client {
public:
- HidConnectionProtectedReportTest() = default;
+ HidConnectionProtectedReportTest() {
+ scoped_feature_list_.InitAndEnableFeature(
+ features::kWebHidRecursiveFiltering);
+ }
HidConnectionProtectedReportTest(const HidConnectionProtectedReportTest&) =
delete;
HidConnectionProtectedReportTest& operator=(
@@ -353,6 +356,7 @@
scoped_refptr<TestHidConnection> connection_;
base::test::TestFuture<scoped_refptr<base::RefCountedBytes>, size_t>
input_report_future_;
+ base::test::ScopedFeatureList scoped_feature_list_;
};
TEST_F(HidConnectionProtectedReportTest, UnprotectedReadWrite) {
@@ -599,4 +603,91 @@
EXPECT_TRUE(connection().closed());
}
+TEST_F(HidConnectionProtectedReportTest, FidoReportsInNestedCollectionBlocked) {
+ // Simulate a device with a vendor-defined top-level collection containing a
+ // nested FIDO application collection that defines input and output reports.
+ auto device_info =
+ CreateHidDeviceInfo(TestReportDescriptors::VendorWithNestedFido());
+ ASSERT_TRUE(device_info);
+ ASSERT_EQ(device_info->collections().size(), 1u);
+ EXPECT_EQ(device_info->collections()[0]->usage->usage_page,
+ mojom::kPageVendor);
+ CreateConnection(device_info);
+
+ SetConnectionClient();
+
+ // Simulate an input report from the nested FIDO collection. It should not be
+ // received by the client.
+ auto buffer =
+ base::MakeRefCounted<base::RefCountedBytes>(std::vector<uint8_t>{1});
+ connection().SimulateInputReport(buffer);
+ EXPECT_FALSE(HasNextInputReport());
+
+ // Try to write an output report to the nested FIDO collection. It should be
+ // blocked.
+ TestFuture<bool> write_future;
+ connection().Write(buffer, write_future.GetCallback());
+ EXPECT_FALSE(write_future.Get());
+
+ // Close the connection.
+ connection().Close();
+ EXPECT_TRUE(connection().closed());
+}
+
+TEST_F(HidConnectionProtectedReportTest,
+ AllowFidoReportsAllowsFidoInNestedCollection) {
+ // Simulate a device with a vendor-defined top-level collection containing a
+ // nested FIDO application collection that defines input and output reports.
+ auto device_info =
+ CreateHidDeviceInfo(TestReportDescriptors::VendorWithNestedFido());
+ ASSERT_TRUE(device_info);
+
+ // Simulate a connection from a FIDO-privileged origin.
+ CreateConnection(device_info, /*allow_protected_reports=*/false,
+ /*allow_fido_reports=*/true);
+
+ // Simulate an input report.
+ TestFuture<bool, scoped_refptr<base::RefCountedBytes>, size_t> read_future;
+ auto buffer =
+ base::MakeRefCounted<base::RefCountedBytes>(std::vector<uint8_t>{1});
+ connection().SimulateInputReport(buffer);
+ connection().Read(read_future.GetCallback());
+ EXPECT_TRUE(read_future.Get<0>());
+
+ // Simulate an output report.
+ TestFuture<bool> write_future;
+ connection().Write(buffer, write_future.GetCallback());
+ EXPECT_TRUE(write_future.Get());
+
+ // Close the connection.
+ connection().Close();
+ EXPECT_TRUE(connection().closed());
+}
+
+TEST_F(HidConnectionProtectedReportTest,
+ KeyboardReportsInNestedCollectionBlocked) {
+ // Simulate a device with a vendor-defined top-level collection containing a
+ // nested keyboard application collection that defines an input report.
+ auto device_info =
+ CreateHidDeviceInfo(TestReportDescriptors::VendorWithNestedKeyboard());
+ ASSERT_TRUE(device_info);
+ ASSERT_EQ(device_info->collections().size(), 1u);
+ EXPECT_EQ(device_info->collections()[0]->usage->usage_page,
+ mojom::kPageVendor);
+ CreateConnection(device_info);
+
+ SetConnectionClient();
+
+ // Simulate an input report from the nested keyboard collection. It should
+ // not be received by the client.
+ auto buffer =
+ base::MakeRefCounted<base::RefCountedBytes>(std::vector<uint8_t>{1});
+ connection().SimulateInputReport(buffer);
+ EXPECT_FALSE(HasNextInputReport());
+
+ // Close the connection.
+ connection().Close();
+ EXPECT_TRUE(connection().closed());
+}
+
} // namespace device
diff --git a/services/device/hid/hid_device_info_unittest.cc b/services/device/hid/hid_device_info_unittest.cc
index 05df2c4c..315b7172 100644
--- a/services/device/hid/hid_device_info_unittest.cc
... (truncated)
Original Bug Report
Potential WebHID security bypass via nested HID collections
Flapjack, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A logic flaw in the WebHID implementation allows a crafted HID device to bypass usage-based protections and security blocklists. By nesting a sensitive collection (like Keyboard or FIDO) inside a benign Top-Level Collection, the device’s protected reports can be exposed to untrusted web content.
Affected files:
services/device/public/cpp/hid/hid_blocklist.ccservices/device/hid/hid_connection.cccontent/browser/hid/hid_service.ccthird_party/blink/renderer/modules/hid/hid_device.ccservices/device/public/cpp/hid/hid_report_utils.cccontent/browser/service_worker/service_worker_hid_delegate_observer.cc
Estimated timestamp from git blame: Unknown (Google3 checkout)
Summary
Chromium’s WebHID implementation protects sensitive HID reports (e.g., Keyboards, FIDO security keys) from being accessed by untrusted websites. These protections rely on evaluating the usage pages of the device’s Top-Level Collections (TLCs).
However, a logic flaw exists where multiple security checks fail to recursively inspect nested child collections. Because the HID descriptor parser propagates report items to ancestor collections, a crafted HID device can define a sensitive usage within a nested collection beneath a benign TLC (such as a Gamepad). This causes the sensitive reports to be incorrectly evaluated against the benign TLC’s usage, bypassing blocklists and runtime protections, and allowing an untrusted website to read sensitive input (e.g., keystrokes).
Potential Attack Steps
Note: These are suggested steps based on source code analysis, as our tooling agent cannot execute a working proof of concept.
- Device Crafting: An attacker connects a crafted physical or emulated HID device. The device’s report descriptor defines a benign Top-Level Collection (TLC), such as
Generic Desktop / Gamepad(Usage Page 0x01, Usage 0x05). - Nested Sensitive Usage: Inside this TLC, the descriptor defines a nested child collection with a protected usage, such as
Keyboard(Usage Page 0x07) orFIDO(Usage Page 0xF1D0). - Report Definition: An Input Report (e.g., Report ID 1) is defined within the nested child collection.
- Descriptor Parsing: During device initialization,
device::HidCollection::AddReportItemparses the descriptor and propagates the Input Report item up the hierarchy to the parent benign TLC. - Blocklist Bypass: The browser evaluates the HID blocklist via
HidBlocklist::CheckBlocklistEntry. This function only iterates over thecollectionsarray (which contains only TLCs). Since the TLC usage isGamepad, it does not match FIDO or Keyboard blocklist rules, and the function fails to recurse into the child collections. Report ID 1 is not marked as protected. - WebHID Filtering Bypass: When an untrusted website requests the device via
navigator.hid.requestDevice(),HidService::RemoveProtectedReportsand Blink’sHIDDevice::UpdateDeviceInfofilter the exposed device information. Both functions evaluate only the TLC’s usage, leaving the sensitive reports exposed in the JavaScriptHIDDeviceobject. - Runtime Protection Bypass: The website calls
device.open()and listens forinputreportevents. When the device sends a sensitive payload on Report ID 1,HidConnection::IsReportProtectedvalidates the report. It usesFindCollectionWithReport, which locates the report in the benign TLC (due to step 4). The runtime check evaluates the benignGamepadusage and allows the report through. - Data Exfiltration: The untrusted website successfully receives the sensitive
inputreportevent.
Suggested Fix
To address this bypass, the security filters must properly account for nested collections:
- Recursive Blocklisting:
CheckBlocklistEntryinservices/device/public/cpp/hid/hid_blocklist.ccshould recursively evaluatecollection->childrenwhen checking usage pages, ensuring reports nested under sensitive usages are added to the protected IDs lists. - Precise Report Mapping:
FindCollectionWithReportinservices/device/public/cpp/hid/hid_report_utils.cccurrently returns the first matching collection, which is often the TLC due to propagation. It should be updated to return the innermost collection that defines the report, or the runtime checks inHidConnection::IsReportProtectedshould recursively verify the usage of all nested collections associated with the report. - UI/Renderer Filtering:
HidService::RemoveProtectedReportsandHIDDevice::UpdateDeviceInfoshould recursively evaluate nested collections to ensure protected nested collections are removed from the WebHID API surface.
Evaluated with Chrome root at commit: 2155cb00003ec35716a76ed3246eae995f87b7ff
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.