[Backport] CVE-2023-5482 and CVE-2023-5849

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4944690
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.

Bug: 1492381, 1492384
Change-Id: Id9ae16c7e6f1c417e0fc4f21d53e9de11560b2b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4944690
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212916}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/515436
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 74 insertions, 0 deletions

diff --git a/chromium/services/device/usb/mojo/device_impl.cc b/chromium/services/device/usb/mojo/device_impl.cc
index bc708148647..4c6affe583c 100644
--- a/chromium/services/device/usb/mojo/device_impl.cc
+++ b/chromium/services/device/usb/mojo/device_impl.cc
@@ -16,6 +16,7 @@
 #include "base/callback.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/optional.h"
 #include "base/stl_util.h"
 #include "services/device/public/cpp/usb/usb_utils.h"
 #include "services/device/usb/usb_descriptors.h"
@@ -84,6 +85,20 @@ void OnIsochronousTransferOut(
   std::move(callback).Run(std::move(packets));
 }
 
+// Returns the sum of `packet_lengths`, or nullopt if the sum would overflow.
+base::Optional<uint32_t> TotalPacketLength(
+    base::span<const uint32_t> packet_lengths) {
+  uint32_t total_bytes = 0;
+  for (const uint32_t packet_length : packet_lengths) {
+    // Check for overflow.
+    if (std::numeric_limits<uint32_t>::max() - total_bytes < packet_length) {
+      return base::nullopt;
+    }
+    total_bytes += packet_length;
+  }
+  return total_bytes;
+}
+
 }  // namespace
 
 // static
@@ -368,6 +383,15 @@ void DeviceImpl::IsochronousTransferIn(
     return;
   }
 
+  base::Optional<uint32_t> 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,
@@ -386,6 +410,14 @@ void DeviceImpl::IsochronousTransferOut(
     return;
   }
 
+  base::Optional<uint32_t> 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<base::RefCountedBytes>(data);
   device_handle_->IsochronousTransferOut(
diff --git a/chromium/third_party/blink/renderer/modules/webusb/usb_device.cc b/chromium/third_party/blink/renderer/modules/webusb/usb_device.cc
index 1ba2a231a0b..41a6ff5cbe1 100644
--- a/chromium/third_party/blink/renderer/modules/webusb/usb_device.cc
+++ b/chromium/third_party/blink/renderer/modules/webusb/usb_device.cc
@@ -6,8 +6,10 @@
 
 #include <algorithm>
 #include <iterator>
+#include <limits>
 #include <utility>
 
+#include "base/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"
@@ -42,6 +44,10 @@ namespace blink {
 namespace {
 
 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.";
@@ -179,6 +185,20 @@ bool ConvertBufferSource(const ArrayBufferOrArrayBufferView& buffer_source,
   return true;
 }
 
+// Returns the sum of `packet_lengths`, or nullopt if the sum would overflow.
+base::Optional<uint32_t> TotalPacketLength(
+    const Vector<unsigned>& packet_lengths) {
+  uint32_t total_bytes = 0;
+  for (const auto packet_length : packet_lengths) {
+    // Check for overflow.
+    if (std::numeric_limits<uint32_t>::max() - total_bytes < packet_length) {
+      return base::nullopt;
+    }
+    total_bytes += packet_length;
+  }
+  return total_bytes;
+}
+
 }  // namespace
 
 USBDevice::USBDevice(UsbDeviceInfoPtr device_info,
@@ -549,6 +569,14 @@ ScriptPromise USBDevice::isochronousTransferIn(
     Vector<unsigned> packet_lengths) {
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
+
+  base::Optional<uint32_t> total_bytes = TotalPacketLength(packet_lengths);
+  if (!total_bytes.has_value()) {
+    resolver->Reject(MakeGarbageCollected<DOMException>(
+        DOMExceptionCode::kDataError, kPacketLengthsTooBig));
+    return promise;
+  }
+
   if (EnsureEndpointAvailable(true /* in */, endpoint_number, resolver)) {
     device_requests_.insert(resolver);
     device_->IsochronousTransferIn(
@@ -569,6 +597,20 @@ ScriptPromise USBDevice::isochronousTransferOut(
   if (!EnsureEndpointAvailable(false /* out */, endpoint_number, resolver))
     return promise;
 
+  base::Optional<uint32_t> total_bytes = TotalPacketLength(packet_lengths);
+  if (!total_bytes.has_value()) {
+    resolver->Reject(MakeGarbageCollected<DOMException>(
+        DOMExceptionCode::kDataError, kPacketLengthsTooBig));
+    return promise;
+  }
+
+  DOMArrayBuffer* array_buffer = data.GetAsArrayBuffer();
+  if (total_bytes.value() != static_cast<uint32_t>(array_buffer->ByteLengthAsSizeT())) {
+     resolver->Reject(MakeGarbageCollected<DOMException>(
+        DOMExceptionCode::kDataError, kBufferSizeMismatch));
+    return promise;
+  }
+
   Vector<uint8_t> buffer;
   if (!ConvertBufferSource(data, &buffer, resolver))
     return promise;