Closed Bug 1736243 (CVE-2022-26381) Opened 3 years ago Closed 2 years ago

heap-use-after-free [@ mozilla::SVGRenderingObserver::OnNonDOMMutationRenderingChange]

Categories

(Core :: SVG, defect)

defect

Tracking

()

VERIFIED FIXED
99 Branch
Tracking Status
firefox-esr91 98+ verified
firefox95 --- wontfix
firefox97 --- wontfix
firefox98 + verified
firefox99 + verified

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main98+r][sec-survey][adv-esr91.7+r])

Crash Data

Attachments

(5 files, 1 obsolete file)

Attached file testcase.html

Found while fuzzing m-c 20211017-650d84a601ba (--enable-address-sanitizer --enable-fuzzing)

This might be a duplicate of bug 1415551. The test case appears to only reproduce the issue on Windows and it is fairly reliable.

==9852==ERROR: AddressSanitizer: heap-use-after-free on address 0x11e0b8ee8f08 at pc 0x7ffa37b57ec1 bp 0x007b693f2b40 sp 0x007b693f2b88
WRITE of size 1 at 0x11e0b8ee8f08 thread T0
    #0 0x7ffa37b57ec0 in mozilla::SVGRenderingObserver::OnNonDOMMutationRenderingChange /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:246
    #1 0x7ffa37b57ec0 in mozilla::SVGRenderingObserverSet::InvalidateAll(void) /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:1065
    #2 0x7ffa37b5271c in mozilla::SVGTextFrame::ReflowSVGNonDisplayText(void) /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:2844
    #3 0x7ffa37b05ae0 in mozilla::SVGContainerFrame::ReflowSVGNonDisplayText(class nsIFrame *) /builds/worker/checkouts/gecko/layout/svg/SVGContainerFrame.cpp:114
    #4 0x7ffa37b05b70 in mozilla::SVGContainerFrame::ReflowSVGNonDisplayText(class nsIFrame *) /builds/worker/checkouts/gecko/layout/svg/SVGContainerFrame.cpp:119
    #5 0x7ffa37b638e7 in mozilla::SVGOuterSVGFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/svg/SVGOuterSVGFrame.cpp:456
    #6 0x7ffa378e055a in nsLineLayout::ReflowFrame(class nsIFrame *, class nsReflowStatus &, class mozilla::ReflowOutput *, bool &) /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:875
    #7 0x7ffa37639487 in nsBlockFrame::ReflowInlineFrame(class mozilla::BlockReflowInput &, class nsLineLayout &, class nsLineList_iterator, class nsIFrame *, enum LineReflowStatus *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4565
    #8 0x7ffa37637d42 in nsBlockFrame::DoReflowInlineFrames(class mozilla::BlockReflowInput &, class nsLineLayout &, class nsLineList_iterator, struct nsFlowAreaRect &, int &, struct nsFloatManager::SavedState *, bool *, enum LineReflowStatus *, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4367
    #9 0x7ffa376301ff in nsBlockFrame::ReflowInlineFrames(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4252
    #10 0x7ffa37628509 in nsBlockFrame::ReflowLine(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3229
    #11 0x7ffa3761b904 in nsBlockFrame::ReflowDirtyLines(class mozilla::BlockReflowInput &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2763
    #12 0x7ffa37611a8d in nsBlockFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1394
    #13 0x7ffa37635988 in nsBlockReflowContext::ReflowBlock(class mozilla::LogicalRect const &, bool, struct nsCollapsingMargin &, int, bool, class nsLineBox *, struct mozilla::ReflowInput &, class nsReflowStatus &, class mozilla::BlockReflowInput &) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:288
    #14 0x7ffa3762b68d in nsBlockFrame::ReflowBlockFrame(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3886
    #15 0x7ffa376286b9 in nsBlockFrame::ReflowLine(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3226
    #16 0x7ffa3761b904 in nsBlockFrame::ReflowDirtyLines(class mozilla::BlockReflowInput &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2763
    #17 0x7ffa37611a8d in nsBlockFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1394
    #18 0x7ffa37661cac in nsContainerFrame::ReflowChild(class nsIFrame *, class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class mozilla::WritingMode const &, class mozilla::LogicalPoint const &, struct nsSize const &, enum nsIFrame::ReflowChildFlags, class nsReflowStatus &, class nsOverflowContinuationTracker *) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1004
    #19 0x7ffa3765f639 in nsCanvasFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/nsCanvasFrame.cpp:787
    #20 0x7ffa37661cac in nsContainerFrame::ReflowChild(class nsIFrame *, class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class mozilla::WritingMode const &, class mozilla::LogicalPoint const &, struct nsSize const &, enum nsIFrame::ReflowChildFlags, class nsReflowStatus &, class nsOverflowContinuationTracker *) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1004
    #21 0x7ffa3771fb72 in nsHTMLScrollFrame::ReflowScrolledFrame(struct mozilla::ScrollReflowInput *, bool, bool, class mozilla::ReflowOutput *) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:763
    #22 0x7ffa3772239a in nsHTMLScrollFrame::ReflowContents(struct mozilla::ScrollReflowInput *, class mozilla::ReflowOutput const &) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:884
    #23 0x7ffa3772f399 in nsHTMLScrollFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:1305
    #24 0x7ffa375f9b4e in nsContainerFrame::ReflowChild(class nsIFrame *, class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, int, int, enum nsIFrame::ReflowChildFlags, class nsReflowStatus &, class nsOverflowContinuationTracker *) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1044
    #25 0x7ffa375f898a in mozilla::ViewportFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:374
    #26 0x7ffa373a70aa in mozilla::PresShell::DoReflow(class nsIFrame *, bool, class mozilla::OverflowChangedTracker *) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9598
    #27 0x7ffa373c0cb2 in mozilla::PresShell::ProcessReflowCommands(bool) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9769
    #28 0x7ffa373be9c4 in mozilla::PresShell::DoFlushPendingNotifications(struct mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4259
    #29 0x7ffa3732354e in mozilla::PresShell::FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1436
    #30 0x7ffa3732354e in nsRefreshDriver::Tick(struct mozilla::layers::BaseTransactionId<class mozilla::VsyncIdType>, class mozilla::TimeStamp, enum nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2398
    #31 0x7ffa3733a173 in mozilla::RefreshDriverTimer::TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:354
    #32 0x7ffa3733a173 in mozilla::RefreshDriverTimer::TickRefreshDrivers(struct mozilla::layers::BaseTransactionId<class mozilla::VsyncIdType>, class mozilla::TimeStamp, class nsTArray<class RefPtr<class nsRefreshDriver>> &) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:331
    #33 0x7ffa37339d6a in mozilla::RefreshDriverTimer::Tick(struct mozilla::layers::BaseTransactionId<class mozilla::VsyncIdType>, class mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:347
    #34 0x7ffa37339883 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(struct mozilla::layers::BaseTransactionId<class mozilla::VsyncIdType>, class mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:782
    #35 0x7ffa37338b13 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(struct mozilla::layers::BaseTransactionId<class mozilla::VsyncIdType>, class mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:705
    #36 0x7ffa37337958 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyParentProcessVsync(void) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:622
    #37 0x7ffa37336f1e in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(struct mozilla::VsyncEvent const &) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:543
    #38 0x7ffa35fe73d9 in mozilla::dom::VsyncChild::RecvNotify(struct mozilla::VsyncEvent const &, float const &) /builds/worker/checkouts/gecko/dom/ipc/VsyncChild.cpp:68
    #39 0x7ffa2f4d65b4 in mozilla::dom::PVsyncChild::OnMessageReceived(class IPC::Message const &) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:178
    #40 0x7ffa2f25ebff in mozilla::ipc::PBackgroundChild::OnMessageReceived(class IPC::Message const &) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundChild.cpp:6207
    #41 0x7ffa2ee1a514 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(class mozilla::ipc::ActorLifecycleProxy *, class IPC::Message const &) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:2043
    #42 0x7ffa2ee1683f in mozilla::ipc::MessageChannel::DispatchMessage(class IPC::Message &&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1968
    #43 0x7ffa2ee186bc in mozilla::ipc::MessageChannel::RunMessage(class mozilla::ipc::MessageChannel::MessageTask &) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1827
    #44 0x7ffa2ee18c68 in mozilla::ipc::MessageChannel::MessageTask::Run(void) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1855
    #45 0x7ffa2da116ed in mozilla::RunnableTask::Run(void) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:467
    #46 0x7ffa2d9c7a23 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(class mozilla::detail::BaseAutoLock<class mozilla::Mutex &> const &) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:770
    #47 0x7ffa2d9c3e8c in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(class mozilla::detail::BaseAutoLock<class mozilla::Mutex &> const &) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:606
    #48 0x7ffa2d9c484e in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:390
    #49 0x7ffa2da1b741 in mozilla::TaskController::InitializeInternal::<unnamed-tag>::operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:123
    #50 0x7ffa2da1b741 in mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:123:7'>::Run /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:531
    #51 0x7ffa2d9f1995 in nsThread::ProcessNextEvent(bool, bool *) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1151
    #52 0x7ffa2da02c2c in NS_ProcessNextEvent(class nsIThread *, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:467
    #53 0x7ffa2ee239ae in mozilla::ipc::MessagePump::Run(class base::MessagePump::Delegate *) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85
    #54 0x7ffa2ed2f415 in MessageLoop::RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:331
    #55 0x7ffa2ed2f415 in MessageLoop::RunHandler(void) /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:324
    #56 0x7ffa2ed2f1e5 in MessageLoop::Run(void) /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:306
    #57 0x7ffa36b1f78a in nsBaseAppShell::Run(void) /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:137
    #58 0x7ffa36d1e08b in nsAppShell::Run(void) /builds/worker/checkouts/gecko/widget/windows/nsAppShell.cpp:603
    #59 0x7ffa3b02d9b4 in XRE_RunAppShell(void) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:917
    #60 0x7ffa2ed2f415 in MessageLoop::RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:331
    #61 0x7ffa2ed2f415 in MessageLoop::RunHandler(void) /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:324
    #62 0x7ffa2ed2f1e5 in MessageLoop::Run(void) /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:306
    #63 0x7ffa3b02cf19 in XRE_InitChildProcess(int, char **const, struct XREChildData const *) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:749
    #64 0x7ff707b81d39 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:58
    #65 0x7ff707b81d39 in NS_internal_main(int, char **, char **) /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:327
    #66 0x7ff707b814d4 in wmain /builds/worker/checkouts/gecko/toolkit/xre/nsWindowsWMain.cpp:131
    #67 0x7ff707c7e747 in invoke_main d:\agent\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:90
    #68 0x7ff707c7e747 in __scrt_common_main_seh d:\agent\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #69 0x7ffa72f37033  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017033)
    #70 0x7ffa73642650  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180052650)

