diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 60d599f569f67..3d4f1fd0dda45 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -137,3 +137,4 @@ cherry-pick-f218b4f37018.patch cherry-pick-d756d71a652c.patch parameterize_axtreeserializer_by_vector_type.patch avoid_allocating_recordid_objects_in_elementtiming_and_lcp.patch +cherry-pick-80106e31c7ea.patch diff --git a/patches/chromium/cherry-pick-80106e31c7ea.patch b/patches/chromium/cherry-pick-80106e31c7ea.patch new file mode 100644 index 0000000000000..3f8ae7d1b2c2b --- /dev/null +++ b/patches/chromium/cherry-pick-80106e31c7ea.patch @@ -0,0 +1,428 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Matt Reynolds +Date: Wed, 25 Oct 2023 00:56:26 +0000 +Subject: usb: Validate isochronous transfer packet lengths + +USBDevice.isochronousTransferIn and +USBDevice.isochronousTransferOut take a parameter containing +a list of packet lengths. This CL adds validation that the +total packet length does not exceed the maximum buffer size. +For isochronousTransferOut, it also checks that the total +length of all packets in bytes is equal to the size of the +data buffer. + +Passing invalid packet lengths causes the promise to be +rejected with a DataError. + +(cherry picked from commit bb36f739e7e0a3722beeb2744744195c22fd6143) + +Bug: 1492381, 1492384 +Change-Id: Id9ae16c7e6f1c417e0fc4f21d53e9de11560b2b7 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4944690 +Reviewed-by: Reilly Grant +Commit-Queue: Matt Reynolds +Cr-Original-Commit-Position: refs/heads/main@{#1212916} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974416 +Commit-Queue: Reilly Grant +Auto-Submit: Matt Reynolds +Cr-Commit-Position: refs/branch-heads/5993@{#1425} +Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594} + +diff --git a/services/device/usb/mojo/device_impl.cc b/services/device/usb/mojo/device_impl.cc +index 34cc1f360a0340fa235ee0e086f5f5b2ee56309d..a44cb3b262203cc519012e60465cc01af9b4260c 100644 +--- a/services/device/usb/mojo/device_impl.cc ++++ b/services/device/usb/mojo/device_impl.cc +@@ -19,6 +19,7 @@ + #include "base/ranges/algorithm.h" + #include "services/device/public/cpp/usb/usb_utils.h" + #include "services/device/usb/usb_device.h" ++#include "third_party/abseil-cpp/absl/types/optional.h" + + namespace device { + +@@ -89,6 +90,20 @@ bool IsAndroidSecurityKeyRequest( + memcmp(data.data(), magic, strlen(magic)) == 0; + } + ++// Returns the sum of `packet_lengths`, or nullopt if the sum would overflow. ++absl::optional TotalPacketLength( ++ base::span packet_lengths) { ++ uint32_t total_bytes = 0; ++ for (const uint32_t packet_length : packet_lengths) { ++ // Check for overflow. ++ if (std::numeric_limits::max() - total_bytes < packet_length) { ++ return absl::nullopt; ++ } ++ total_bytes += packet_length; ++ } ++ return total_bytes; ++} ++ + } // namespace + + // static +@@ -397,6 +412,15 @@ void DeviceImpl::IsochronousTransferIn( + return; + } + ++ absl::optional total_bytes = TotalPacketLength(packet_lengths); ++ if (!total_bytes.has_value()) { ++ mojo::ReportBadMessage("Invalid isochronous packet lengths."); ++ std::move(callback).Run( ++ {}, BuildIsochronousPacketArray( ++ packet_lengths, mojom::UsbTransferStatus::TRANSFER_ERROR)); ++ return; ++ } ++ + uint8_t endpoint_address = endpoint_number | 0x80; + device_handle_->IsochronousTransferIn( + endpoint_address, packet_lengths, timeout, +@@ -415,6 +439,14 @@ void DeviceImpl::IsochronousTransferOut( + return; + } + ++ absl::optional total_bytes = TotalPacketLength(packet_lengths); ++ if (!total_bytes.has_value() || total_bytes.value() != data.size()) { ++ mojo::ReportBadMessage("Invalid isochronous packet lengths."); ++ std::move(callback).Run(BuildIsochronousPacketArray( ++ packet_lengths, mojom::UsbTransferStatus::TRANSFER_ERROR)); ++ return; ++ } ++ + uint8_t endpoint_address = endpoint_number; + auto buffer = base::MakeRefCounted(data); + device_handle_->IsochronousTransferOut( +diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc +index b23918682064047f644344f96c7a13ccbbe7d95d..2d401f70b2b9d827153adb8b14a5acb6d4a9a8de 100644 +--- a/services/device/usb/mojo/device_impl_unittest.cc ++++ b/services/device/usb/mojo/device_impl_unittest.cc +@@ -9,7 +9,6 @@ + + #include + #include +-#include + #include + #include + #include +@@ -54,8 +53,11 @@ MATCHER_P(BufferSizeIs, size, "") { + + class ConfigBuilder { + public: +- explicit ConfigBuilder(uint8_t value) +- : config_(BuildUsbConfigurationInfoPtr(value, false, false, 0)) {} ++ explicit ConfigBuilder(uint8_t configuration_value) ++ : config_(BuildUsbConfigurationInfoPtr(configuration_value, ++ /*self_powered=*/false, ++ /*remote_wakeup=*/false, ++ /*maximum_power=*/0)) {} + + ConfigBuilder(const ConfigBuilder&) = delete; + ConfigBuilder& operator=(const ConfigBuilder&) = delete; +@@ -413,8 +415,10 @@ class USBDeviceImplTest : public testing::Test { + + ASSERT_EQ(packets.size(), packet_lengths.size()); + for (size_t i = 0; i < packets.size(); ++i) { +- EXPECT_EQ(packets[i]->length, packet_lengths[i]) +- << "Packet lengths differ at index: " << i; ++ if (packets[i]->status == mojom::UsbTransferStatus::COMPLETED) { ++ EXPECT_EQ(packets[i]->length, packet_lengths[i]) ++ << "Packet lengths differ at index: " << i; ++ } + } + + std::move(callback).Run(buffer, std::move(packets)); +@@ -428,10 +432,8 @@ class USBDeviceImplTest : public testing::Test { + UsbDeviceHandle::IsochronousTransferCallback& callback) { + ASSERT_FALSE(mock_outbound_data_.empty()); + const std::vector& bytes = mock_outbound_data_.front(); +- size_t length = +- std::accumulate(packet_lengths.begin(), packet_lengths.end(), 0u); +- ASSERT_EQ(bytes.size(), length); +- for (size_t i = 0; i < length; ++i) { ++ ASSERT_EQ(buffer->size(), bytes.size()); ++ for (size_t i = 0; i < bytes.size(); ++i) { + EXPECT_EQ(bytes[i], buffer->front()[i]) + << "Contents differ at index: " << i; + } +@@ -444,8 +446,10 @@ class USBDeviceImplTest : public testing::Test { + + ASSERT_EQ(packets.size(), packet_lengths.size()); + for (size_t i = 0; i < packets.size(); ++i) { +- EXPECT_EQ(packets[i]->length, packet_lengths[i]) +- << "Packet lengths differ at index: " << i; ++ if (packets[i]->status == mojom::UsbTransferStatus::COMPLETED) { ++ EXPECT_EQ(packets[i]->length, packet_lengths[i]) ++ << "Packet lengths differ at index: " << i; ++ } + } + + std::move(callback).Run(buffer, std::move(packets)); +@@ -1084,6 +1088,122 @@ TEST_F(USBDeviceImplTest, IsochronousTransfer) { + EXPECT_CALL(mock_handle(), Close()); + } + ++TEST_F(USBDeviceImplTest, IsochronousTransferOutBufferSizeMismatch) { ++ mojo::Remote device = GetMockDeviceProxy(); ++ ++ EXPECT_CALL(mock_device(), OpenInternal); ++ ++ base::test::TestFuture open_future; ++ device->Open(open_future.GetCallback()); ++ EXPECT_TRUE(open_future.Get()->is_success()); ++ ++ constexpr size_t kPacketCount = 4; ++ constexpr size_t kPacketLength = 8; ++ std::vector fake_packets; ++ for (size_t i = 0; i < kPacketCount; ++i) { ++ fake_packets.push_back(mojom::UsbIsochronousPacket::New( ++ kPacketLength, kPacketLength, UsbTransferStatus::TRANSFER_ERROR)); ++ } ++ ++ std::string outbound_data = "aaaaaaaabbbbbbbbccccccccdddddddd"; ++ std::vector fake_outbound_data(outbound_data.size()); ++ base::ranges::copy(outbound_data, fake_outbound_data.begin()); ++ ++ std::string inbound_data = "ddddddddccccccccbbbbbbbbaaaaaaaa"; ++ std::vector fake_inbound_data(inbound_data.size()); ++ base::ranges::copy(inbound_data, fake_inbound_data.begin()); ++ ++ AddMockConfig(ConfigBuilder(/*configuration_value=*/1) ++ .AddInterface(/*interface_number=*/7, ++ /*alternate_setting=*/0, /*class_code=*/1, ++ /*subclass_code=*/2, /*protocol_code=*/3) ++ .Build()); ++ AddMockOutboundPackets(fake_outbound_data, mojo::Clone(fake_packets)); ++ AddMockInboundPackets(fake_inbound_data, mojo::Clone(fake_packets)); ++ ++ // The `packet_lengths` parameter for IsochronousTransferOut describes the ++ // number of bytes in each packet. Set the size of the last packet one byte ++ // shorter than the buffer size and check that the returned packets indicate ++ // a transfer error. ++ std::vector short_packet_lengths(kPacketCount, kPacketLength); ++ short_packet_lengths.back() = kPacketLength - 1; ++ ++ base::test::TestFuture> ++ transfer_out_future; ++ device->IsochronousTransferOut( ++ /*endpoint_number=*/1, fake_outbound_data, short_packet_lengths, ++ /*timeout=*/0, transfer_out_future.GetCallback()); ++ ASSERT_EQ(kPacketCount, transfer_out_future.Get().size()); ++ for (const auto& packet : transfer_out_future.Get()) { ++ EXPECT_EQ(packet->status, UsbTransferStatus::TRANSFER_ERROR); ++ } ++ ++ EXPECT_CALL(mock_handle(), Close); ++} ++ ++TEST_F(USBDeviceImplTest, IsochronousTransferPacketLengthsOverflow) { ++ mojo::Remote device = GetMockDeviceProxy(); ++ ++ EXPECT_CALL(mock_device(), OpenInternal); ++ ++ base::test::TestFuture open_future; ++ device->Open(open_future.GetCallback()); ++ EXPECT_TRUE(open_future.Get()->is_success()); ++ ++ constexpr size_t kPacketCount = 2; ++ constexpr size_t kPacketLength = 8; ++ std::vector fake_packets; ++ for (size_t i = 0; i < kPacketCount; ++i) { ++ fake_packets.push_back(mojom::UsbIsochronousPacket::New( ++ kPacketLength, kPacketLength, UsbTransferStatus::TRANSFER_ERROR)); ++ } ++ ++ std::string outbound_data = "aaaaaaaabbbbbbbb"; ++ std::vector fake_outbound_data(outbound_data.size()); ++ base::ranges::copy(outbound_data, fake_outbound_data.begin()); ++ ++ std::string inbound_data = "bbbbbbbbaaaaaaaa"; ++ std::vector fake_inbound_data(inbound_data.size()); ++ base::ranges::copy(inbound_data, fake_inbound_data.begin()); ++ ++ AddMockConfig(ConfigBuilder(/*configuration_value=*/1) ++ .AddInterface(/*interface_number=*/7, ++ /*alternate_setting=*/0, /*class_code=*/1, ++ /*subclass_code=*/2, /*protocol_code=*/3) ++ .Build()); ++ AddMockOutboundPackets(fake_outbound_data, mojo::Clone(fake_packets)); ++ AddMockInboundPackets(fake_inbound_data, mojo::Clone(fake_packets)); ++ ++ // The `packet_lengths` parameter for IsochronousTransferOut and ++ // IsochronousTransferIn describes the number of bytes in each packet. Set ++ // the packet sizes so the total will exceed the maximum value for uint32_t ++ // and check that the returned packets indicate a transfer error. ++ std::vector overflow_packet_lengths = {0xffffffff, 1}; ++ ++ base::test::TestFuture> ++ transfer_out_future; ++ device->IsochronousTransferOut( ++ /*endpoint_number=*/1, fake_outbound_data, overflow_packet_lengths, ++ /*timeout=*/0, transfer_out_future.GetCallback()); ++ ASSERT_EQ(kPacketCount, transfer_out_future.Get().size()); ++ for (const auto& packet : transfer_out_future.Get()) { ++ EXPECT_EQ(packet->status, UsbTransferStatus::TRANSFER_ERROR); ++ } ++ ++ base::test::TestFuture, ++ std::vector> ++ transfer_in_future; ++ device->IsochronousTransferIn( ++ /*endpoint_number=*/1, overflow_packet_lengths, /*timeout=*/0, ++ transfer_in_future.GetCallback()); ++ ASSERT_EQ(kPacketCount, transfer_in_future.Get<1>().size()); ++ for (const auto& packet : transfer_in_future.Get<1>()) { ++ EXPECT_EQ(packet->status, UsbTransferStatus::TRANSFER_ERROR); ++ } ++ ++ EXPECT_CALL(mock_handle(), Close); ++} ++ + class USBDeviceImplSecurityKeyTest : public USBDeviceImplTest, + public testing::WithParamInterface { + }; +diff --git a/third_party/blink/renderer/modules/webusb/usb_device.cc b/third_party/blink/renderer/modules/webusb/usb_device.cc +index 3d562fa22bd84dc438abfe9fa883eff6f5846b1b..c64c7fb1b15f7f523b37671abca2ab50655cf4d8 100644 +--- a/third_party/blink/renderer/modules/webusb/usb_device.cc ++++ b/third_party/blink/renderer/modules/webusb/usb_device.cc +@@ -4,9 +4,11 @@ + + #include "third_party/blink/renderer/modules/webusb/usb_device.h" + ++#include + #include + + #include "base/containers/span.h" ++#include "third_party/abseil-cpp/absl/types/optional.h" + #include "third_party/blink/public/platform/platform.h" + #include "third_party/blink/renderer/bindings/core/v8/script_promise.h" + #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h" +@@ -43,6 +45,10 @@ namespace { + + const char kAccessDeniedError[] = "Access denied."; + const char kBufferTooBig[] = "The data buffer exceeded its maximum size."; ++const char kPacketLengthsTooBig[] = ++ "The total packet length exceeded the maximum size."; ++const char kBufferSizeMismatch[] = ++ "The data buffer size must match the total packet length."; + const char kDetachedBuffer[] = "The data buffer has been detached."; + const char kDeviceStateChangeInProgress[] = + "An operation that changes the device state is in progress."; +@@ -106,6 +112,20 @@ String ConvertTransferStatus(const UsbTransferStatus& status) { + } + } + ++// Returns the sum of `packet_lengths`, or nullopt if the sum would overflow. ++absl::optional TotalPacketLength( ++ const Vector& packet_lengths) { ++ uint32_t total_bytes = 0; ++ for (const auto packet_length : packet_lengths) { ++ // Check for overflow. ++ if (std::numeric_limits::max() - total_bytes < packet_length) { ++ return absl::nullopt; ++ } ++ total_bytes += packet_length; ++ } ++ return total_bytes; ++} ++ + } // namespace + + USBDevice::USBDevice(USB* parent, +@@ -555,6 +575,13 @@ ScriptPromise USBDevice::isochronousTransferIn( + if (exception_state.HadException()) + return ScriptPromise(); + ++ absl::optional total_bytes = TotalPacketLength(packet_lengths); ++ if (!total_bytes.has_value()) { ++ exception_state.ThrowDOMException(DOMExceptionCode::kDataError, ++ kPacketLengthsTooBig); ++ return ScriptPromise(); ++ } ++ + auto* resolver = MakeGarbageCollected( + script_state, exception_state.GetContext()); + ScriptPromise promise = resolver->Promise(); +@@ -586,6 +613,18 @@ ScriptPromise USBDevice::isochronousTransferOut( + return ScriptPromise(); + } + ++ absl::optional total_bytes = TotalPacketLength(packet_lengths); ++ if (!total_bytes.has_value()) { ++ exception_state.ThrowDOMException(DOMExceptionCode::kDataError, ++ kPacketLengthsTooBig); ++ return ScriptPromise(); ++ } ++ if (total_bytes.value() != data.ByteLength()) { ++ exception_state.ThrowDOMException(DOMExceptionCode::kDataError, ++ kBufferSizeMismatch); ++ return ScriptPromise(); ++ } ++ + auto* resolver = MakeGarbageCollected( + script_state, exception_state.GetContext()); + ScriptPromise promise = resolver->Promise(); +diff --git a/third_party/blink/web_tests/external/wpt/webusb/usbDevice.https.any.js b/third_party/blink/web_tests/external/wpt/webusb/usbDevice.https.any.js +index b1b0c133ce160a314ea392514ac5b38e4cac136d..804af2afb9db3a0d5fafbeb26aed64f89badb1b3 100644 +--- a/third_party/blink/web_tests/external/wpt/webusb/usbDevice.https.any.js ++++ b/third_party/blink/web_tests/external/wpt/webusb/usbDevice.https.any.js +@@ -1247,3 +1247,60 @@ usb_test((t) => { + .then(() => promise_rejects_dom(t, 'NotFoundError', device.reset())); + }); + }, 'resetDevice rejects when called on a disconnected device'); ++ ++usb_test(async (t) => { ++ const PACKET_COUNT = 4; ++ const PACKET_LENGTH = 8; ++ const {device, fakeDevice} = await getFakeDevice(); ++ await device.open(); ++ await device.selectConfiguration(2); ++ await device.claimInterface(0); ++ await device.selectAlternateInterface(0, 1); ++ const buffer = new Uint8Array(PACKET_COUNT * PACKET_LENGTH); ++ const packetLengths = new Array(PACKET_COUNT).fill(PACKET_LENGTH); ++ packetLengths[0] = PACKET_LENGTH - 1; ++ await promise_rejects_dom( ++ t, 'DataError', device.isochronousTransferOut(1, buffer, packetLengths)); ++}, 'isochronousTransferOut rejects when buffer size exceeds packet lengths'); ++ ++usb_test(async (t) => { ++ const PACKET_COUNT = 4; ++ const PACKET_LENGTH = 8; ++ const {device, fakeDevice} = await getFakeDevice(); ++ await device.open(); ++ await device.selectConfiguration(2); ++ await device.claimInterface(0); ++ await device.selectAlternateInterface(0, 1); ++ const buffer = new Uint8Array(PACKET_COUNT * PACKET_LENGTH); ++ const packetLengths = new Array(PACKET_COUNT).fill(PACKET_LENGTH); ++ packetLengths[0] = PACKET_LENGTH + 1; ++ await promise_rejects_dom( ++ t, 'DataError', device.isochronousTransferOut(1, buffer, packetLengths)); ++}, 'isochronousTransferOut rejects when packet lengths exceed buffer size'); ++ ++usb_test(async (t) => { ++ const PACKET_COUNT = 2; ++ const PACKET_LENGTH = 8; ++ const {device, fakeDevice} = await getFakeDevice(); ++ await device.open(); ++ await device.selectConfiguration(2); ++ await device.claimInterface(0); ++ await device.selectAlternateInterface(0, 1); ++ const packetLengths = [0xffffffff, 1]; ++ await promise_rejects_dom( ++ t, 'DataError', device.isochronousTransferIn(1, packetLengths)); ++}, 'isochronousTransferIn rejects when packet lengths exceed maximum size'); ++ ++usb_test(async (t) => { ++ const PACKET_COUNT = 2; ++ const PACKET_LENGTH = 8; ++ const {device, fakeDevice} = await getFakeDevice(); ++ await device.open(); ++ await device.selectConfiguration(2); ++ await device.claimInterface(0); ++ await device.selectAlternateInterface(0, 1); ++ const buffer = new Uint8Array(PACKET_LENGTH * PACKET_COUNT); ++ const packetLengths = [0xffffffff, 1]; ++ await promise_rejects_dom( ++ t, 'DataError', device.isochronousTransferOut(1, buffer, packetLengths)); ++}, 'isochronousTransferOut rejects when packet lengths exceed maximum size');