1785109 - Hit MOZ_CRASH(JS holder CustomElementRegistry contains pointers to GC things in more than one zone (found in mConstructors key) ) at /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1352

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20220815-1c3c5be93f58 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Hit MOZ_CRASH(JS holder CustomElementRegistry contains pointers to GC things in more than one zone (found in mConstructors key) ) at /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1352

#0 0x7fb418016f53 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:261:3
#1 0x7fb418016f53 in checkZone /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1349:5
#2 0x7fb418016f53 in CheckZoneTracer::Trace(JS::Heap<JSObject*>*, char const*, void*) const /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1373:7
#3 0x7fb419a09042 in mozilla::dom::CustomElementRegistry::cycleCollection::Trace(void*, TraceCallbacks const&, void*) /builds/worker/checkouts/gecko/dom/base/CustomElementRegistry.cpp:413:16
#4 0x7fb41800c26a in CheckHolderIsSingleZone /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1418:17
#5 0x7fb41800c26a in mozilla::CycleCollectedJSRuntime::TraceJSHolders(JSTracer*, mozilla::JSHolderMap::Iter&, js::SliceBudget&) /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1478:7
#6 0x7fb41800bac3 in mozilla::CycleCollectedJSRuntime::TraceNativeGrayRoots(JSTracer*, mozilla::JSHolderMap::WhichHolders, js::SliceBudget&) /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSRuntime.cpp:1459:19
#7 0x7fb41fb2d01b in traceEmbeddingGrayRoots /builds/worker/checkouts/gecko/js/src/gc/RootMarking.cpp:403:10
#8 0x7fb41fb2d01b in js::gc::IncrementalProgress js::gc::GCRuntime::markGrayRoots<js::gc::SweepGroupZonesIter>(js::SliceBudget&, js::gcstats::PhaseKind) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:549:7
#9 0x7fb41fb2cee4 in js::gc::GCRuntime::markGrayRootsInCurrentGroup(JS::GCContext*, js::SliceBudget&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:1087:10
#10 0x7fb41fb4e350 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:2089:23
#11 0x7fb41fb44bf8 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:2124:19
#12 0x7fb41fb34516 in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:2261:53
#13 0x7fb41faa4d43 in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, JS::GCReason, bool) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:3315:11
#14 0x7fb41faa873a in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget const&, JS::GCReason) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:3819:3
#15 0x7fb41faa9a05 in js::gc::GCRuntime::collect(bool, js::SliceBudget const&, JS::GCReason) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:4007:9
#16 0x7fb41fa7c547 in js::gc::GCRuntime::gc(JS::GCOptions, JS::GCReason) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:4084:3
#17 0x7fb41fac6a8b in JS::NonIncrementalGC(JSContext*, JS::GCOptions, JS::GCReason) /builds/worker/checkouts/gecko/js/src/gc/GCAPI.cpp:289:21
#18 0x7fb419d7cc04 in GarbageCollectImpl(JS::GCReason, nsJSContext::IsShrinking, js::SliceBudget const&) /builds/worker/checkouts/gecko/dom/base/nsJSEnvironment.cpp:1070:5
#19 0x7fb419d7ca2a in nsJSContext::GarbageCollectNow(JS::GCReason, nsJSContext::IsShrinking) /builds/worker/checkouts/gecko/dom/base/nsJSEnvironment.cpp:1077:3
#20 0x7fb41af2dc98 in mozilla::dom::FuzzingFunctions_Binding::garbageCollect(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/obj-build/dom/bindings/FuzzingFunctionsBinding.cpp:52:3
#21 0x7fb42064f0b7 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:455:13
#22 0x7fb42064e971 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:543:12
#23 0x7fb420645ea5 in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:615:10
#24 0x7fb420645ea5 in Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3371:16
#25 0x7fb42063d302 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:427:13
#26 0x7fb4206513c6 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:821:13
#27 0x7fb4206517db in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:853:10
#28 0x7fb41f308d8e in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CompilationAndEvaluation.cpp:518:10
#29 0x7fb41f308f9e in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) /builds/worker/checkouts/gecko/js/src/vm/CompilationAndEvaluation.cpp:542:10
#30 0x7fb419bc251a in mozilla::dom::JSExecutionContext::ExecScript() /builds/worker/checkouts/gecko/dom/base/JSExecutionContext.cpp:296:8
#31 0x7fb41ccaad63 in ExecuteCompiledScript /builds/worker/checkouts/gecko/dom/script/ScriptLoader.cpp:2138:16
#32 0x7fb41ccaad63 in mozilla::dom::ScriptLoader::EvaluateScript(nsIGlobalObject*, JS::loader::ScriptLoadRequest*) /builds/worker/checkouts/gecko/dom/script/ScriptLoader.cpp:2401:12
#33 0x7fb41cca9fad in mozilla::dom::ScriptLoader::EvaluateScriptElement(JS::loader::ScriptLoadRequest*) /builds/worker/checkouts/gecko/dom/script/ScriptLoader.cpp:2207:10
#34 0x7fb41cca68c4 in mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*) /builds/worker/checkouts/gecko/dom/script/ScriptLoader.cpp:1857:10
#35 0x7fb41cca4e71 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, JS::loader::ScriptKind) /builds/worker/checkouts/gecko/dom/script/ScriptLoader.cpp:1269:10
#36 0x7fb41cc9bae7 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/worker/checkouts/gecko/dom/script/ScriptLoader.cpp:910:10
#37 0x7fb41cc9b67c in mozilla::dom::ScriptElement::MaybeProcessScript() /builds/worker/checkouts/gecko/dom/script/ScriptElement.cpp:118:18
#38 0x7fb4191301a5 in AttemptToExecute /builds/worker/workspace/obj-build/dist/include/nsIScriptElement.h:221:18
#39 0x7fb4191301a5 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOpExecutor.cpp:942:22
#40 0x7fb41912df0d in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOpExecutor.cpp:733:7
#41 0x7fb4191357b1 in nsHtml5ExecutorReflusher::Run() /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOpExecutor.cpp:79:16
#42 0x7fb418106602 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/checkouts/gecko/xpcom/threads/SchedulerGroup.cpp:140:20
#43 0x7fb41813833e in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:538:16
#44 0x7fb4181109e9 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:851:26
#45 0x7fb41810f573 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:683:15
#46 0x7fb41810f7e3 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:461:36
#47 0x7fb41813bb96 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:187:37
#48 0x7fb41813bb96 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#49 0x7fb4181254af in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1205:16
#50 0x7fb41812babd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:465:10
#51 0x7fb418d02776 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#52 0x7fb418c27e57 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
#53 0x7fb418c27d62 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
#54 0x7fb418c27d62 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
#55 0x7fb41cf127c8 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:150:27
#56 0x7fb41f02576b in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:893:20
#57 0x7fb418d0366a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#58 0x7fb418c27e57 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
#59 0x7fb418c27d62 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
#60 0x7fb418c27d62 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
#61 0x7fb41f024c83 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:752:34
#62 0x55596d639429 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#63 0x55596d639429 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:362:18
#64 0x7fb430413082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
#65 0x55596d60f1cc in _start (/home/worker/builds/m-c-20220815214739-fuzzing-debug/firefox-bin+0x161cc) (BuildId: 772afb885e205e9982df9074704aafa27fbaf1c0)

