Title: [230238] branches/safari-605-branch
Revision
230238
Author
jmarc...@apple.com
Date
2018-04-03 20:27:33 -0700 (Tue, 03 Apr 2018)

Log Message

Cherry-pick r230101. rdar://problem/39155394

    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/_javascript_Core:

    <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):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230101 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-605-branch/JSTests/ChangeLog (230237 => 230238)


--- branches/safari-605-branch/JSTests/ChangeLog	2018-04-04 03:27:30 UTC (rev 230237)
+++ branches/safari-605-branch/JSTests/ChangeLog	2018-04-04 03:27:33 UTC (rev 230238)
@@ -1,5 +1,48 @@
 2018-04-03  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r230101. rdar://problem/39155394
+
+    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/_javascript_Core:
+    
+    <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):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230101 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-03-30  Robin Morisset  <rmoris...@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-04-03  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r230026. rdar://problem/39155085
 
     appendQuotedJSONString stops on arithmetic overflow instead of propagating it upwards

Added: branches/safari-605-branch/JSTests/stress/large-unshift-splice.js (0 => 230238)


--- branches/safari-605-branch/JSTests/stress/large-unshift-splice.js	                        (rev 0)
+++ branches/safari-605-branch/JSTests/stress/large-unshift-splice.js	2018-04-04 03:27:33 UTC (rev 230238)
@@ -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);

Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (230237 => 230238)


--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-04-04 03:27:30 UTC (rev 230237)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-04-04 03:27:33 UTC (rev 230238)
@@ -1,5 +1,58 @@
 2018-04-03  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r230101. rdar://problem/39155394
+
+    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/_javascript_Core:
+    
+    <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):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230101 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-03-30  Robin Morisset  <rmoris...@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-04-03  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r230026. rdar://problem/39155085
 
     appendQuotedJSONString stops on arithmetic overflow instead of propagating it upwards

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/ArrayPrototype.cpp (230237 => 230238)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2018-04-04 03:27:30 UTC (rev 230237)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2018-04-04 03:27:33 UTC (rev 230238)
@@ -346,7 +346,7 @@
     RELEASE_ASSERT(currentCount <= (length - header));
 
     // Guard against overflow.
-    if (count > (UINT_MAX - length)) {
+    if (count > UINT_MAX - length) {
         throwOutOfMemoryError(exec, scope);
         return;
     }

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/JSArray.cpp (230237 => 230238)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSArray.cpp	2018-04-04 03:27:30 UTC (rev 230237)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSArray.cpp	2018-04-04 03:27:33 UTC (rev 230238)
@@ -1060,10 +1060,13 @@
             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 @@
             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();
         

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/JSObject.h (230237 => 230238)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSObject.h	2018-04-04 03:27:30 UTC (rev 230237)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSObject.h	2018-04-04 03:27:33 UTC (rev 230238)
@@ -976,7 +976,7 @@
     // 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) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to