Add validation to length, packets and packetLength parameters

chrome/browser/extensions/api/usb/usb_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/usb/usb_api.h"
 
 #include <string>
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 46 matching lines @@
     "Error setting alternate interface setting.";
 static const char* kErrorConvertDirection = "Invalid transfer direction.";
 static const char* kErrorConvertRecipient = "Invalid transfer recipient.";
 static const char* kErrorConvertRequestType = "Invalid request type.";
 static const char* kErrorMalformedParameters = "Error parsing parameters.";
 static const char* kErrorNoDevice = "No such device.";
 static const char* kErrorPermissionDenied =
     "Permission to access device was denied";
 static const char* kErrorInvalidTransferLength = "Transfer length must be a "
     "positive number less than 104,857,600.";
+static const char* kErrorInvalidNumberOfPackets = "Number of packets must be a "
+    "positive number less than 4,194,304.";
+static const char* kErrorInvalidPacketLength = "Packet length must be a "
+    "positive number less than 65,536.";
 
 static const size_t kMaxTransferLength = 100 * 1024 * 1024;
+static const int kMaxPackets = 4 * 1024 * 1024;
+static const int kMaxPacketLength = 64 * 1024;
+
 static UsbDevice* device_for_test_ = NULL;
 
 static bool ConvertDirection(const Direction& input,
                              UsbDevice::TransferDirection* output) {
   switch (input) {
     case usb::DIRECTION_IN:
       *output = UsbDevice::INBOUND;
       return true;
     case usb::DIRECTION_OUT:
       *output = UsbDevice::OUTBOUND;
@@ skipping 64 matching lines @@
       return true;
     }
   }
   return false;
 }
 
 template<class T>
 static scoped_refptr<net::IOBuffer> CreateBufferForTransfer(
     const T& input, UsbDevice::TransferDirection direction, size_t size) {
 
-  if (size > kMaxTransferLength)
+  if (size >= kMaxTransferLength)
     return NULL;
 
   // Allocate a |size|-bytes buffer, or a one-byte buffer if |size| is 0. This
   // is due to an impedance mismatch between IOBuffer and URBs. An IOBuffer
   // cannot represent a zero-length buffer, while an URB can.
   scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(std::max(
       static_cast<size_t>(1), size));
 
   if (direction == UsbDevice::INBOUND) {
     return buffer;
@@ skipping 457 matching lines @@
   const IsochronousTransferInfo& transfer = parameters_->transfer_info;
   const GenericTransferInfo& generic_transfer = transfer.transfer_info;
 
   size_t size = 0;
   UsbDevice::TransferDirection direction;
 
   if (!ConvertDirectionSafely(generic_transfer.direction, &direction)) {
     AsyncWorkCompleted();
     return;
   }
-
   if (!GetTransferSize(generic_transfer, &size)) {
     CompleteWithError(kErrorInvalidTransferLength);
     return;
   }
+  if (transfer.packets < 0 || transfer.packets >= kMaxPackets) {
+    CompleteWithError(kErrorInvalidNumberOfPackets);
+    return;
+  }
+  unsigned int packets = transfer.packets;

[miket_OOO, 2013/01/28 21:23:40]

Would a size_t work here?

[meacer, 2013/01/28 21:26:25]

I made it an int just because it's used in a multiplication below.

+  if (transfer.packet_length < 0 ||
+      transfer.packet_length >= kMaxPacketLength) {
+    CompleteWithError(kErrorInvalidPacketLength);
+    return;
+  }
+  unsigned int packet_length = transfer.packet_length;
+  const uint64 total_length = packets * packet_length;
+  if (packets > size || total_length > size) {
+    CompleteWithError(kErrorTransferLength);
+    return;
+  }
 
   scoped_refptr<net::IOBuffer> buffer = CreateBufferForTransfer(
       generic_transfer, direction, size);
   if (!buffer) {
     CompleteWithError(kErrorMalformedParameters);
     return;
   }
 
   device->device()->IsochronousTransfer(direction, generic_transfer.endpoint,
-      buffer, size, transfer.packets, transfer.packet_length, 0, base::Bind(
+      buffer, size, packets, packet_length, 0, base::Bind(
           &UsbIsochronousTransferFunction::OnCompleted, this));
 }
 
 }  // namespace extensions