Bug 170264 – REGRESSION (Safari 10.1): Inserting elements into arrays fails when array contains very large numbers

• Component: JavaScriptCore
• Status: RESOLVED FIXED

The following code doesn't behave as expected on the latest version of Safari (10.1+) and on the WebKit nightly builds:

var arr = [0, 2147483648]; // NOTE: the second number is greater than a signed 32bit int
arr.shift(); // remove the first element so arr is [2147483648]
arr[1] = 1; // Safari fails to add the new element and the array is unchanged

On all other browsers and Safari 10.0, arr is [2147483648, 1]. On Safari 10.1 and newer, arr is [2147483648].

The above code works fine if the numbers in the array are less than 2147483648.

[Radar WebKit Bug Importer]

<rdar://problem/31375593>

[Michael Saboff]

This is fixed with change set r214714 <https://trac.webkit.org/changeset/214714> and is a duplicate of <https://bugs.webkit.org/show_bug.cgi?id=164412>

*** This bug has been marked as a duplicate of bug 164412 ***

[MikeM]

Is this really a duplicate just because the fix of bug 164412 fixes this also.

This was an appalling bug which demonstrates the inadequacy of the test suite.

[Saam Barati]

(In reply to MikeM from comment #3)
> Is this really a duplicate just because the fix of bug 164412 fixes this
> also.
Yes this is really the same bug. Note that a number larger than INT_MAX is stored using double representation.
> 
> This was an appalling bug which demonstrates the inadequacy of the test
> suite.
WebKit is an open source project. You're more than welcome to add tests to help make the software better.

[MikeM]

This should be marked as P1 as it's a serious security issue.

For example, crypto libraries must be considered broken. Safari needs to release an immediate fix, surely.

[Alexey Proskuryakov]

That would be a serious issue indeed. Do you have examples of broken websites or apps?

[MikeM]

Well, it breaks my bignumber library which is downloaded from npm by about a million users a month.

https://github.com/MikeMcl/bignumber.js/issues/120
https://www.npmjs.com/package/bignumber.js

[Saam Barati]

gonna add this as a test.

[Saam Barati]

Created attachment 306199 [details]
patch for test

[Saam Barati]

Created attachment 306201 [details]
patch with test

We also ran into this as users of https://www.npmjs.com/package/bignumber.js

This was painful to debug as the tab loading the library would hang with both the page content and developer tools becoming unresponsive so tracking down the source of the problem was not easy.

This mode of failure is also a complete blocker for any end user of the site in question. Rather than just encountering javascript errors any invocation of this behavior renders the entire page unusable.

As one consumer of bignumber.js we're temporarily disabling features to protect 10^5 users/month from being unable to use our product.

[Michael Saboff]

Landed a test similar to what Saam posted with the exception that it loops to tier up.  This test landed change set r214977 <https://trac.webkit.org/changeset/214977/webkit>