1904649 - Assertion failure: false, at js/src/vm/SelfHosting.cpp:436

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Nils Bars]

Attachment: bug.js

Steps to reproduce:

Checkout commit 9fcc11127fbfbdc88cbf37489dac90542e141c77 and invoke the js shell as follows:

js --fuzzing-safe <test-case>


Actual results:

Assertion failure: false, at js/src/vm/SelfHosting.cpp:436

Comment 1

[Jan de Mooij [:jandem]]

Self-hosted JavaScript assertion info: ./../js/src/builtin/TypedArray.js:810: unexpected missing element

Reduced test below. Glancing at the code, these lines in resizableTypedArrayElementShiftBy look suspicious because both branchPtr calls have the same condition.

var buf = new SharedArrayBuffer(51, {maxByteLength:51});
new Float16Array(buf).lastIndexOf();

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1833647

Comment 3

[André Bargull [:anba]]

Taking because it blocks bug 1835034, for which I've started some initial patches.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44c41a7dfb0b
Part 1: Jump not branch for float16. r=jandem
https://hg.mozilla.org/integration/autoland/rev/4247c85f7948
Part 2: Add Float16Array to existing tests. r=jandem

Comment 5

[Andrew McCreight [:mccr8]]

What are the security implications of this problem? Security issues that affect more than Nightly need sec-approval before landing, if they are worse than sec-moderate. This bug does not even have a rating yet. Thanks.

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1904649 - Part 1: Jump not branch for float16. r=jandem!

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1904649 - Part 2: Add Float16Array to existing tests. r=jandem!

Depends on D214922

Comment 9

[André Bargull [:anba]]

Float16Array isn't available outside of Nightly, see also bug 1903329, which removed the Nightly-only restriction and enabled Float16Array by default.

Comment 10

[Andrew McCreight [:mccr8]]

Thanks for the clarification.

Comment 11

[Andrew McCreight [:mccr8]]

This still needs a security rating, if only for possible bug bounty consideration, so if somebody could say what the security implications are it would be appreciated.

Comment 12

[André Bargull [:anba]]

I don't see any obvious sec-issues with this bug:

• This issue only affects length-tracking Float16Array with a growable SharedArrayBuffer.
• When tracking the length, the byte length of the underlying growable SharedArrayBuffer is read and then divided by the TypedArray's BYTES_PER_ELEMENT.
• Before the patch, the SharedArrayBuffer's byte length was divided by 8, but the correct divisor should have been 2.
• That means a too small length value was reported.

The overall affected operations are:

• The in operator. This led to reporting that an element is absent even though it's actually present. (The in operator with an indexed operand is compiled as index < typedArray.length.)
• The TypedArray.prototype.length and TypedArray.prototype.byteLength accessor properties. This led to reporting a too small length resp. byte-length value.
• The self-hosting intrinsic functions TypedArrayLength and PossiblyWrappedTypedArrayLength. This is probably the most interesting case, but after checking all callers to these two functions, I didn't see any obvious issue when reporting a too small length value.

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/44c41a7dfb0b
https://hg.mozilla.org/mozilla-central/rev/4247c85f7948

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1833647