0x11e0b8ee8f08 is located 8 bytes inside of 120-byte region [0x11e0b8ee8f00,0x11e0b8ee8f78)
freed by thread T0 here:
    #0 0x7ffa43d55bdb in free Z:\task_163283270682301\fetches\llvm-project\llvm\projects\compiler-rt\lib\asan\asan_malloc_win.cpp:82
    #1 0x7ffa37bae2da in operator delete /builds/worker/workspace/obj-build/dist/include/mozilla/cxxalloc.h:51
    #2 0x7ffa37bae2da in mozilla::SVGTextPathObserver::`scalar deleting dtor'(unsigned int) /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:967
    #3 0x7ffa3741d17d in mozilla::FrameProperties::PropertyValue::DestroyValueFor /builds/worker/checkouts/gecko/layout/base/FrameProperties.h:325
    #4 0x7ffa3741d17d in mozilla::FrameProperties::RemoveAll(class nsIFrame const *) /builds/worker/checkouts/gecko/layout/base/FrameProperties.h:256
    #5 0x7ffa373ab0fc in nsIFrame::RemoveAllProperties /builds/worker/workspace/obj-build/dist/include/nsIFrame.h:4224
    #6 0x7ffa373ab0fc in mozilla::PresShell::NotifyDestroyingFrame(class nsIFrame *) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:2216
    #7 0x7ffa377fd70e in nsIFrame::DestroyFrom(class nsIFrame *, struct mozilla::layout::PostFrameDestroyData &) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:883
    #8 0x7ffa37607fe6 in nsContainerFrame::DestroyFrom(class nsIFrame *, struct mozilla::layout::PostFrameDestroyData &) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:298
    #9 0x7ffa37645034 in nsBlockFrame::DoRemoveFrameInternal(class nsIFrame *, unsigned int, struct mozilla::layout::PostFrameDestroyData &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:6336
    #10 0x7ffa37647441 in nsBlockFrame::DoRemoveFrame /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.h:544
    #11 0x7ffa37647441 in nsBlockFrame::DeleteNextInFlowChild(class nsIFrame *, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:6557
    #12 0x7ffa378e1017 in nsLineLayout::ReflowFrame(class nsIFrame *, class nsReflowStatus &, class mozilla::ReflowOutput *, bool &) /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:1016
    #13 0x7ffa37639487 in nsBlockFrame::ReflowInlineFrame(class mozilla::BlockReflowInput &, class nsLineLayout &, class nsLineList_iterator, class nsIFrame *, enum LineReflowStatus *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4565
    #14 0x7ffa37637d42 in nsBlockFrame::DoReflowInlineFrames(class mozilla::BlockReflowInput &, class nsLineLayout &, class nsLineList_iterator, struct nsFlowAreaRect &, int &, struct nsFloatManager::SavedState *, bool *, enum LineReflowStatus *, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4367
    #15 0x7ffa376301ff in nsBlockFrame::ReflowInlineFrames(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4252
    #16 0x7ffa37628509 in nsBlockFrame::ReflowLine(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3229
    #17 0x7ffa3761b904 in nsBlockFrame::ReflowDirtyLines(class mozilla::BlockReflowInput &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2763
    #18 0x7ffa37611a8d in nsBlockFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1394
    #19 0x7ffa37ba8c5c in mozilla::SVGTextFrame::DoReflow(void) /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:5122
    #20 0x7ffa37b8af48 in mozilla::SVGTextFrame::MaybeReflowAnonymousBlockChild(void) /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:5063
    #21 0x7ffa37b52584 in mozilla::SVGTextFrame::ReflowSVGNonDisplayText /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:2850
    #22 0x7ffa37b52584 in mozilla::SVGTextPathObserver::OnRenderingChange(void) /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:502
    #23 0x7ffa37b57d79 in mozilla::SVGRenderingObserver::OnNonDOMMutationRenderingChange /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:247
    #24 0x7ffa37b57d79 in mozilla::SVGRenderingObserverSet::InvalidateAll(void) /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:1066

