1750116 - Assertion failure: mType == eType_Fallback || mType == eType_Null, at src/dom/base/nsObjectLoadingContent.cpp:2118

Status: ASSIGNED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20220113-cc74f768d0eb (--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

Assertion failure: mType == eType_Fallback || mType == eType_Null, at src/dom/base/nsObjectLoadingContent.cpp:2118

#0 0x7f12cee808ce in nsObjectLoadingContent::ConfigureFallback() src/dom/base/nsObjectLoadingContent.cpp:2118:3
#1 0x7f12cee79d36 in nsObjectLoadingContent::LoadObject(bool, bool, nsIRequest*) src/dom/base/nsObjectLoadingContent.cpp:1735:5
#2 0x7f12cee7782f in nsObjectLoadingContent::OnStartRequest(nsIRequest*) src/dom/base/nsObjectLoadingContent.cpp:639:10
#3 0x7f12cd862c64 in mozilla::net::HttpChannelChild::DoOnStartRequest(nsIRequest*) src/netwerk/protocol/http/HttpChannelChild.cpp:568:20
#4 0x7f12cd862743 in mozilla::net::HttpChannelChild::OnStartRequest(mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, mozilla::net::HttpChannelOnStartRequestArgs const&) src/netwerk/protocol/http/HttpChannelChild.cpp:500:3
#5 0x7f12cda4a27b in mozilla::net::ChannelEventQueue::FlushQueue() src/netwerk/ipc/ChannelEventQueue.cpp:94:12
#6 0x7f12cda7eadc in MaybeFlushQueue /builds/worker/workspace/obj-build/dist/include/mozilla/net/ChannelEventQueue.h:335:5
#7 0x7f12cda7eadc in CompleteResume /builds/worker/workspace/obj-build/dist/include/mozilla/net/ChannelEventQueue.h:314:5
#8 0x7f12cda7eadc in mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run() src/netwerk/ipc/ChannelEventQueue.cpp:152:17
#9 0x7f12cd18ee5e in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:468:16
#10 0x7f12cd168cb6 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:771:26
#11 0x7f12cd167978 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:607:15
#12 0x7f12cd167bf3 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:391:36
#13 0x7f12cd191f09 in operator() src/xpcom/threads/TaskController.cpp:127:37
#14 0x7f12cd191f09 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#15 0x7f12cd17d5b3 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1195:16
#16 0x7f12cd18469a in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:467:10
#17 0x7f12cdc27a44 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:107:5
#18 0x7f12cdb47e17 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
#19 0x7f12cdb47d22 in RunHandler src/ipc/chromium/src/base/message_loop.cc:324:3
#20 0x7f12cdb47d22 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
#21 0x7f12d1dde148 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#22 0x7f12d3e03b03 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:870:20
#23 0x7f12cdc2898a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
#24 0x7f12cdb47e17 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
#25 0x7f12cdb47d22 in RunHandler src/ipc/chromium/src/base/message_loop.cc:324:3
#26 0x7f12cdb47d22 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
#27 0x7f12d3e0313c in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:707:34
#28 0x55ec030ad029 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#29 0x55ec030ad029 in main src/browser/app/nsBrowserApp.cpp:327:18
#30 0x7f12e1eba0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
#31 0x55ec030887bc in _start (/home/worker/builds/m-c-20220113155309-fuzzing-debug/firefox-bin+0x157bc)

Comment 1

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220113215749-afb99f2fbec3.
The bug appears to have been introduced in the following build range:

Start: 71e9852a8a9a9e0b4d6a762dc4ac15ebb861495e (20210406182106)
End: 196e52cf4488228b0fa1ad939de0c021d963c744 (20210406193206)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=71e9852a8a9a9e0b4d6a762dc4ac15ebb861495e&tochange=196e52cf4488228b0fa1ad939de0c021d963c744

Comment 2

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/HnBO79xm0vZ3aJt4wm-FPQ/index.html

Comment 3

[Andrew McCreight [:mccr8]]

David, how much of an issue do you think this is? It looks like a regression from one of your big NPAPI cleanup patches. Thanks.

Comment 4

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

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

Comment 5

[David Parks [:handyman]]

This isn't that urgent -- it should not be exploitable. From the code, it looks like this doesn't have a way to change mType after this but the fuzzer finds a way. Pernosco shows that the asserting stack is:

__libc_start_main () at libc-start.c:310
main () at nsBrowserApp.cpp:327content_process_main () at plugin-container.cpp:57
::BootstrapImpl::XRE_InitChildProcess at Bootstrap.cpp:67XRE_InitChildProcess () at nsEmbedFunctions.cpp:701
Run () at message_loop.cc:306RunHandler () at message_loop.cc:324
RunInternal () at message_loop.cc:331Run () at MessagePump.cpp:235
XRE_RunAppShell () at nsEmbedFunctions.cpp:864
nsBaseAppShell::Run at nsBaseAppShell.cpp:137
Run () at message_loop.cc:306RunHandler () at message_loop.cc:324
RunInternal () at message_loop.cc:331Run () at MessagePump.cpp:268
Run () at MessagePump.cpp:85
NS_ProcessNextEvent () at nsThreadUtils.cpp:467
ProcessNextEvent () at nsThread.cpp:1183
Run () at nsThreadUtils.h:531operator() () at TaskController.cpp:124
ProcessPendingMTTask () at TaskController.cpp:391
ExecuteNextTaskOnlyMainThreadInternal () at TaskController.cpp:607
DoExecuteNextTaskOnlyMainThreadInternal () at TaskController.cpp:771
Run () at TaskController.cpp:468
Run () at NonBlockingAsyncInputStream.cpp:33
RunAsyncWaitCallback () at NonBlockingAsyncInputStream.cpp:398
{virtual override thunk({offset(-8)}, nsInputStreamPump::OnInputStreamReady)}OnInputStreamReady () at nsInputStreamPump.cpp:374
OnStateStart () at nsInputStreamPump.cpp:465
{virtual override thunk({offset(-144)}, nsBaseChannel::OnStartRequest)}OnStartRequest () at nsBaseChannel.cpp:819
/home/twsmith/workspace/browsers/m-c-20220104214425-fuzzing-debug/libxul.so+28331e0 () at nsObjectLoadingContent.cpp:0
OnStartRequest () at nsObjectLoadingContent.cpp:743
OnStartRequest () at nsObjectLoadingContent.cpp:639
LoadObject () at nsObjectLoadingContent.cpp:1735
ConfigureFallback () at nsObjectLoadingContent.cpp:2118

and the change that causes the assert to trigger by changing mType to eType_Loading is

__libc_start_main () at libc-start.c:310
main () at nsBrowserApp.cpp:327
content_process_main () at plugin-container.cpp:57
::BootstrapImpl::XRE_InitChildProcess at Bootstrap.cpp:67
XRE_InitChildProcess () at nsEmbedFunctions.cpp:701
Run () at message_loop.cc:306
RunHandler () at message_loop.cc:324
RunInternal () at message_loop.cc:331
Run () at MessagePump.cpp:235
XRE_RunAppShell () at nsEmbedFunctions.cpp:864
nsBaseAppShell::Run at nsBaseAppShell.cpp:137
Run () at message_loop.cc:306
RunHandler () at message_loop.cc:324
RunInternal () at message_loop.cc:331
Run () at MessagePump.cpp:268
Run () at MessagePump.cpp:85
NS_ProcessNextEvent () at nsThreadUtils.cpp:467
ProcessNextEvent () at nsThread.cpp:1183
Run () at nsThreadUtils.h:531
operator() () at TaskController.cpp:124
ProcessPendingMTTask () at TaskController.cpp:391
ExecuteNextTaskOnlyMainThreadInternal () at TaskController.cpp:607
DoExecuteNextTaskOnlyMainThreadInternal () at TaskController.cpp:771
Run () at TaskController.cpp:468
Run () at NonBlockingAsyncInputStream.cpp:33
RunAsyncWaitCallback () at NonBlockingAsyncInputStream.cpp:398
{virtual override thunk({offset(-8)}, nsInputStreamPump::OnInputStreamReady)}OnInputStreamReady () at nsInputStreamPump.cpp:374
OnStateStart () at nsInputStreamPump.cpp:465
{virtual override thunk({offset(-144)}, nsBaseChannel::OnStartRequest)}OnStartRequest () at nsBaseChannel.cpp:819
/home/twsmith/workspace/browsers/m-c-20220104214425-fuzzing-debug/libxul.so+28331e0 () at nsObjectLoadingContent.cpp:0
OnStartRequest () at nsObjectLoadingContent.cpp:743
OnStartRequest () at nsObjectLoadingContent.cpp:639
LoadObject () at nsObjectLoadingContent.cpp:1732
UnloadObject () at nsObjectLoadingContent.cpp:1958
nsFrameLoader::Destroy at nsFrameLoader.cpp:1840
StartDestroy () at nsFrameLoader.cpp:1977
FinalizeFrameLoader () at Document.cpp:9206
AddScriptRunner () at nsContentUtils.cpp:5756
AddScriptRunner () at nsContentUtils.cpp:5750
Run () at nsThreadUtils.h:1200
apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> () at nsThreadUtils.h:1153
applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> () at nsThreadUtils.h:1147
MaybeInitializeFinalizeFrameLoaders () at Document.cpp:9249
Run () at nsFrameLoader.cpp:1985
DestroyDocShell () at nsFrameLoader.cpp:2056
Destroy () at nsDocShell.cpp:4447
Stop () at nsDocShell.cpp:4196
Stop () at nsDocShell.h:185
Stop () at nsDocLoader.cpp:258
Cancel () at nsLoadGroup.cpp:240
NotifyRemovalObservers () at nsLoadGroup.cpp:614
{virtual override thunk({offset(-8)}, nsDocShell::OnStopRequest)}OnStopRequest () at nsDocShell.cpp:1354
0OnStopRequest () at nsDocLoader.cpp:677
DocLoaderIsEmpty () at nsDocLoader.cpp:796
NotifyDoneWithOnload () at nsDocLoader.cpp:869
ChildDoneWithOnload () at nsDocLoader.h:228
DocLoaderIsEmpty () at nsDocLoader.cpp:794
doStopDocumentLoad () at nsDocLoader.cpp:975
DoFireOnStateChange () at nsDocLoader.cpp:1377
{virtual override thunk({offset(-448)}, nsDocShell::OnStateChange)}OnStateChange () at nsDocShell.cpp:5667
EndPageLoad () at nsDocShell.cpp:6278
LoadComplete () at nsDocumentViewer.cpp:1086
Dispatch () at EventDispatcher.cpp:1085
HandleEventTargetChain () at EventDispatcher.cpp:550
HandleEvent () at EventDispatcher.cpp:348
HandleEvent () at EventListenerManager.h:395
HandleEventInternal () at EventListenerManager.cpp:1500
HandleEventSubType () at EventListenerManager.cpp:1303
HandleEvent<mozilla::dom::EventTarget *> () at Unified_cpp_dom_events1.cpp:65
HandleEvent () at UnifiedBindings5.cpp:62
Call () at CallAndConstruct.cpp:117
Call () at Interpreter.cpp:589
InternalCall () at Interpreter.cpp:572
InternalCallOrConstruct () at Interpreter.cpp:544
RunScript () at Interpreter.cpp:394
Interpret () at Interpreter.cpp:3064
SetObjectElementOperation () at Interpreter.cpp:1812
SetProperty () at ObjectOperations-inl.h:308
NativeSetProperty<js::Qualified> () at NativeObject.cpp:2527
SetExistingProperty () at NativeObject.cpp:2493
CallSetter () at Interpreter.cpp:730
Call () at Interpreter.cpp:589
InternalCall () at Interpreter.cpp:572
InternalCallOrConstruct () at Interpreter.cpp:512
CallJSNative () at Interpreter.cpp:425
GenericSetter<mozilla::dom::binding_detail::NormalThisPolicy> () at BindingUtils.cpp:3254
set_innerText () at UnifiedBindings7.cpp:342
SetInnerText () at nsGenericHTMLElement.cpp:2932
RemoveChildNode () at nsINode.cpp:2159
UnbindFromTree () at nsGenericHTMLElement.cpp:501
UnbindFromTree () at Element.cpp:2003
::HTMLObjectElement::UnbindFromTree at HTMLObjectElement.cpp:111
nsObjectLoadingContent::UnbindFromTree at nsObjectLoadingContent.cpp:377
UnloadObject () at nsObjectLoadingContent.cpp:1965

So the UnloadObject call triggers a nsFrameLoader::Destroy that eventually does some JS that eventually comes back and calls UnloadObject again. This UnloadObject call has aResetState as true, which causes it to change mType to eType_Loading. When it unwinds back to the first UnloadObject, it then fails the assert.

I don't immediately see the fix but I also don't think this is likely to be a big problem.

Comment 6

[Tyson Smith [:tsmith]]

This assertion is hit by the browser fuzzer fairly frequently. I wouldn't call it a blocker but it is reported multiple times a day.

Comment 7

[Jim Mathies [:jimm]]

Hey Hsin-Ye, we don't have anyone who owns plugin code anymore (technically there shouldn't be any left). Is there someone on your end that can pick this up?

Comment 8

[Tyson Smith [:tsmith]]

This issue currently has this third largest bucket. It has been reported by browser fuzzers 13,000 times.

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Tyson Smith [:tsmith] (PTO until Feb 15) from comment #8)

This issue currently has this third largest bucket. It has been reported by browser fuzzers 13,000 times.

Thanks for the info. Will take this to the team discussion.

Comment 10

[Andreas Farre [:farre]]

Attachment: Bug 1750116 - Unload before setting type to fallback to. r=peterv!

UnloadObject will call nsFrameLoader::Destroy if there is a frame
loader, which will set mType to eType_Loading and
ConfigureFallback will complain.

Comment 11

[Bugmon [:jkratzer for issues]]

Bugmon was unable reproduce this issue.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 12

[Jason Kratzer [:jkratzer]]

A change to the Taskcluster build definitions over the weekend caused Bugmon to fail when reproducing issues. This issue has been corrected. Re-enabling bugmon.