Found while fuzzing m-c 20211229-649a1395ee6c (--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 --xvfb

Assertion failure: mBatching.mCounter > 0 (Bad mBatching.mCounter), at src/layout/generic/nsFrameSelection.cpp:2154

#0 0x7f5b92d0aafc in nsFrameSelection::EndBatchChanges(short) src/layout/generic/nsFrameSelection.cpp:2154:3
#1 0x7f5b8fb96155 in mozilla::dom::Selection::EndBatchChanges(short) src/dom/base/Selection.cpp:3197:21
#2 0x7f5b92959a1a in ~AutoUpdateViewBatch src/editor/libeditor/EditorBase.h:2673:34
#3 0x7f5b92959a1a in mozilla::EditorBase::UndoAsAction(unsigned int, nsIPrincipal*) src/editor/libeditor/EditorBase.cpp:1054:1
#4 0x7f5b92975a38 in mozilla::UndoCommand::DoCommand(mozilla::Command, mozilla::EditorBase&, nsIPrincipal*) const src/editor/libeditor/EditorCommands.cpp:277:29
#5 0x7f5b8fa27e04 in mozilla::dom::Document::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) src/dom/base/Document.cpp:5414:37
#6 0x7f5b90c31823 in mozilla::dom::Document_Binding::execCommand(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/DocumentBinding.cpp:3772:36
#7 0x7f5b90fb1588 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3306:13
#8 0x7f5b94a11d4f in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:425:13
#9 0x7f5b94a1144d in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:512:12
#10 0x7f5b94a12f2e in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:572:10
#11 0x7f5b94a08736 in CallFromStack src/js/src/vm/Interpreter.cpp:576:10
#12 0x7f5b94a08736 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3309:16
#13 0x7f5b949ff653 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:394:13
#14 0x7f5b94a11348 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:544:13
#15 0x7f5b94a12f2e in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:572:10
#16 0x7f5b94a13131 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:589:8
#17 0x7f5b94bd2541 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/vm/CallAndConstruct.cpp:117:10
#18 0x7f5b90cc536c in mozilla::dom::EventHandlerNonNull::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/EventHandlerBinding.cpp:283:37
#19 0x7f5b91499a19 in void mozilla::dom::EventHandlerNonNull::Call<nsCOMPtr<mozilla::dom::EventTarget> >(nsCOMPtr<mozilla::dom::EventTarget> const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventHandlerBinding.h:365:12
#20 0x7f5b91498bcd in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) src/dom/events/JSEventHandler.cpp:201:12
#21 0x7f5b91479c7b in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1309:22
#22 0x7f5b9147a939 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1500:17
#23 0x7f5b9146f964 in HandleEvent src/dom/events/EventListenerManager.h:395:5
#24 0x7f5b9146f964 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:348:17
#25 0x7f5b9146ee87 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:550:16
#26 0x7f5b914716e8 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1085:11
#27 0x7f5b92c1d433 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1086:7
#28 0x7f5b9421f074 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:6278:20
#29 0x7f5b9421eb63 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:5667:7
#30 0x7f5b9421f9ff in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
#31 0x7f5b8f088f2c in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1377:3
#32 0x7f5b8f0884ba in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:975:14
#33 0x7f5b8f086840 in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) src/uriloader/base/nsDocLoader.cpp:794:9
#34 0x7f5b8f08869c in ChildDoneWithOnload /builds/worker/workspace/obj-build/dist/include/nsDocLoader.h:228:5
#35 0x7f5b8f08869c in nsDocLoader::NotifyDoneWithOnload(nsDocLoader*) src/uriloader/base/nsDocLoader.cpp:869:14
#36 0x7f5b8f08684b in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) src/uriloader/base/nsDocLoader.cpp:796:9
#37 0x7f5b8f0879fd in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:677:5
#38 0x7f5b942404cd in nsDocShell::OnStopRequest(nsIRequest*, nsresult) src/docshell/base/nsDocShell.cpp:13540:23
#39 0x7f5b8de00aca in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) src/netwerk/base/nsLoadGroup.cpp:614:22
#40 0x7f5b8de020b3 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:518:10
#41 0x7f5b8fa54fc5 in mozilla::dom::Document::DoUnblockOnload() src/dom/base/Document.cpp:11554:18
#42 0x7f5b8fa1fc53 in mozilla::dom::Document::UnblockOnload(bool) src/dom/base/Document.cpp:11484:9
#43 0x7f5b8fa3b9eb in mozilla::dom::Document::DispatchContentLoadedEvents() src/dom/base/Document.cpp:7999:3
#44 0x7f5b8faecc7b in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1147:12
#45 0x7f5b8faecc7b in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1153:12
#46 0x7f5b8faecc7b in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1200:13
#47 0x7f5b8dc07402 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:144:20
#48 0x7f5b8dc36d1e in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:468:16
#49 0x7f5b8dc109b6 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:771:26
#50 0x7f5b8dc0f678 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:607:15
#51 0x7f5b8dc0f8f3 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:391:36
#52 0x7f5b8dc3a386 in operator() src/xpcom/threads/TaskController.cpp:124:37
#53 0x7f5b8dc3a386 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#54 0x7f5b8dc252f3 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1183:16
#55 0x7f5b8dc2c55a in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:467:10
#56 0x7f5b8e6ceea6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:85:21
#57 0x7f5b8e5ee387 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
#58 0x7f5b8e5ee292 in RunHandler src/ipc/chromium/src/base/message_loop.cc:324:3
#59 0x7f5b8e5ee292 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
#60 0x7f5b9286b0a8 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#61 0x7f5b948958d3 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:864:20
#62 0x7f5b8e6cfd9a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
#63 0x7f5b8e5ee387 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
#64 0x7f5b8e5ee292 in RunHandler src/ipc/chromium/src/base/message_loop.cc:324:3
#65 0x7f5b8e5ee292 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
#66 0x7f5b94894f0b in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:701:34
#67 0x55e726029029 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#68 0x55e726029029 in main src/browser/app/nsBrowserApp.cpp:327:18
#69 0x7f5ba42b80b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
#70 0x55e7260047bc in _start (/home/worker/builds/m-c-20211229214859-fuzzing-debug/firefox-bin+0x157bc)

A Pernosco session is available here: https://pernos.co/debug/EJOvxoJZ94IkbU7pMVRLww/index.html

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

Start: 768e04aaea528ec9a0af31c49708cf73ad505a2a (20210324040732)
End: d69774c978c67130b12313703d125ffd80f65483 (20210324041713)
Pushlog: https://hg.mozilla.org/mozilla-unified/pushloghtml?fromchange=768e04aaea528ec9a0af31c49708cf73ad505a2a&tochange=d69774c978c67130b12313703d125ffd80f65483

[Child 33572: Main Thread]: I/SelectionBatch 1f99b5131f0 nsFrameSelection::StartBatchChanges(UndoAsAction)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::StartBatchChanges(DoTransactionInternal)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::EndBatchChanges  (DoTransactionInternal, NO_REASON)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::StartBatchChanges(SetValueWithTextEditor)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::EndBatchChanges  (SetValueWithTextEditor, NO_REASON)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::StartBatchChanges(SetStartAndEndInternal)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::EndBatchChanges  (SetStartAndEndInternal, NO_REASON)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::StartBatchChanges(SetStartAndEndInternal)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30 nsFrameSelection::EndBatchChanges  (SetStartAndEndInternal, NO_REASON)
[Child 33572: Main Thread]: I/SelectionBatch 1f99b512e30nsFrameSelection::EndBatchChanges  (UndoAsAction, NO_REASON)

nsFrameSelection instance is changed in this timing:
https://searchfox.org/mozilla-central/rev/e580b1098c1f012880368989f6e386812fe6ccc6/editor/libeditor/EditorBase.cpp#1019,1040

Selecton::mFrameSelection is set only by the constructor (except the case cleared by the CC). And EditorBase::SelectionRef() should return same Selection instance while EditActionDataSetter is alive:
https://searchfox.org/mozilla-central/rev/e580b1098c1f012880368989f6e386812fe6ccc6/editor/libeditor/EditorBase.cpp#1011

However, there is an exception. If TextEditor is recreated by a reframing, Selection is updated.
https://searchfox.org/mozilla-central/rev/e580b1098c1f012880368989f6e386812fe6ccc6/editor/libeditor/EditorBase.h#1307-1317

Oh, this is potentially a security bug.

If the selection object is updated while an selection's API is used, the object could be broken before selection does all things. This does not affect to HTMLEditor, but affects to TextEditor both <input> and <textarea>.

This is a regression of bug 1518002 (65+).

Regressed by: 1518002

Oh, is it okay to expose a security bug number to another public bug?

When Selection instance is updated, the old selection may be in batch.
In the case, UpdateSelectionCache should clean up the batch in the old
selection and start new one in the new selection instead.

This was required for debugging the bug.

Depends on D139349

Only attachment 9264922 [details] should be uplift. It can be grafted cleanly in ESR91 branch.

Comment on attachment 9264922 [details]
Bug 1753508 - Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: This patch includes 2 fixes, one is the life time bug of Selection (security bug), and the other is selection batch management (normal bug). This may make the other developers confused. And it must be unclear that whether the issue occurs with TextEditor or HTMLEditor or both (it's only with TextEditor). However, I had no idea how to kick flushing layout during TextEditor handling an edit action. Therefore, I think that it's hard to find how to reproduce the security issue.
• 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?: ESR91
• 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?: (Only the first one should be uplift, the latter one is required only for debugging similar issue.)
• How likely is this patch to cause regressions; how much testing does it need?: I just tested with the testcase reported with this bug. And even if regression occurs, it does not affect to opt builds because nsFrameSelection handles unexpected cases except when starting new batch with new nsFrameSelection has a bug.

Comment on attachment 9264922 [details]
Bug 1753508 - Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug!

Approved to land and uplift

Comment on attachment 9264923 [details]
Bug 1753508 - Add logger of selection batch r=smaug!

Approved to land

Comment on attachment 9264922 [details]
Bug 1753508 - Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug!

Beta/Release Uplift Approval Request

• User impact if declined: May be kept as a victim of attack using UAF.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Load attachment 9262173 [details]

2. Then, the tab shouldn't crash

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): For fixing the assertion hit, this just restart selection batch in new selection if there is a selection batch and <input> or <textarea> is reframed.

For fixing the security issue, this just grab the old selection in the stach which was retired until all nested editing are completely handled.

• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: May be kept as a victim of attack using UAF.
• Fix Landed on Version: 100
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See above. Note that the following patch was required for debugging this bug. Therefore, it does not required to uplift. Then, only this can be grafted cleanly.

Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug
https://hg.mozilla.org/integration/autoland/rev/a8ebd1b979ac4946fe8e9ab59adeb9176b651a26
https://hg.mozilla.org/mozilla-central/rev/a8ebd1b979ac

Add logger of selection batch r=smaug
https://hg.mozilla.org/integration/autoland/rev/0ec773fd13011ca02a1385323bf84667e97080be
https://hg.mozilla.org/mozilla-central/rev/0ec773fd1301

Comment on attachment 9264922 [details]
Bug 1753508 - Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug!

Approved for 99.0b3. Thanks.

https://hg.mozilla.org/releases/mozilla-beta/rev/ab3a00887b5e
https://hg.mozilla.org/releases/mozilla-beta/rev/3faf6e9011ee

Comment on attachment 9264922 [details]
Bug 1753508 - Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug!

Doesn't sounds like this rises to the severity level needed to justify landing in a dot release.

Hi,
@Masayouki are there some extra steps for reproducing this issue or maybe I just missed something? I tried to reproduce the issue in esr91 and release 98 which are still affected, in order to verify the fix, using Windows 10 and Ubuntu 20, by simply loading the attachment provided in comment 14 but without success. There is no tab crash on my end in the affected versions.
Thanks!

(In reply to Alin Ilea from comment #19)

I tried to reproduce the issue in esr91 and release 98 which are still affected,

The fix has not landed in ESR and 98 is marked as wontfix. Currently only nightly and beta contain the fix (see the history above).

Yeah, the goal was to reproduce the issue in those unfixed builds for the purpose of verifying the fix on builds with the fix :)

Comment on attachment 9264922 [details]
Bug 1753508 - Make EditorBase::AutoEditActionDataSetter::UpdateSelectionCache clean up and restart selection batch which the editor started r=smaug!

Approved for 91.8esr.

https://hg.mozilla.org/releases/mozilla-esr91/rev/36658d4106cf

(In reply to Ryan VanderMeulen [:RyanVM] from comment #21)

Yeah, the goal was to reproduce the issue in those unfixed builds for the purpose of verifying the fix on builds with the fix :)

Bah I misread, sorry! stupid daylight savings, I've felt like a zombie all morning.

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220828210513-9056ada439d4.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.