previously allocated by thread T0 here:
    #0 0x7ffa43d55ceb in malloc Z:\task_163283270682301\fetches\llvm-project\llvm\projects\compiler-rt\lib\asan\asan_malloc_win.cpp:98
    #1 0x7ffa4bfc154d in moz_xmalloc /builds/worker/checkouts/gecko/memory/mozalloc/mozalloc.cpp:52
    #2 0x7ffa37b5c6c3 in operator new /builds/worker/workspace/obj-build/dist/include/mozilla/cxxalloc.h:33
    #3 0x7ffa37b5c6c3 in mozilla::GetEffectProperty /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:1165
    #4 0x7ffa37b5c6c3 in mozilla::SVGObserverUtils::GetAndObserveTextPathsPath(class nsIFrame *) /builds/worker/checkouts/gecko/layout/svg/SVGObserverUtils.cpp:1420
    #5 0x7ffa37ba22f4 in mozilla::SVGTextFrame::GetTextPath(class nsIFrame *) /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:4542
    #6 0x7ffa37ba3276 in mozilla::SVGTextFrame::DoTextPathLayout(void) /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:4609
    #7 0x7ffa37ba7b3e in mozilla::SVGTextFrame::DoGlyphPositioning(void) /builds/worker/checkouts/gecko/layout/svg/SVGTextFrame.cpp:4954
    #8 0x7ffa37b05ae0 in mozilla::SVGContainerFrame::ReflowSVGNonDisplayText(class nsIFrame *) /builds/worker/checkouts/gecko/layout/svg/SVGContainerFrame.cpp:114
    #9 0x7ffa37b05b70 in mozilla::SVGContainerFrame::ReflowSVGNonDisplayText(class nsIFrame *) /builds/worker/checkouts/gecko/layout/svg/SVGContainerFrame.cpp:119
    #10 0x7ffa37b638e7 in mozilla::SVGOuterSVGFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/svg/SVGOuterSVGFrame.cpp:456
    #11 0x7ffa378e055a in nsLineLayout::ReflowFrame(class nsIFrame *, class nsReflowStatus &, class mozilla::ReflowOutput *, bool &) /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:875
    #12 0x7ffa37639487 in nsBlockFrame::ReflowInlineFrame(class mozilla::BlockReflowInput &, class nsLineLayout &, class nsLineList_iterator, class nsIFrame *, enum LineReflowStatus *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4565
    #13 0x7ffa37637d42 in nsBlockFrame::DoReflowInlineFrames(class mozilla::BlockReflowInput &, class nsLineLayout &, class nsLineList_iterator, struct nsFlowAreaRect &, int &, struct nsFloatManager::SavedState *, bool *, enum LineReflowStatus *, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4367
    #14 0x7ffa376301ff in nsBlockFrame::ReflowInlineFrames(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4252
    #15 0x7ffa37628509 in nsBlockFrame::ReflowLine(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3229
    #16 0x7ffa3761b904 in nsBlockFrame::ReflowDirtyLines(class mozilla::BlockReflowInput &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2763
    #17 0x7ffa37611a8d in nsBlockFrame::Reflow(class nsPresContext *, class mozilla::ReflowOutput &, struct mozilla::ReflowInput const &, class nsReflowStatus &) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1394
    #18 0x7ffa37635988 in nsBlockReflowContext::ReflowBlock(class mozilla::LogicalRect const &, bool, struct nsCollapsingMargin &, int, bool, class nsLineBox *, struct mozilla::ReflowInput &, class nsReflowStatus &, class mozilla::BlockReflowInput &) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:288
    #19 0x7ffa3762b68d in nsBlockFrame::ReflowBlockFrame(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3886
    #20 0x7ffa376286b9 in nsBlockFrame::ReflowLine(class mozilla::BlockReflowInput &, class nsLineList_iterator, bool *) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3226
