1368844 - WebAssembly.Memory thows assert

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Natalie Silvanovich]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

Ran the following code in a debug build of spidermonkey:

var a =  new WebAssembly.Memory({initial: 59199});



Actual results:

An assert fired: 

Assertion failure: length <= (2147483647), at /home/user/test_everywhere/spidermonkey/js/src/vm/ArrayBufferObject.cpp:895
Segmentation fault (core dumped)


Expected results:

No asserts.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks for opening an issue!

[Tracking Requested - why for this release]: simple way to DOS a browser

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: checkinternal.patch

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 8873076 [details] [diff] [review]
checkinternal.patch

Review of attachment 8873076 [details] [diff] [review]:
-----------------------------------------------------------------

Ooph, thanks.  Could you add release asserts to ArrayBufferObject::createForWasm() that initial/maximum are less than their respective MaxMemory*Pages?

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Yes, I've added the assert for the initial. For maximum, since the maximal value is 2**32 - 1, and the maximum value is an u32, the assert would be tautological.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Once this lands, please request uplift to 54.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/27a355cf6036
Check WebAssembly.{Memory,Table} sizes against internal limits; r=luke

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/27a355cf6036

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: fix.patch

Approval Request Comment
[Feature/Bug causing the regression]: wasm
[User impact if declined]: easy way to DOS firefox
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not much
[Why is the change risky/not risky?]: conservative, small change
[String changes made/needed]: n/a

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Is ESR52 also affected?

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8874837 [details] [diff] [review]
fix.patch

54 RC build is released today. Beta54- and mark 54 won't fix.

Comment 11

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Is ESR52 also affected?

No, wasm is disabled on esr52.