Changeset 230101 in webkit


Ignore:
Timestamp:
Mar 30, 2018 5:05:34 AM (6 years ago)
Author:
rmorisset@apple.com
Message:

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

Location:
trunk
Files:
1 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r230026 r230101  
     12018-03-30  Robin Morisset  <rmorisset@apple.com>
     2
     3        Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
     4        https://bugs.webkit.org/show_bug.cgi?id=183657
     5
     6        Reviewed by Keith Miller.
     7
     8        * stress/large-unshift-splice.js: Added.
     9        (make_contig_arr):
     10
    1112018-03-28  Robin Morisset  <rmorisset@apple.com>
    212
  • trunk/Source/JavaScriptCore/ChangeLog

    r230098 r230101  
     12018-03-30  Robin Morisset  <rmorisset@apple.com>
     2
     3        Out-of-bounds accesses due to a missing check for MAX_STORAGE_VECTOR_LENGTH in unshiftCountForAnyIndexingType
     4        https://bugs.webkit.org/show_bug.cgi?id=183657
     5        <rdar://problem/38464399>
     6
     7        Reviewed by Keith Miller.
     8
     9        There was just a missing check in unshiftCountForIndexingType.
     10        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',
     11        and the slow path has an assert that there is no unhandled exception (line 360 of ArrayPrototype.cpp).
     12        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.
     13
     14        * runtime/ArrayPrototype.cpp:
     15        (JSC::unshift):
     16        * runtime/JSArray.cpp:
     17        (JSC::JSArray::unshiftCountWithAnyIndexingType):
     18        * runtime/JSObject.h:
     19        (JSC::JSObject::ensureLength):
     20
    1212018-03-29  Mark Lam  <mark.lam@apple.com>
    222
  • trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

    r229850 r230101  
    349349
    350350    // Guard against overflow.
    351     if (count > (UINT_MAX - length)) {
     351    if (count > UINT_MAX - length) {
    352352        throwOutOfMemoryError(exec, scope);
    353353        return;
  • trunk/Source/JavaScriptCore/runtime/JSArray.cpp

    r228576 r230101  
    10611061            return unshiftCountWithArrayStorage(exec, startIndex, count, ensureArrayStorage(vm));
    10621062        }
    1063        
     1063
     1064        if (oldLength + count > MAX_STORAGE_VECTOR_LENGTH)
     1065            return false;
     1066
    10641067        if (!ensureLength(vm, oldLength + count)) {
    10651068            throwOutOfMemoryError(exec, scope);
    1066             return false;
     1069            return true;
    10671070        }
    10681071        butterfly = this->butterfly();
     
    11051108            return unshiftCountWithArrayStorage(exec, startIndex, count, ensureArrayStorage(vm));
    11061109        }
    1107        
     1110
     1111        if (oldLength + count > MAX_STORAGE_VECTOR_LENGTH)
     1112            return false;
     1113
    11081114        if (!ensureLength(vm, oldLength + count)) {
    11091115            throwOutOfMemoryError(exec, scope);
    1110             return false;
     1116            return true;
    11111117        }
    11121118        butterfly = this->butterfly();
  • trunk/Source/JavaScriptCore/runtime/JSObject.h

    r230092 r230101  
    983983    bool WARN_UNUSED_RETURN ensureLength(VM& vm, unsigned length)
    984984    {
    985         ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
     985        RELEASE_ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
    986986        ASSERT(hasContiguous(indexingType()) || hasInt32(indexingType()) || hasDouble(indexingType()) || hasUndecided(indexingType()));
    987987
Note: See TracChangeset for help on using the changeset viewer.