Merge r230101 - Out-of-bounds accesses due to a missing check for MAX…

…_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType

https://bugs.webkit.org/show_bug.cgi?id=183657
JSTests:

Reviewed by Keith Miller.

* stress/large-unshift-splice.js: Added.
(make_contig_arr):

Source/JavaScriptCore:

<rdar://problem/38464399>

Reviewed by Keith Miller.

There was just a missing check in unshiftCountForIndexingType.
I've also replaced 'return false' by 'return true' in the case of an 'out-of-memory' exception, because 'return false' means 'please continue to the slow path',
and the slow path has an assert that there is no unhandled exception (line 360 of ArrayPrototype.cpp).
Finally, I made the assert in ensureLength a release assert as it would have caught this bug and prevented it from being a security risk.

* runtime/ArrayPrototype.cpp:
(JSC::unshift):
* runtime/JSArray.cpp:
(JSC::JSArray::unshiftCountWithAnyIndexingType):
* runtime/JSObject.h:
(JSC::JSObject::ensureLength):

JSTests/ChangeLog

@@ -1,3 +1,13 @@
+2018-03-30  Robin Morisset  <rmorisset@apple.com>
+
+        Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
+        https://bugs.webkit.org/show_bug.cgi?id=183657
+
+        Reviewed by Keith Miller.
+
+        * stress/large-unshift-splice.js: Added.
+        (make_contig_arr):
+
 2018-03-28  Robin Morisset  <rmorisset@apple.com>
 
         appendQuotedJSONString stops on arithmetic overflow instead of propagating it upwards

JSTests/stress/large-unshift-splice.js

@@ -0,0 +1,16 @@
+//@ skip if $memoryLimited
+
+function make_contig_arr(sz)
+{
+    let a = []; 
+    while (a.length < sz / 8)
+        a.push(3.14); 
+    a.length *= 8;
+    return a;
+}
+
+let ARRAY_LENGTH = 0x10000000;
+let a = make_contig_arr(ARRAY_LENGTH);
+let b = make_contig_arr(0xff00);
+b.unshift(a.length-0x10000, 0);
+Array.prototype.splice.apply(a, b);

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,23 @@
+2018-03-30  Robin Morisset  <rmorisset@apple.com>
+
+        Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
+        https://bugs.webkit.org/show_bug.cgi?id=183657
+        <rdar://problem/38464399>
+
+        Reviewed by Keith Miller.
+
+        There was just a missing check in unshiftCountForIndexingType.
+        I've also replaced 'return false' by 'return true' in the case of an 'out-of-memory' exception, because 'return false' means 'please continue to the slow path',
+        and the slow path has an assert that there is no unhandled exception (line 360 of ArrayPrototype.cpp).
+        Finally, I made the assert in ensureLength a release assert as it would have caught this bug and prevented it from being a security risk.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::unshift):
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::unshiftCountWithAnyIndexingType):
+        * runtime/JSObject.h:
+        (JSC::JSObject::ensureLength):
+
 2018-03-28  Robin Morisset  <rmorisset@apple.com>
 
         appendQuotedJSONString stops on arithmetic overflow instead of propagating it upwards

Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -346,7 +346,7 @@ void unshift(ExecState* exec, JSObject* thisObj, unsigned header, unsigned curre
     RELEASE_ASSERT(currentCount <= (length - header));
 
     // Guard against overflow.
-    if (count > (UINT_MAX - length)) {
+    if (count > UINT_MAX - length) {
         throwOutOfMemoryError(exec, scope);
         return;
     }

Source/JavaScriptCore/runtime/JSArray.cpp

@@ -1060,10 +1060,13 @@ bool JSArray::unshiftCountWithAnyIndexingType(ExecState* exec, unsigned startInd
             scope.release();
             return unshiftCountWithArrayStorage(exec, startIndex, count, ensureArrayStorage(vm));
         }
-
+
+        if (oldLength + count > MAX_STORAGE_VECTOR_LENGTH)
+            return false;
+
         if (!ensureLength(vm, oldLength + count)) {
             throwOutOfMemoryError(exec, scope);
-            return false;
+            return true;
         }
         butterfly = this->butterfly();
 
@@ -1104,10 +1107,13 @@ bool JSArray::unshiftCountWithAnyIndexingType(ExecState* exec, unsigned startInd
             scope.release();
             return unshiftCountWithArrayStorage(exec, startIndex, count, ensureArrayStorage(vm));
         }
-
+
+        if (oldLength + count > MAX_STORAGE_VECTOR_LENGTH)
+            return false;
+
         if (!ensureLength(vm, oldLength + count)) {
             throwOutOfMemoryError(exec, scope);
-            return false;
+            return true;
         }
         butterfly = this->butterfly();
 

Source/JavaScriptCore/runtime/JSObject.h

@@ -982,7 +982,7 @@ class JSObject : public JSCell {
     // the array is contiguous.
     bool WARN_UNUSED_RETURN ensureLength(VM& vm, unsigned length)
     {
-        ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
+        RELEASE_ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
         ASSERT(hasContiguous(indexingType()) || hasInt32(indexingType()) || hasDouble(indexingType()) || hasUndecided(indexingType()));
 
         if (m_butterfly->vectorLength() < length) {