1624886 - Crash in [@ js::wasm::WasmFrameIter::instance]

Status: VERIFIED FIXED

(Core :: JavaScript: WebAssembly, defect, P2)

Description

[Simon Giesecke [:sg] [he/him]]

This bug is for crash report bp-a6b5456b-0329-44a1-8839-619400200319.

Top 10 frames of crashing thread:

0 xul.dll js::wasm::WasmFrameIter::instance const js/src/wasm/WasmFrameIter.cpp:277
1 xul.dll js::SavedStacks::saveCurrentStack js/src/vm/SavedStacks.cpp:1291
2 xul.dll js::CaptureStack js/src/jsexn.cpp:216
3 xul.dll JSContext::setPendingExceptionAndCaptureStack js/src/vm/JSContext.cpp:1469
4 xul.dll js::ThrowOperation js/src/vm/Interpreter.cpp:4351
5 xul.dll trunc 
6  @0xc538f78 
7  @0x1898b137 
8  @0xc538f78 
9  @0x186966ef 

This started occurring with Nightly build id 20200319143354.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Ryan, is this something you can look at?

Comment 2

[Ryan Hunt [:rhunt]]

Yes, will take a look at this.

Comment 3

[Gabi Cheta [:Gabi] Desktop QA]

Issue reproducible on Firefox 76.0b1 with Windows 7x86.
Crash report: https://crash-stats.mozilla.org/report/index/2314772a-e261-45d6-9946-c918a0200407#tab-details
Crash occurred when accessing https://www.crazygames.com/game/fighter-aircraft-pilot.

Comment 4

[[:philipp]]

bug 1620197 & bug 1612534 would have landed in the build identified in comment #0

Comment 5

[Ryan Hunt [:rhunt]]

Yes, this seems likely to be regressed by bug 1620197. Running mozregression gave me this range [1], which includes it.

[1] https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7aa696aefc462bd6a6b50170f04bdad47a0a7e27&tochange=268543e53e1b11ce0e468d985ea3777563e7b8a8

Comment 6

[Ryan Hunt [:rhunt]]

Attachment: Bug 1624886 - Fix argBytes calculation in GenerateImportInterpExit. r?lth

Bug 1620197 accidentally moved the parentheses for 'max' outside of the
multiplication by sizeof(Value).

Comment 7

[Ryan Hunt [:rhunt]]

Found a bug and verified it resolved the repro steps given in comment 3. I will add a proper regression test, but I don't have time to make one tonight.

Comment 9

[Ryan Hunt [:rhunt]]

Comment on attachment 9139018 [details]
Bug 1624886 - Fix argBytes calculation in GenerateImportInterpExit. r?lth

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unknown. The issue causes an incorrect stack frame to be set up on wasm exits to JS, which can cause a crash when a stack iteration is triggered (e.g an exception). It's hard to rule out if user controlled data could be interpreted as part of the stack frame.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: 76
• If not all supported branches, which bug introduced the flaw?: Bug 1620197
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely. This patch reverts an unintentional move of parentheses.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9139018 [details]
Bug 1624886 - Fix argBytes calculation in GenerateImportInterpExit. r?lth

Per Slack discussion with Dan, approved to land and approved for 76.0b3 to fix a topcrash regression.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/7502fc7d616a153f13516aafaa33acdb621222d2

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7502fc7d616a

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/a383b6cf96bd29eae8fbbbf21d41ac18f160460c

Comment 14

[Gabi Cheta [:Gabi] Desktop QA]

Verified as fixed with Firefox 76.0b3 and Nightly 77.0a1 on Windows 7x86.