Fix signed integer overflow in ImageData

third_party/WebKit/Source/core/html/ImageData.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 129 matching lines @@
 
 DOMArrayBufferView* ImageData::allocateAndValidateDataArray(
     const unsigned& length,
     ImageDataStorageFormat storageFormat,
     ExceptionState* exceptionState) {
   if (!length)
     return nullptr;
 
   DOMArrayBufferView* dataArray = nullptr;
   unsigned dataLength = 0;
+  unsigned dataItemLength = 1;
   switch (storageFormat) {
     case kUint8ClampedArrayStorageFormat:
       dataArray = DOMUint8ClampedArray::createOrNull(length);
-      dataLength = dataArray->view()->byteLength();
       break;
     case kUint16ArrayStorageFormat:
       dataArray = DOMUint16Array::createOrNull(length);
-      dataLength = dataArray->view()->byteLength() / 2;
+      dataItemLength = 2;
       break;
     case kFloat32ArrayStorageFormat:
       dataArray = DOMFloat32Array::createOrNull(length);
-      dataLength = dataArray->view()->byteLength() / 4;
+      dataItemLength = 4;
       break;
     default:
       NOTREACHED();
   }
 
+  if (dataArray)
+    dataLength = dataArray->view()->byteLength() / dataItemLength;
+
   if (!dataArray || length != dataLength) {
     if (exceptionState)
       exceptionState->throwDOMException(V8RangeError,
                                         "Out of memory at ImageData creation");
     return nullptr;
   }
 
   return dataArray;
 }
 
 ImageData* ImageData::create(const IntSize& size) {
   if (!ImageData::validateConstructorArguments(kParamSize, &size))
     return nullptr;
   DOMArrayBufferView* byteArray = allocateAndValidateDataArray(
-      4 * size.width() * size.height(), kUint8ClampedArrayStorageFormat);
-  return new ImageData(size, byteArray);
+      4 * (unsigned)(size.width()) * (unsigned)(size.height()),

[Justin Novosad, 2017/03/20 16:03:55]

Are we sure that this multiplication will never overlflow the unsigned type?

[zakerinasab, 2017/03/20 16:06:33]

Yes, this is taken care of in validateConstructorArguments(). Please see line
127:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/Imag...

[Justin Novosad, 2017/03/20 16:18:40]

As far as I can tell, validateConstructorArguments does not verify that width
and height are positive.  Also you should not use a c-style cast. Please use
static_cast<unsigned>()

[zakerinasab, 2017/03/20 16:31:26]

Oh, right. Fixed now.

+      kUint8ClampedArrayStorageFormat);
+  return byteArray ? new ImageData(size, byteArray) : nullptr;
 }
 
 // This function accepts size (0, 0) and always returns the ImageData in
 // "srgb" color space and "uint8" storage format.
 ImageData* ImageData::createForTest(const IntSize& size) {
   CheckedNumeric<unsigned> dataSize = 4;
   dataSize *= size.width();
   dataSize *= size.height();
   if (!dataSize.IsValid())
     return nullptr;
@@ skipping 225 matching lines @@
       SECURITY_CHECK(static_cast<unsigned>(size.width() * size.height() * 4) <=
                      m_dataF32->length());
       break;
 
     default:
       NOTREACHED();
   }
 }
 
 }  // namespace blink