1754892 - Crash in [@ js::ModuleObject::getCycleRoot]

Status: NEW

(Core :: JavaScript Engine, defect, P2)

Description

[Andrew McCreight [:mccr8]]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/9b0785a4-80d7-4310-89e7-036c90220210

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(cycleRoot.isObject())

Top 10 frames of crashing thread:

0 xul.dll js::ModuleObject::getCycleRoot const js/src/builtin/ModuleObject.cpp:1129
1 xul.dll intrinsic_GetCycleRoot js/src/vm/SelfHosting.cpp:1904
2 xul.dll Interpret js/src/vm/Interpreter.cpp:3309
3 xul.dll js::Call js/src/vm/Interpreter.cpp:589
4 xul.dll js::CallSelfHostedFunction js/src/vm/SelfHosting.cpp:1586
5 xul.dll static js::ModuleObject::GatherAsyncParentCompletions js/src/builtin/ModuleObject.cpp:2233
6 xul.dll js::AsyncModuleExecutionFulfilled js/src/builtin/ModuleObject.cpp:2260
7 xul.dll js::AsyncModuleExecutionFulfilledHandler js/src/builtin/ModuleObject.cpp:2206
8 xul.dll js::Call js/src/vm/Interpreter.cpp:589
9 xul.dll PromiseReactionJob js/src/builtin/Promise.cpp:2067

I'm not sure what the deal with these crashes is. The URLs indicate somebody running a page locally.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Hmm. Could this just be us retrieving the cycleRoot before it's set?

Jon?

Comment 2

[Jon Coppeard (:jonco)]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #1)

Hmm. Could this just be us retrieving the cycleRoot before it's set?

Looks like cycle root is set when we transition to the MODULE_STATUS_EVALUATED state:

https://searchfox.org/mozilla-central/source/js/src/builtin/Module.js#739-740

(In reply to Andrew McCreight [:mccr8] from comment #0)

The URLs indicate somebody running a page locally.

Is this someone doing local testing during development?

Comment 3

[Nicolas B. Pierron [:nbp]]

Crash-stat reports are hinting toward a single person attempting to minimize/exploit a test case which reproduces this issue really fast.
The hints are few unique pointers across crashes (Address Randomization disabled), time stamps are temporarily close-by, a single locale.

We do not yet know whether this signature is exploitable, but it is definitely being investigated.

Comment 4

[Andrew McCreight [:mccr8]]

The URLs are along the lines of http://localhost:3000/ for what it is worth.

Comment 5

[Daniel Veditz [:dveditz]]

Steve: not entirely sure what to do with this one. The breakpoint itself seems to catch badness, but the pattern of crashes seems to indicate intent to find worse.

Comment 6

[Steven DeTar [:sdetar]]

Jan do have thoughts what to do with this bug (based on comments above)?

Comment 7

[Jan de Mooij [:jandem]]

I've been looking into this but I don't see anything actionable.

decoder, fuzzing is able to generate/mutate modules that use top-level await like this, right?

Comment 8

[Jon Coppeard (:jonco)]

This is a release assert failure and is not a security vulnerability.

Comment 9

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

Since the crash volume is low (less than 5 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.

Comment 10

[Christian Holler (:decoder)]

(In reply to Jan de Mooij [:jandem] from comment #7)

I've been looking into this but I don't see anything actionable.

decoder, fuzzing is able to generate/mutate modules that use top-level await like this, right?

Yes, although the whole async module testing code is likely not working as well as the regular JS testing in general. I believe there are also shell limitations in terms of what can be tested with toplevel await.