Bug 62405 – Fix integer overflow in Array.prototype.push

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

There are three integer overflows here, leading to safe (not a security risk) but incorrect (non-spec-compliant) behaviour.

Two overflows occur when calculating the new length after pushing (one in the fast version of push in JSArray, one in the generic version in ArrayPrototype).
The other occurs calculating indices to write to when multiple items are pushed.

These errors result in three test-262 failures.

[Gavin Barraclough]

Created attachment 96660 [details]
The patch

[Gavin Barraclough]

Created attachment 96661 [details]
Patch with updated LayoutTest results.

[WebKit Review Bot]

Attachment 96660 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 96661 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/sput..." exit_code: 1

Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Oliver Hunt]

Comment on attachment 96661 [details]
Patch with updated LayoutTest results.

r=me

[Geoffrey Garen]

r=me

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:410
> +            Identifier propertyName(exec, JSValue((int64_t)length + (int64_t)n).toString(exec));

static_cast, please.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:414
> +    JSValue newLength = jsNumber((int64_t)length + (int64_t)exec->argumentCount());

Ditto.

[Gavin Barraclough]

Fixed in r88503

[Gavin Barraclough]

*** Bug 55033 has been marked as a duplicate of this bug. ***