Closed Bug 1666592 Opened 4 years ago Closed 4 years ago

use-after-poison in nsFlexContainerFrame::Reflow

Categories

(Core :: Layout: Flexbox, defect, P2)

Product:
Core
Component:
Layout: Flexbox
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
83 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- unaffected
firefox82 --- wontfix
firefox83 --- verified

People

(Reporter: legacyfirmware, Assigned: TYLin)

References

(Regression)

Details

(4 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main83-])

Attachments

(4 files)

minimized_repro.html
121 bytes, text/html
Details
Debug-nsFrameList-poison.patch (For debugging only)
4.30 KB, patch
Details | Diff | Splinter Review
Bug 1666592 - Check overflow list's emptiness as soon as we finish using it.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1666592 - Prevent PullItemsNextInFlow() from reparenting child's NIF if NIF's parent is already this container.
47 bytes, text/x-phabricator-request
Details | Review
Attached file minimized_repro.html —

Found with custom fuzzer.
Repro: Open with firefox (firefox file:///tmp/minimized_repro.html)
Tested with: latest asan via fuzzfetch, m-c-20200922154306-asan-opt
OS: Ubuntu 18.04.4 LTS

==13484==ERROR: AddressSanitizer: use-after-poison on address 0x625000158478 at pc 0x7fcfe71f6bf7 bp 0x7ffc3f314050 sp 0x7ffc3f314048
READ of size 8 at 0x625000158478 thread T0 (file:// Content)
error: address range table at offset 0x5d90 has an invalid tuple (length = 0) at offset 0x5db0
    #0 0x7fcfe71f6bf6 in IsEmpty /builds/worker/workspace/obj-build/dist/include/nsFrameList.h:289:44
    #1 0x7fcfe71f6bf6 in nsContainerFrame::NormalizeChildLists() /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1878:24
    #2 0x7fcfe72281d6 in nsFlexContainerFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsFlexContainerFrame.cpp:4324:3
    #3 0x7fcfe71b679b in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:294:11
    #4 0x7fcfe71ad5cc in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3834:11
    #5 0x7fcfe71a9c85 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3170:5
    #6 0x7fcfe71a08b1 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2708:7
    #7 0x7fcfe71988e2 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #8 0x7fcfe71e38b7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #9 0x7fcfe71e7e29 in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:705:7
    #10 0x7fcfe71e6cb0 in nsColumnSetFrame::ReflowColumns(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig&, bool) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:414:37
    #11 0x7fcfe71ebc65 in nsColumnSetFrame::FindBestBalanceBSize(mozilla::ReflowInput const&, nsPresContext*, nsColumnSetFrame::ReflowConfig&, nsColumnSetFrame::ColumnBalanceData, mozilla::ReflowOutput&, bool, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:1137:9
    #12 0x7fcfe71ecb98 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:1252:5
    #13 0x7fcfe71b679b in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:294:11
    #14 0x7fcfe71ad5cc in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3834:11
    #15 0x7fcfe71a9c85 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3170:5
    #16 0x7fcfe71a08b1 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2708:7
    #17 0x7fcfe71988e2 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #18 0x7fcfe71b679b in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:294:11
    #19 0x7fcfe71ad5cc in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3834:11
    #20 0x7fcfe71a9c85 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3170:5
    #21 0x7fcfe71a08b1 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2708:7
    #22 0x7fcfe71988e2 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #23 0x7fcfe71e38b7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #24 0x7fcfe71e7e29 in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:705:7
    #25 0x7fcfe71e6cb0 in nsColumnSetFrame::ReflowColumns(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig&, bool) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:414:37
    #26 0x7fcfe71ebc65 in nsColumnSetFrame::FindBestBalanceBSize(mozilla::ReflowInput const&, nsPresContext*, nsColumnSetFrame::ReflowConfig&, nsColumnSetFrame::ColumnBalanceData, mozilla::ReflowOutput&, bool, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:1137:9
    #27 0x7fcfe71ecb98 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsColumnSetFrame.cpp:1252:5
    #28 0x7fcfe71b679b in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:294:11
    #29 0x7fcfe71ad5cc in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3834:11
    #30 0x7fcfe71a9c85 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3170:5
    #31 0x7fcfe71a08b1 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:2708:7
    #32 0x7fcfe71988e2 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #33 0x7fcfe71e38b7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #34 0x7fcfe71e253c in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsCanvasFrame.cpp:757:5
    #35 0x7fcfe71e38b7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #36 0x7fcfe72766fb in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:755:3
    #37 0x7fcfe7277fed in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:879:3
    #38 0x7fcfe727e7b3 in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:1277:3
    #39 0x7fcfe7188501 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1115:14
    #40 0x7fcfe7187b92 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:297:7
    #41 0x7fcfe6fa733d in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9652:11
    #42 0x7fcfe6fb9d97 in mozilla::PresShell::ProcessReflowCommands(bool) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9825:24
    #43 0x7fcfe6fb8808 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4240:11
    #44 0x7fcfe6f475af in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2152:20
    #45 0x7fcfe6f555d9 in TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:373:13
    #46 0x7fcfe6f555d9 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:352:7
    #47 0x7fcfe6f55251 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:367:5
    #48 0x7fcfe6f54464 in RunRefreshDrivers /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:828:5
    #49 0x7fcfe6f54464 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:746:16
    #50 0x7fcfe6f538d0 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyParentProcessVsync() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:648:7
    #51 0x7fcfe6f530d0 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:569:9
    #52 0x7fcfe76f28b8 in mozilla::layout::VsyncChild::RecvNotify(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/ipc/VsyncChild.cpp:55:16
    #53 0x7fcfe0ccc6b2 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:187:54
    #54 0x7fcfe0a52492 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundChild.cpp:6080:32
    #55 0x7fcfe06ad9fb in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:2150:25
    #56 0x7fcfe06aa9d1 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:2074:9
    #57 0x7fcfe06abe2e in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1922:3
    #58 0x7fcfe06ac58e in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1953:13
    #59 0x7fcfdf54c3e9 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:244:16
    #60 0x7fcfdf50b323 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:514:26
    #61 0x7fcfdf508d07 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:373:15
    #62 0x7fcfdf50915d in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:170:36
    #63 0x7fcfdf55a091 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:84:37
    #64 0x7fcfdf55a091 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
    #65 0x7fcfdf52e8d3 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1234:14
    #66 0x7fcfdf5389cc in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #67 0x7fcfe06b483a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:87:21
    #68 0x7fcfe05dadb1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #69 0x7fcfe05dadb1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #70 0x7fcfe05dadb1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #71 0x7fcfe6a4c597 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:137:27
    #72 0x7fcfea43b3df in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
    #73 0x7fcfe05dadb1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #74 0x7fcfe05dadb1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #75 0x7fcfe05dadb1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #76 0x7fcfea43a974 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
    #77 0x5568d591c2bd in content_process_main(mozilla::Bootstrap*, int, char**) /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #78 0x5568d591c6fa in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:304:18
    #79 0x7fcfff43fb96 in __libc_start_main /build/glibc-2ORdQG/glibc-2.27/csu/../csu/libc-start.c:310
    #80 0x5568d586fc75 in _start (/home/dalanath/firefox/m-c-20200922154306-asan-opt/firefox+0x57c75)

0x625000158478 is located 7032 bytes inside of 8192-byte region [0x625000156900,0x625000158900)
allocated by thread T0 (file:// Content) here:
    #0 0x5568d58e9afd in malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x7fcfdf4e5e70 in mozilla::ArenaAllocator<8192ul, 8ul>::AllocateChunk(unsigned long) /builds/worker/workspace/obj-build/dist/include/mozilla/ArenaAllocator.h:171:15
    #2 0x7fcfe710167e in InternalAllocate /builds/worker/workspace/obj-build/dist/include/mozilla/ArenaAllocator.h:205:25
    #3 0x7fcfe710167e in Allocate /builds/worker/workspace/obj-build/dist/include/mozilla/ArenaAllocator.h:67:12
    #4 0x7fcfe710167e in mozilla::ArenaAllocator<8192ul, 8ul>::Allocate(unsigned long) /builds/worker/workspace/obj-build/dist/include/mozilla/ArenaAllocator.h:71:15
    #5 0x7fcfe71858e5 in AllocateByObjectID /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:259:32
    #6 0x7fcfe71858e5 in AllocateFrame /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:251:12
    #7 0x7fcfe71858e5 in operator new /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:35:1
    #8 0x7fcfe71858e5 in NS_NewViewportFrame(mozilla::PresShell*, mozilla::ComputedStyle*) /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:32:10
    #9 0x7fcfe703f85b in nsCSSFrameConstructor::ConstructRootFrame() /builds/worker/checkouts/gecko/layout/base/nsCSSFrameConstructor.cpp:2419:7
    #10 0x7fcfe6fa3408 in mozilla::PresShell::Initialize() /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:1849:36
    #11 0x7fcfe28ef9bc in nsContentSink::StartLayout(bool) /builds/worker/checkouts/gecko/dom/base/nsContentSink.cpp:1140:30
    #12 0x7fcfe169ed69 in nsHtml5TreeOpExecutor::StartLayout(bool*) /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOpExecutor.cpp:678:18
==13484==WARNING: Symbolizer buffer too small
    #13 0x7fcfe16a97a2  (/home/dalanath/firefox/m-c-20200922154306-asan-opt/libxul.so+0x4e357a2)
==13484==WARNING: Symbolizer buffer too small
    #14 0x7fcfe169d8a6  (/home/dalanath/firefox/m-c-20200922154306-asan-opt/libxul.so+0x4e298a6)
    #15 0x7fcfe169ca25 in beComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #16 0x7fcfe169ca25 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #17 0x7fcfe169ca25 in decltype(auto) mozilla::detail::VariantImplementation<unsigned char, 7ul, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu>::match<nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*)::TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> >(nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*)::TreeOperationMatcher&&, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu>&) /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #18 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #19 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #20 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #21 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #22 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #23 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #24 0x7fcfe16a3888 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opSetDocumentCharset, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> > /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:269:14
    #25 0x7fcfe16a3888 in match<TreeOperationMatcher> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:795:12
    #26 0x7fcfe16a3888 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*) /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOperation.cpp:1176:21
    #27 0x7fcfdf4f7dad in nsHtml5ExecutorFlusher::Run() /builds/worker/checkouts/gecko/parser/html/nsHtml5StreamParser.cpp:128:18
    #28 0x7fcfdf54c3e9 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/checkouts/gecko/xpcom/threads/SchedulerGroup.cpp:146:20
    #29 0x7fcfdf50b323 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:244:16
    #30 0x7fcfdf508d07 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:514:26
    #31 0x7fcfdf50915d in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:170:36
    #32 0x7fcfdf55a091 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:84:37
    #33 0x7fcfdf55a091 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
    #34 0x7fcfdf52e8d3 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1234:14
    #35 0x7fcfdf5389cc in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #36 0x7fcfe06b483a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:87:21
    #37 0x7fcfe05dadb1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #38 0x7fcfe05dadb1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #39 0x7fcfe05dadb1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #40 0x7fcfe6a4c597 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:137:27
    #41 0x7fcfea43b3df in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
    #42 0x7fcfe05dadb1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #43 0x7fcfe05dadb1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #44 0x7fcfe05dadb1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #45 0x7fcfea43a974 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
    #46 0x5568d591c2bd in content_process_main(mozilla::Bootstrap*, int, char**) /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #47 0x5568d591c6fa in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:304:18
    #48 0x7fcfff43fb96 in __libc_start_main /build/glibc-2ORdQG/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: use-after-poison /builds/worker/workspace/obj-build/dist/include/nsFrameList.h:289:44 in IsEmpty
Shadow bytes around the buggy address:
  0x0c4a80023030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80023040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80023050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80023060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80023070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4a80023080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[f7]
  0x0c4a80023090: f7 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a800230a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a800230b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a800230c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a800230d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==13484==ABORTING
Flags: sec-bounty?

:dholbert, is this your bailiwick?

Group: firefox-core-security → layout-core-security
Type: task → defect
Component: Security → Layout: Flexbox
Flags: needinfo?(dholbert)
Product: Firefox → Core

Looks like it's flexbox-in-multicol, which is an area that TYLin has been looking at more closely than I have recently.

TYLin, do you have cycles to take a look here?

Flags: needinfo?(dholbert) → needinfo?(aethanyc)

Judging by the allocation stack, this might be frame poisoning.

Yeah, these nsFrameList objects are allocated in the pres shell arena so our "frame-poisoning" makes this a safe crash.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: csectype-framepoisoning, sec-other, testcase
OS: Unspecified → All
Priority: -- → P2
Hardware: Unspecified → All

I can reproduce the testcase by using grizzly and running python3 -m grizzly.replay <firefox_asan_bin> minimized_repro.html --xvfb.

However, the testcase doesn't crash or produce any assertions on my local debug-build, so it's hard to debug what went wrong. Our frame-poisoning mechanism should produce a crash if the object is used after being poisoned, shouldn't it? Anyone has ideas on how to proceed?

I usually record these with rr and attach to the crashing process, then debug from there.

I always have difficulties when debugging rr sessions recorded by asan build. Reasons include lack of source code mapping in GDB, fields get optimized out, and methods are inlined (cannot set breakpoint to the method), etc.

I hack a patch to record freed pointers in pres shell arena, and assert that nsFrameList's this pointer being freed is used to call nsFrameList::IsEmpty() (based on the callstack in comment 0). Once the assertion hit, we can simply break in nsFrameList::Delete conditioned on this pointer's address. The root cause is a reverse-continue away.

The root cause is in nsContainerFrame::NormalizeChildLists(), where we call PullItemsNextInFlow before checking ourOverflow->IsEmpty() [1]. PullItemsNextInFlow calls StealFrame(childNIF), so if the child being stolen is the only child in the overflow list, StealFrame deletes the overflow list correctly, resulting ourOverflow being poisoned.

This explains why nothing happens on debug / release build. The code here is simply checking a poisoned nsFramelist's emptiness, and a poisoned frame list has non-null mFirstChild pointer, so it's treated as non-empty. Luckily we don't free the overflow list again.

[1] https://searchfox.org/mozilla-central/rev/f21850ca45036ddb84cd25aa355a6969d7c94c4f/layout/generic/nsContainerFrame.cpp#1883-1885,1887-1889

Flags: needinfo?(aethanyc)
Attachment #9178821 - Attachment is patch: true
Assignee: nobody → aethanyc
Status: NEW → ASSIGNED

Thanks for getting to the bottom of this!

PullItemsNextInFlow calls StealFrame(childNIF), so if the child being stolen is the only child in the overflow list, StealFrame deletes the overflow list correctly, resulting ourOverflow being poisoned.

Hmm, let me see if I understand this correctly... PullItemsNextInFlow is supposed to pull from the next-in-flow of the frame (this) that owns ourOverflow:
https://searchfox.org/mozilla-central/rev/f21850ca45036ddb84cd25aa355a6969d7c94c4f/layout/generic/nsContainerFrame.cpp#1837
but it seems that one if the children (on items) has a next-in-flow that is actually parented to this, and on ourOverflow to boot, so we steal it from our self (deleting ourOverflow since it's the last frame on the list) and then move it to our next-in-flow. Is that correct?

If so, it seems to me that PullItemsNextInFlow shouldn't do that in the first place. It should be fine to have a child on the Principal list and its next-in-flow on the OverflowList of the same parent.

Also, the documentation for PullItemsNextInFlow doesn't make sense to me. It says that it pulls up frames "into aItems" but aItems is const and doesn't change so that description seems confusing to me.

(BTW, PullItemsNextInFlow was added in bug 1640051 so I'm guessing this is a regression from that bug.)

Blocks: 1640051
Keywords: regression

but it seems that one if the children (on items) has a next-in-flow that is actually parented to this, and on ourOverflow to boot, so we steal it from our self (deleting ourOverflow since it's the last frame on the list) and then move it to our next-in-flow. Is that correct?

I take a look at the testcase again. I think you're right. That means the child and the child's next-in-flow are both in this frame's overflow list at the beginning of the reflow(). So this chunk of code collects the child into items, and PullItemsNextInFlow move the child's next-in-flow, which is the only child in this frame's overflow list, to this frame's next-in-flow.

Before bug 1640051 extract the code and name it PullItemsNextInFlow, it was used on the item list, which contain all first-in-flow children we just collected from this frame's next-in-flows, so the first-in-flow child's next-in-flow can never be in this frame. But in this bug's testcase, both child and the child's next-in-flow are in this frame's overflow list. This can happen during the previous reflow(), where the child cannot fit, got moved to the overflow list, and ended up sitting together with the child's next-in-flow. And we reflow this frame again without reflow this frame's next-in-flow because it was in a column container doing column balancing or something.

If so, it seems to me that PullItemsNextInFlow shouldn't do that in the first place. It should be fine to have a child on the Principal list and its next-in-flow on the OverflowList of the same parent.
Also, the documentation for PullItemsNextInFlow doesn't make sense to me. It says that it pulls up frames "into aItems" but aItems is const and doesn't change so that description seems confusing to me.

Agree with this! I'll upload a patch to fix PullItemsNextInFlow and its documentation.

No longer blocks: 1640051
Regressed by: 1640051
Has Regression Range: --- → yes

We can do this because all the child in aItems are going to be moved to
this container's principal child list. It's fine to leave the child's
next-in-flow in other lists of the same parent.

https://hg.mozilla.org/integration/autoland/rev/9e0769bfbd9c2502a466ad0f3394e9dcb7f08041
https://hg.mozilla.org/mozilla-central/rev/9e0769bfbd9c

Group: layout-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox83: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage]

Confirmed crash with 83.0a1(20200922094538) on Ubuntu 20.
Fix verified with 83.0a1 (2020-10-05).

Status: RESOLVED → VERIFIED
status-firefox83: fixed → verified
Flags: qe-verify+

Unfortunately this detected a type of UAF that we have specifically mitigated in the layout engine by allocating each frame object only from arenas that contain exactly the same type of frame object. Each member of a replaced object will function exactly the same (data is data, virtual pointers point to the same methods) so all it can do is mess up the visual layout, or crash on our poison values which point to unmapped memory. This does not qualify for a security bug bounty. (Also we had apparently found this crash earlier ourselves, which would also have disqualified it.)

Flags: sec-bounty? → sec-bounty-
Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage] → [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main83-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: