Changeset 230605 in webkit

Timestamp:

Apr 12, 2018 4:21:46 PM (6 years ago)

Author:

Kocsen Chung

Message:

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

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

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

Location:

branches/safari-605.1.33.0-branch

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/large-unshift-splice.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/ArrayPrototype.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSArray.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/JSObject.h (modified) (1 diff)

branches/safari-605.1.33.0-branch/JSTests/ChangeLog

@@ +1 @@
+2018-04-12  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Cherry-pick r230101. rdar://problem/39355291
+
+    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):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230101 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-02-21  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605.1.33.0-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-04-12  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Cherry-pick r230101. rdar://problem/39355291
+
+    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):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230101 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-02-28  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605.1.33.0-branch/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -347 +347 @@
 
     // Guard against overflow.
-    if (count > (UINT_MAX - length)) {
+    if (count > UINT_MAX - length) {
         throwOutOfMemoryError(exec, scope);
         return;

branches/safari-605.1.33.0-branch/Source/JavaScriptCore/runtime/JSArray.cpp

@@ -1061 +1061 @@
             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();
@@ -1105 +1108 @@
             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();

branches/safari-605.1.33.0-branch/Source/JavaScriptCore/runtime/JSObject.h

@@ -977 +977 @@
     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()));