Comment 1

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220816160129-90f3275f3489.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: 09aa8d80251ab46ad827b027d5676143a3af41f1 (20210818115204)
End: 1c3c5be93f5897f3265ec5eeef08c09cb09fb774 (20220815214739)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I think my patch for bug 1777574 is close to being ready for review but this bug shows that there are issues also outside Nightly.
So we may need to spot fix this on beta/esr.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

hmm, the testcase needs to be opened in a certain way so that window.opener is non-null

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1785109, mark CustomElementRegistry as multizone holder, r=mccr8

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9290266 [details]
Bug 1785109, mark CustomElementRegistry as multizone holder, r=mccr8

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Crashing because of accessing a JS::Heap value which has been already GCed might not be too hard.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: The same rather trivial patch should work everywhere.
• How likely is this patch to cause regressions; how much testing does it need?: Very unlike to cause regressions. This basically disables one GC related optimization for this one class.
• Is Android affected?: Yes

Comment 6

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

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:smaug, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9290266 [details]
Bug 1785109, mark CustomElementRegistry as multizone holder, r=mccr8

Approved to land and uplift

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9290266 [details]
Bug 1785109, mark CustomElementRegistry as multizone holder, r=mccr8

Beta/Release Uplift Approval Request

• User impact if declined: Possible security sensitive access garbage collected JS value.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: (This is a bit tricky to test. I'll do it once the patch lands to Nightly)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It disables one optimization
• String changes made/needed: NA
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/678444ca62ed

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9290266 [details]
Bug 1785109, mark CustomElementRegistry as multizone holder, r=mccr8

Approved for 105.0b2 and 102.3esr.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/72f185cf2fe0
https://hg.mozilla.org/releases/mozilla-esr102/rev/e13f3bfc9576

Comment 12

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20220823024657-95412593f3f1.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.