summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorReilly Grant <reillyg@chromium.org>2020-06-12 17:41:53 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2020-10-05 12:02:29 +0000
commit46dbf8fb796336a8f5b77ce3d3c577669ab253ab (patch)
tree0defce34e8db99380908ce8fbd85117f4cb3f489
parent82a0e2faa2ab61eb7fa0b0403c6990d25e6ab801 (diff)
downloadqtwebengine-chromium-46dbf8fb796336a8f5b77ce3d3c577669ab253ab.tar.gz
[Backport] CVE-2020-6569: Integer overflow in WebUSB
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2242751: [usb] Use checked math to calculate Transfer size The size of the UsbDeviceHandleUsbfs::Transfer object depends on the number of isochronous packets that are requested. This change protects against integer overflow since the number of packets is controlled by script. Since the number of packets is also limited by the maximum Mojo message size this can be a CHECK rather than having code to handle overflow. Bug: 995732 Change-Id: Ie64be2d8fb8c8c1e49c7f676fb81446fce77d984 Reviewed-by: Michal Klocek <michal.klocek@qt.io> Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r--chromium/device/usb/usb_device_handle_usbfs.cc20
1 files changed, 14 insertions, 6 deletions
diff --git a/chromium/device/usb/usb_device_handle_usbfs.cc b/chromium/device/usb/usb_device_handle_usbfs.cc
index afbd070586b..e0d578bc0a8 100644
--- a/chromium/device/usb/usb_device_handle_usbfs.cc
+++ b/chromium/device/usb/usb_device_handle_usbfs.cc
@@ -16,6 +16,7 @@
#include "base/files/file_descriptor_watcher_posix.h"
#include "base/logging.h"
#include "base/memory/ref_counted_memory.h"
+#include "base/numerics/checked_math.h"
#include "base/posix/eintr_wrapper.h"
#include "base/sequence_checker.h"
#include "base/stl_util.h"
@@ -159,7 +160,7 @@ class UsbDeviceHandleUsbfs::FileThreadHelper {
DISALLOW_COPY_AND_ASSIGN(FileThreadHelper);
};
-struct UsbDeviceHandleUsbfs::Transfer {
+struct UsbDeviceHandleUsbfs::Transfer final {
Transfer() = delete;
Transfer(scoped_refptr<base::RefCountedBytes> buffer,
TransferCallback callback,
@@ -168,7 +169,7 @@ struct UsbDeviceHandleUsbfs::Transfer {
IsochronousTransferCallback callback);
~Transfer();
- void* operator new(std::size_t size, size_t number_of_iso_packets);
+ void* operator new(size_t size, size_t number_of_iso_packets);
void RunCallback(UsbTransferStatus status, size_t bytes_transferred);
void RunIsochronousCallback(const std::vector<IsochronousPacket>& packets);
@@ -356,7 +357,9 @@ UsbDeviceHandleUsbfs::Transfer::Transfer(
: buffer(buffer),
callback(std::move(callback)),
callback_runner(callback_runner) {
- memset(&urb, 0, sizeof(urb));
+ // This buffer size calculation is checked in operator new().
+ memset(&urb, 0,
+ sizeof(urb) + sizeof(urb.iso_frame_desc[0]) * urb.number_of_packets);
urb.usercontext = this;
urb.buffer = buffer->front();
}
@@ -374,10 +377,15 @@ UsbDeviceHandleUsbfs::Transfer::Transfer(
UsbDeviceHandleUsbfs::Transfer::~Transfer() = default;
void* UsbDeviceHandleUsbfs::Transfer::operator new(
- std::size_t size,
+ size_t size,
size_t number_of_iso_packets) {
- void* p = ::operator new(
- size + sizeof(usbdevfs_iso_packet_desc) * number_of_iso_packets);
+ // The checked math should pass as long as Mojo message size limits are being
+ // enforced.
+ size_t total_size =
+ base::CheckAdd(size, base::CheckMul(sizeof(urb.iso_frame_desc[0]),
+ number_of_iso_packets))
+ .ValueOrDie();
+ void* p = ::operator new(total_size);
Transfer* transfer = static_cast<Transfer*>(p);
transfer->urb.number_of_packets = number_of_iso_packets;
return p;