diff options
author | Reilly Grant <reillyg@chromium.org> | 2020-06-12 17:41:53 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2020-10-05 12:02:29 +0000 |
commit | 46dbf8fb796336a8f5b77ce3d3c577669ab253ab (patch) | |
tree | 0defce34e8db99380908ce8fbd85117f4cb3f489 | |
parent | 82a0e2faa2ab61eb7fa0b0403c6990d25e6ab801 (diff) | |
download | qtwebengine-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.cc | 20 |
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; |