Flags: in-testsuite?
Keywords: bugmon

(In reply to Sylvestre Ledru [:Sylvestre] from comment #1)

I can reproduce it on linux:
https://crash-stats.mozilla.org/report/index/4d76a195-4fe5-489f-b28c-7bc8f0211015#tab-details

I think this is a different bug.

Keywords: bugmon

Here is what I get (again on Windows):

https://crash-stats.mozilla.org/report/index/0699a63a-0a46-4498-9635-a08d90211019

This seems more like what I'd expect since it has a similar stack and the poison value (in rax).

Crash Signature: [@ mozilla::SVGRenderingObserverSet::InvalidateAll ]

Hmm I also get (on Linux)

https://crash-stats.mozilla.org/report/index/b2c5df1f-59e6-426a-817a-636230210615

This has what appears to be a frame poison value in rax.

Crash Signature: [@ mozilla::SVGRenderingObserverSet::InvalidateAll ] → [@ mozilla::SVGRenderingObserverSet::InvalidateAll ] [@ nsIFrame::SetParent ]

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

OS: Windows → Unspecified
Keywords: sec-high
Flags: needinfo?(dholbert)
Assignee: nobody → dholbert

FWIW, when I load the attached testcase on linux (in Nightly with a fresh profile), I get an immediate content-process crash, with signature mozilla::SVGRenderingObserverSet::InvalidateAll (as ~expected, unlike sledru's different-looking crash in comment 1).

Crash report: bp-fed810d5-7560-4495-bc0c-4158d0211118

In my case, it seems to have crashed with a null deref.

Side note: I spun off bug 1741956 for crashes in XGetImage (like sledru's in comment 1). I suspect that's not related to this bug at all -- I wonder if it might have been posted here by mistake? I notice that sledru's comment 1 was posted on Oct 18th, but the crash report he linked was processed 3 days earlier (2021-10-15 06:15:08 UTC). So I wonder if that was a copypaste error when copying over the crash report link, just getting the wrong crash ID from about:crashes or something.

sledru: if you can somehow trigger parent-process XGetImage-signature crashes with this bug's testcase, let us know; otherwise, I'm guessing that was a copypaste error. Also: if your about:crashes seems to contain any more-related-looking[1] crash reports from around when you posted comment 1, that would help confirm that this is indeed what happened.)

[1] I'd expect your crash to look similar to my crash from comment 6 or tyson's from comment 4; it should be a content-process crash with SVGTextFrame in the backtrace, probably.

Flags: needinfo?(dholbert) → needinfo?(sledru)
In a debug build, the attached testcase aborts with this assertion-failure in `SVGRenderingObserver::DebugObserverSet()`:
```
Assertion failure: inObserverSet == mInObserverSet (failed to track whether we're in our referenced element's observer set!),
at layout/svg/SVGObserverUtils.cpp:1115
```
https://searchfox.org/mozilla-central/rev/3881c4ca80d1b4b2f43be695438ecaf90ee4f86c/layout/svg/SVGObserverUtils.cpp#1113-1115

Backtrace for this assertion-failure attached.

In another debug-build load of the testcase, I got this additional non-fatal assertion, which appeared just before I hit the same fatal assertion-failure from comment 8:

[Child 62601, Main Thread] ###!!! ASSERTION: removing observer from an element we're not observing?: 'observers->Contains(aObserver)', file layout/svg/SVGObserverUtils.cpp:1625

I captured this run in rr and have submitted it to pernosco (along with the recording that we've already got in comment 5). I'll post the recording link when I've got it.

There's an idea from jwatt in bug 1539477 not sure if it would fix this but we could give it a shot.

Thanks! That's an interesting connection.

Here's the pernosco recording for the run in comment 9, at the moment we print the non-fatal assertion failure:
https://pernos.co/debug/CkryXMa2GCmndAKMKmmkqg/index.html#f{m[Cx0y,AA_,t[hw,9Ik_,f{e[Cx0t,AUjo4w_,s{af3y0oVAA,bDkA,uD/+9RQ,oEAjAXw___/

So working backwards from the non-fatal assertion in comment 9 (where we've gotten a RemoveRenderingObserver call for an element that we're not observing), we get this sequence of calls:
(1) SVGObserverUtils::AddRenderingObserver (to add a rendering observer)
(2) SVGObserverUtils::InvalidateRenderingObservers (which clears the set)
(3) SVGObserverUtils::RemoveRenderingObserver (to remove the rendering observer that we added in (1)).

In call #3 here, we trip over the non-fatal assertion because the rendering observer set is empty (it was emptied in call #2).

...and actually, call #3 happens while we are inside of call #2. We are, in fact, doing some reentrant reflow, which is why we're acting on the same data in a reentrant way.

Here's a pruned version of the backtrace, at the moment when the non-fatal assertion fails, in the afore-linked pernosco trace (I'll attach the full backtrace as an attachment):

#0 mozilla::SVGObserverUtils::RemoveRenderingObserver
#1 mozilla::SVGRenderingObserver::StopObserving
#2 mozilla::SVGIDRenderingObserver::~SVGIDRenderingObserver (this=0x7f7caa26a280)
#3 mozilla::SVGRenderingObserverProperty::~SVGRenderingObserverProperty
#6 mozilla::SVGRenderingObserverProperty::Release
[...SNIP...]
#10 mozilla::FrameProperties::RemoveAll
[...SNIP...]
#16 nsInlineFrame::DestroyFrom
#17 nsBlockFrame::DoRemoveFrameInternal
#18 nsBlockFrame::DoRemoveFrame
#19 nsBlockFrame::DeleteNextInFlowChild
#20 nsLineLayout::ReflowFrame
[...SNIP...]
#27 mozilla::SVGTextFrame::DoReflow
#28 mozilla::SVGTextFrame::MaybeReflowAnonymousBlockChild
#29 mozilla::SVGTextFrame::ReflowSVGNonDisplayText 
#30 mozilla::SVGTextPathObserver::OnRenderingChange
#31 mozilla::SVGRenderingObserver::OnNonDOMMutationRenderingChange
#32 mozilla::SVGRenderingObserverSet::InvalidateAll
#33 mozilla::SVGObserverUtils::InvalidateRenderingObservers
#34 mozilla::SVGTextFrame::ReflowSVGNonDisplayText
[...SNIP...]
58 mozilla::PresShell::DoReflow
Attachment #9251427 - Attachment description: backtrace of assertion failure → backtrace of fatal assertion failure

A bit more about what's happening here:

  • We call ReflowSVGNonDisplayText for the SVG Text frame with id a.
  • This invalidates its rendering observers (a collection of SVGTextPathObserver objects), pulling them into a local variable like so in SVGRenderingObserverSet::InvalidateAll:
  const auto observers = std::move(mObservers);

  for (const auto& observer : observers) {
    observer->OnNonDOMMutationRenderingChange();
  }
  • Its first rendering observer is the next SVG text frame. As part of invalidating there, we call ReflowSVGNonDisplayText() on that next text frame.
  • That next text frame has two lines in its anonymous block. During this reflow, it deletes a no-longer-needed next-continuation for its textPath's inline-frame (deleting the part that was in the second line).
  • While deleting this frame, we of course have to delete all of its frame-properties, including its SVGTextPathObserver. This is bad since we are currently iterating over observers (many stack levels up) which includes a pointer to this object, which it hasn't yet reached in its iteration.
  • In the SVGTextPathObserver destructor, SVGTextPathObserver attempts to unregister from the observation target (for safety, so that it doesn't get a callback after it's been deleted). But this fails because the observation target doesn't have the observer in its mObservers collection anymore -- it has moved to the local observers variable, which we don't have access to modify.
  • Later on, the above-quoted for (const auto& observer : observers) { loop presumably reaches the now-deleted SVGTextPathObserver (which still has a straggling pointer that was left behind in the observers collection). We use that pointer and trigger a use-after-free when using it, and we crash.

So, the short version of comment 16 is:

  • We're looping over some pointers and calling OnNonDOMMutationRenderingChange on each one.
  • In one of these iterations, the OnNonDOMMutationRenderingChange call causes frame-deletion which deletes one of the objects that we're going to iterate over in a later iteration of the loop.
  • So when we get to the iteration for that object, we're hosed.

I'm not yet sure what the right fix here is. Assuming that each individual operation is essentially correct here (i.e. our line-breaking is correct, we're correct to do frame deletion, etc), we could potentially work around this by changing how we iterate somehow so that the deleted SVGTextPathObserver is actually successful at unregistering itself. (We could do this by e.g. removing observers from mObservers one at a time as we process them; or by using some sort of weak-pointer setup instead of raw pointers, or maybe something else.)

Alternate approach: we could probably just make SVGTextPathObserver (or rather its superclass SVGRenderingObserver) refcounted, so that it can be left alive (in some "inactivated" state) when the frame's property table is torn down.

It looks like a lot of other observer classes in SVGObserverUtils are refcounted (and we have other mObservers; collections there that store RefPtrs rather than raw pointers), so there's some prior-art for this approach being reasonable.

Here's a strawman patch, which
(1) makes us use a RefPtr in mObservers (and the local auto observers var that copies its type). This prevents us from deleting the observer when the aforementioned continuation-frame-destruction happens.

(2) Makes the observer itself use a weak pointer to keep track of the frame, so that we'll safely be able to detect that it should be skipped when we get to it in the observer : observers loop (since its frame is destroyed at that point).

We still fail one assertion (failed to track whether we're in our referenced element's observer set!) for one of the observers, which is normally fatal but which this patch downgrades for convenience.

With this patch, I'm not seeing any crashes with the attached testcase, and I'm able to reload it over and over without issue (aside from the downgraded assertion-failure).

jwatt, I probably need to seek your advice here. There's a large comment above SVGRenderingObserverSet, from bug 1495562:
https://searchfox.org/mozilla-central/rev/080e18fa4748456003164f58b0d925b8c3826a67/layout/svg/SVGObserverUtils.cpp#990-1016
...which says (among other things) that it does not store strong references to the observers, and that becomes false with my attached strawman patch, of course (per (1) above). I wonder if this change risks introducing leaks or other issues somehow, or if we should just redesign how this invalidation works to simplify things somehow per your XXXjwatt comment there?

Flags: needinfo?(jwatt)
Attachment #9259022 - Attachment description: strawman patch → strawman patch (avoids the crash, though it still triggers an assertion and maybe causes other problems [?])

Pretty sure bug 1756793 is the same bug and the patch is the right way to fix this.

Depends on: 1756793

Thanks! That does indeed look like the same bug, yeah.

I'm glad the fix turned out to be simple; I hadn't considered that maybe we could just decline-to-observe-the-referenced-element on continuations, and I was worried that we'd have to rethink how this observer stuff is managed, per comment 17 - 18.

(Coincidentally, I was meaning to circle back to look at this this week, though I'm glad you got to it and found a simpler fix first! :))

In a build with bug 1756793's patch (getting the FirstContinuation in GetAndObserveTextPathsPath), I verified that I can load the attached testcase on this bug here, without crashing or asserting.

(vs. in a build without that patch, the content process aborts with the assert mentioned in comment 8.)

Flags: needinfo?(jwatt)
No longer depends on: 1756793
Assignee: dholbert → emilio

After discussion with emilio & dveditz, I've moved emilio's patch here and marked bug 1756793 as a dupe.

Attachment #9259022 - Attachment is obsolete: true

Note: I'll port emilio's sec-approval request from bug 1756793 comment 3 over to this bug, now that I've duped that bug over here.

(Also: bug 1756793 comment 2 has some additional notes -- originally the extended commit message of the patch, later-redacted to avoid giving away too much -- explaining the patch a bit more.)

Comment on attachment 9265271 [details]
Bug 1736243 - Make SVGObserverUtils::GetAndObserveTextPathsPath not observe on continuations. r=jwatt,jfkthame

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not super-easily, but easily enough for someone with deep layout knowledge.
  • 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?: 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?: This patch should apply cleanly to all supported branches without needing any changes.
  • How likely is this patch to cause regressions; how much testing does it need?: not very likely.
Attachment #9265271 - Flags: sec-approval?

Comment on attachment 9265271 [details]
Bug 1736243 - Make SVGObserverUtils::GetAndObserveTextPathsPath not observe on continuations. r=jwatt,jfkthame

Approved to land and uplift

Attachment #9265271 - Flags: sec-approval? → sec-approval+

https://hg.mozilla.org/integration/autoland/rev/cf5e6b2865d5e7a370446b0deb00e8ef86489bac

Please go ahead and request Beta/ESR approval on this ASAP so we can get it uplifted before next week's RC builds.

Flags: needinfo?(emilio)

Comment on attachment 9265271 [details]
Bug 1736243 - Make SVGObserverUtils::GetAndObserveTextPathsPath not observe on continuations. r=jwatt,jfkthame

Beta/Release Uplift Approval Request

  • User impact if declined: Sec-sensitive crashes.
  • 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: Test-case here or on dupe bugs.
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): One-liner
  • String changes made/needed: none
Flags: needinfo?(emilio)
Attachment #9265271 - Flags: approval-mozilla-release?
Attachment #9265271 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9265271 [details]
Bug 1736243 - Make SVGObserverUtils::GetAndObserveTextPathsPath not observe on continuations. r=jwatt,jfkthame

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: 98
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): One-liner.
Attachment #9265271 - Flags: approval-mozilla-esr91?

Comment on attachment 9265271 [details]
Bug 1736243 - Make SVGObserverUtils::GetAndObserveTextPathsPath not observe on continuations. r=jwatt,jfkthame

Err, wrong flag.

Attachment #9265271 - Flags: approval-mozilla-release?
Group: layout-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch
Attachment #9265271 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [post-critsmash-triage] [qa-triaged]

I've reproduced this crash using an affected Nightly asan build (2021-10-17), under Ubuntu 18.04 x64.

The crash is not reproducing anymore on the latest asan builds: Beta 98.0b10 and Nightly 99.0a1.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Comment on attachment 9265271 [details]
Bug 1736243 - Make SVGObserverUtils::GetAndObserveTextPathsPath not observe on continuations. r=jwatt,jfkthame

Approved for ESR.

Attachment #9265271 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+

This is also verified as fixed on 91.7.0esr asan build, with Ubuntu 18.04 x64.

Whiteboard: [adv-main98+r]

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.

Flags: needinfo?(emilio)
Whiteboard: [adv-main98+r] → [adv-main98+r][sec-survey]
Whiteboard: [adv-main98+r][sec-survey] → [adv-main98+r][sec-survey][adv-esr91.7+r]
Flags: needinfo?(emilio)
Alias: CVE-2022-26381

The ZDI plans to publish a blog post about their discovery of this bug (reported as the duplicate bug 1756793) on April 7th, tomorrow.
Any objections to opening this bug to the public?

No objections from me; it looks like all supported releases have been verified as fixed, including ESR91 (with the fix included in 91.7.0esr released March 8th, nearly a month ago).

The ZDI blog post is here.

Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: