usb: Reject transfer size more than 32MB on renderer

Enforce a 32MB transfer size limit on WebUSB renderer side. This enforcement is to avoid user allocating a big buffer (e.g. close to 4GB) that might crash the browser in case no enough memory resources. Bug: 338955051 Change-Id: Ieac572e1abf6e85199e34550bf8e6434cad57052 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5633505 Commit-Queue: Jack Hsieh <[email protected]> Reviewed-by: Reilly Grant <[email protected]> Reviewed-by: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#1317691}
azx1573 · Jun 20, 2024 · a60cf17 · a60cf17
1 parent 1ee4eca
commit a60cf17
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 18 deletions.
diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc
@@ -2573,6 +2573,11 @@ BASE_FEATURE(kWebRtcUseMinMaxVEADimensions,
 // Allow access to WebSQL APIs.
 BASE_FEATURE(kWebSQLAccess, "kWebSQLAccess", base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Kill switch for https://crbug.com/338955051.
+BASE_FEATURE(kWebUSBTransferSizeLimit,
+             "WebUSBTransferSizeLimit",
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 // Enables small accelerated canvases for webview (crbug.com/1004304)
 BASE_FEATURE(kWebviewAccelerateSmallCanvases,
              "WebviewAccelerateSmallCanvases",

diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h
@@ -1661,6 +1661,8 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWebRtcUseMinMaxVEADimensions);
 
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWebSQLAccess);
 
+BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWebUSBTransferSizeLimit);
+
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWebviewAccelerateSmallCanvases);
 
 // When adding new features or constants for features, please keep the features

diff --git a/third_party/blink/renderer/modules/webusb/usb_device.cc b/third_party/blink/renderer/modules/webusb/usb_device.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/containers/span.h"
+#include "third_party/blink/public/common/features.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"
@@ -44,7 +45,6 @@ namespace blink {
 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[] =
@@ -63,6 +63,8 @@ const char kProtectedInterfaceClassError[] =
     "The requested interface implements a protected class.";
 const char kTransferPermissionDeniedError[] = "The transfer was not allowed.";
 
+const int kUsbTransferLengthLimit = 32 * 1024 * 1024;
+
 bool CheckFatalTransferStatus(ScriptPromiseResolverBase* resolver,
                               const UsbTransferStatus& status) {
   switch (status) {
@@ -126,6 +128,24 @@ std::optional<uint32_t> TotalPacketLength(
   return total_bytes;
 }
 
+bool ShouldRejectUsbTransferLength(size_t length,
+                                   ExceptionState& exception_state) {
+  if (!base::FeatureList::IsEnabled(
+          blink::features::kWebUSBTransferSizeLimit)) {
+    return false;
+  }
+
+  if (length <= kUsbTransferLengthLimit) {
+    return false;
+  }
+  exception_state.ThrowDOMException(
+      DOMExceptionCode::kDataError,
+      String::Format(
+          "The data buffer exceeded supported maximum size of %d bytes",
+          kUsbTransferLengthLimit));
+  return true;
+}
+
 }  // namespace
 
 USBDevice::USBDevice(USB* parent,
@@ -405,7 +425,7 @@ ScriptPromise<IDLUndefined> USBDevice::selectAlternateInterface(
 ScriptPromise<USBInTransferResult> USBDevice::controlTransferIn(
     ScriptState* script_state,
     const USBControlTransferParameters* setup,
-    unsigned length,
+    uint16_t length,
     ExceptionState& exception_state) {
   EnsureNoDeviceOrInterfaceChangeInProgress(exception_state);
   if (exception_state.HadException())
@@ -469,9 +489,8 @@ ScriptPromise<USBOutTransferResult> USBDevice::controlTransferOut(
       return EmptyPromise();
     }
 
-    if (optional_data.ByteLength() > std::numeric_limits<uint32_t>::max()) {
-      exception_state.ThrowDOMException(DOMExceptionCode::kDataError,
-                                        kBufferTooBig);
+    if (ShouldRejectUsbTransferLength(optional_data.ByteLength(),
+                                      exception_state)) {
       return EmptyPromise();
     }
 
@@ -520,6 +539,9 @@ ScriptPromise<USBInTransferResult> USBDevice::transferIn(
     uint8_t endpoint_number,
     unsigned length,
     ExceptionState& exception_state) {
+  if (ShouldRejectUsbTransferLength(length, exception_state)) {
+    return EmptyPromise();
+  }
   EnsureEndpointAvailable(/*in_transfer=*/true, endpoint_number,
                           exception_state);
   if (exception_state.HadException())
@@ -556,9 +578,7 @@ ScriptPromise<USBOutTransferResult> USBDevice::transferOut(
     return EmptyPromise();
   }
 
-  if (data.ByteLength() > std::numeric_limits<uint32_t>::max()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kDataError,
-                                      kBufferTooBig);
+  if (ShouldRejectUsbTransferLength(data.ByteLength(), exception_state)) {
     return EmptyPromise();
   }
 
@@ -592,6 +612,9 @@ ScriptPromise<USBIsochronousInTransferResult> USBDevice::isochronousTransferIn(
                                       kPacketLengthsTooBig);
     return EmptyPromise();
   }
+  if (ShouldRejectUsbTransferLength(total_bytes.value(), exception_state)) {
+    return EmptyPromise();
+  }
 
   auto* resolver = MakeGarbageCollected<
       ScriptPromiseResolver<USBIsochronousInTransferResult>>(
@@ -631,6 +654,9 @@ USBDevice::isochronousTransferOut(ScriptState* script_state,
                                       kPacketLengthsTooBig);
     return EmptyPromise();
   }
+  if (ShouldRejectUsbTransferLength(total_bytes.value(), exception_state)) {
+    return EmptyPromise();
+  }
   if (total_bytes.value() != data.ByteLength()) {
     exception_state.ThrowDOMException(DOMExceptionCode::kDataError,
                                       kBufferSizeMismatch);

diff --git a/third_party/blink/renderer/modules/webusb/usb_device.h b/third_party/blink/renderer/modules/webusb/usb_device.h
@@ -90,7 +90,7 @@ class USBDevice : public ScriptWrappable,
   ScriptPromise<USBInTransferResult> controlTransferIn(
       ScriptState*,
       const USBControlTransferParameters* setup,
-      unsigned length,
+      uint16_t length,
       ExceptionState&);
   ScriptPromise<USBOutTransferResult> controlTransferOut(
       ScriptState*,

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
@@ -1279,28 +1279,60 @@ usb_test(async (t) => {
 }, '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();
+  const {device} = await getFakeDevice();
   await device.open();
   await device.selectConfiguration(2);
   await device.claimInterface(0);
   await device.selectAlternateInterface(0, 1);
-  const packetLengths = [0xffffffff, 1];
+  const packetLengths = [33554432, 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();
+  const {device} = 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];
+  const buffer = new Uint8Array(33554432 + 1);
+  const packetLengths = [33554432, 1];
   await promise_rejects_dom(
       t, 'DataError', device.isochronousTransferOut(1, buffer, packetLengths));
 }, 'isochronousTransferOut rejects when packet lengths exceed maximum size');
+
+usb_test(async (t) => {
+  const {device} = await getFakeDevice();
+  await device.open();
+  await device.selectConfiguration(2);
+  await device.claimInterface(0);
+  await device.selectAlternateInterface(0, 1);
+  await promise_rejects_dom(
+      t, 'DataError', device.transferIn(1, 33554433));
+}, 'transferIn rejects when packet lengths exceed maximum size');
+
+usb_test(async (t) => {
+  const {device} = await getFakeDevice();
+  await device.open();
+  await device.selectConfiguration(2);
+  await device.claimInterface(0);
+  await device.selectAlternateInterface(0, 1);
+  await promise_rejects_dom(
+      t, 'DataError', device.transferOut(1, new ArrayBuffer(33554433)));
+}, 'transferOut rejects when packet lengths exceed maximum size');
+
+usb_test(async (t) => {
+  const {device} = await getFakeDevice();
+  await device.open();
+  await device.selectConfiguration(2);
+  await device.claimInterface(0);
+  await device.selectAlternateInterface(0, 1);
+  await promise_rejects_dom(
+      t, 'DataError', device.controlTransferOut({
+        requestType: 'vendor',
+        recipient: 'device',
+        request: 0x42,
+        value: 0x1234,
+        index: 0x5678
+      }, new ArrayBuffer(33554433)));
+}, 'controlTransferOut rejects when packet lengths exceed maximum size');