Closed Bug 1663222 Opened 4 years ago Closed 3 years ago

use-after-poison in [@ nsBlockFrame::ReflowBlockFrame]

Categories

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

Product:
Core
Component:
Layout: Columns
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- wontfix
firefox82 --- wontfix
firefox85 --- wontfix
firefox86 --- fixed

People

(Reporter: tsmith, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main86+r])

Crash Data

Attachments

(2 files, 1 obsolete file)

testcase.html
333 bytes, text/html
Details
Bug 1663222 - Move any overflow-container continuations on the OverflowContainers list for the children we pull from our prev-in-flow to the ExcessOverflowContainers to keep frames ordered. r=TYLin
47 bytes, text/x-phabricator-request
Details | Review
Attached file testcase.html —

Reduced with m-c 20200903-aa032cbc9455

==107560==ERROR: AddressSanitizer: use-after-poison on address 0x625000304a10 at pc 0x7f0e3696c050 bp 0x7ffc6b5e5280 sp 0x7ffc6b5e5278
READ of size 8 at 0x625000304a10 thread T0 (file:// Content)
    #0 0x7f0e3696c04f in get /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:286:27
    #1 0x7f0e3696c04f in operator-> /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:316:12
    #2 0x7f0e3696c04f in nsIFrame::StyleDisplay() const /builds/worker/workspace/obj-build/dist/include/nsStyleStructList.h:46:1
    #3 0x7f0e3c43d001 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /gecko/layout/generic/nsBlockFrame.cpp:3505:33
    #4 0x7f0e3c43b355 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /gecko/layout/generic/nsBlockFrame.cpp:3169:5
    #5 0x7f0e3c4335bf in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /gecko/layout/generic/nsBlockFrame.cpp:2994:11
    #6 0x7f0e3c42a246 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #7 0x7f0e3c447d60 in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) /gecko/layout/generic/nsBlockReflowContext.cpp:294:11
    #8 0x7f0e3c43ed9d in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /gecko/layout/generic/nsBlockFrame.cpp:3833:11
    #9 0x7f0e3c43b355 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /gecko/layout/generic/nsBlockFrame.cpp:3169:5
    #10 0x7f0e3c432144 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /gecko/layout/generic/nsBlockFrame.cpp:2707:7
    #11 0x7f0e3c42a246 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #12 0x7f0e3c474b97 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #13 0x7f0e3c478fb9 in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool) /gecko/layout/generic/nsColumnSetFrame.cpp:705:7
    #14 0x7f0e3c477f60 in nsColumnSetFrame::ReflowColumns(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig&, bool) /gecko/layout/generic/nsColumnSetFrame.cpp:414:37
    #15 0x7f0e3c47cf55 in nsColumnSetFrame::FindBestBalanceBSize(mozilla::ReflowInput const&, nsPresContext*, nsColumnSetFrame::ReflowConfig&, nsColumnSetFrame::ColumnBalanceData, mozilla::ReflowOutput&, bool, nsReflowStatus&) /gecko/layout/generic/nsColumnSetFrame.cpp:1137:9
    #16 0x7f0e3c47de48 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/nsColumnSetFrame.cpp:1252:5
    #17 0x7f0e3c447d60 in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) /gecko/layout/generic/nsBlockReflowContext.cpp:294:11
    #18 0x7f0e3c43ed9d in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /gecko/layout/generic/nsBlockFrame.cpp:3833:11
    #19 0x7f0e3c43b355 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) /gecko/layout/generic/nsBlockFrame.cpp:3169:5
    #20 0x7f0e3c432144 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) /gecko/layout/generic/nsBlockFrame.cpp:2707:7
    #21 0x7f0e3c42a246 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/nsBlockFrame.cpp:1368:3
    #22 0x7f0e3c474b97 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #23 0x7f0e3c47385e in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/nsCanvasFrame.cpp:749:5
    #24 0x7f0e3c474b97 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /gecko/layout/generic/nsContainerFrame.cpp:1075:14
    #25 0x7f0e3c503c55 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*) /gecko/layout/generic/nsGfxScrollFrame.cpp:755:3
    #26 0x7f0e3c5054cd in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) /gecko/layout/generic/nsGfxScrollFrame.cpp:879:3
    #27 0x7f0e3c50bd63 in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/nsGfxScrollFrame.cpp:1277:3
    #28 0x7f0e3c419ff1 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /gecko/layout/generic/nsContainerFrame.cpp:1115:14
    #29 0x7f0e3c419674 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /gecko/layout/generic/ViewportFrame.cpp:297:7
    #30 0x7f0e3c23a26d in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /gecko/layout/base/PresShell.cpp:9643:11
    #31 0x7f0e3c24ccc7 in mozilla::PresShell::ProcessReflowCommands(bool) /gecko/layout/base/PresShell.cpp:9816:24
    #32 0x7f0e3c24b739 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /gecko/layout/base/PresShell.cpp:4239:11
    #33 0x7f0e3c1da2a3 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /gecko/layout/base/nsRefreshDriver.cpp:2139:20
    #34 0x7f0e3c1e7369 in TickDriver /gecko/layout/base/nsRefreshDriver.cpp:372:13
    #35 0x7f0e3c1e7369 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /gecko/layout/base/nsRefreshDriver.cpp:351:7
    #36 0x7f0e3c1e6fe1 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /gecko/layout/base/nsRefreshDriver.cpp:366:5
    #37 0x7f0e3c1f5754 in RunRefreshDrivers /gecko/layout/base/nsRefreshDriver.cpp:818:5
    #38 0x7f0e3c1f5754 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /gecko/layout/base/nsRefreshDriver.cpp:736:16
    #39 0x7f0e3c1f4d3f in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyParentProcessVsync() /gecko/layout/base/nsRefreshDriver.cpp:638:7
    #40 0x7f0e3c1e4772 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() /gecko/layout/base/nsRefreshDriver.cpp:537:20
    #41 0x7f0e33d9f339 in mozilla::RunnableTask::Run() /gecko/xpcom/threads/TaskController.cpp:242:16
    #42 0x7f0e33d9b847 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:512:26
    #43 0x7f0e33d996d7 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:371:15
    #44 0x7f0e33d99b2d in mozilla::TaskController::ProcessPendingMTTask(bool) /gecko/xpcom/threads/TaskController.cpp:168:36
    #45 0x7f0e33dab141 in operator() /gecko/xpcom/threads/TaskController.cpp:83:37
    #46 0x7f0e33dab141 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_4>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
    #47 0x7f0e33dcf0d4 in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1234:14
    #48 0x7f0e33dd985c in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #49 0x7f0e350a216f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:87:21
    #50 0x7f0e34fa6f61 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #51 0x7f0e34fa6f61 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #52 0x7f0e34fa6f61 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #53 0x7f0e3bcf3107 in nsBaseAppShell::Run() /gecko/widget/nsBaseAppShell.cpp:137:27
    #54 0x7f0e3f9b163f in XRE_RunAppShell() /gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
    #55 0x7f0e34fa6f61 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #56 0x7f0e34fa6f61 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #57 0x7f0e34fa6f61 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #58 0x7f0e3f9b0bdc in XRE_InitChildProcess(int, char**, XREChildData const*) /gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
    #59 0x5593d4f7b8dd in content_process_main(mozilla::Bootstrap*, int, char**) /gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #60 0x5593d4f7bd17 in main /gecko/browser/app/nsBrowserApp.cpp:303:18
    #61 0x7f0e5029f0b2 in __libc_start_main /build/glibc-YYA7BZ/glibc-2.31/csu/../csu/libc-start.c:308:16
    #62 0x5593d4ecf279 in _start (/home/worker/builds/m-c-20200903151816-fuzzing-asan-opt/firefox+0x5c279)
Flags: in-testsuite?

column-span related.

Component: Layout: Block and Inline → Layout: Columns
Flags: needinfo?(aethanyc)

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

I had no luck to reproduce this with

python3 -m ffpuppet ~/ffpuppet_debug/firefox-asan/firefox-bin -p prefs.js -d -u ~/Downloads/1663222.html

where the asan build is downloaded by python3 -m fuzzfetch --asan -n firefox-asan and prefs.js generated by python3 -m prefpicker browser-fuzzing.yml prefs.js.

Try with xvfb. Also Grizzly[1] has a replay mode now which makes it even easier to repo issues (handles ffpuppet and prefpicker for you). You can pip install grizzly-framework and use python3 -m grizzly.replay <firefix_bin> <testcase> --xvfb. It also has rr support and --repeat to handle retrying stubborn test cases.

[1] https://github.com/MozillaSecurity/grizzly

Keywords: sec-high
Severity: -- → S2
Priority: -- → P2

Assigning to TYLin for now.

Assignee: nobody → aethanyc
Flags: needinfo?(aethanyc)

(In reply to Tyson Smith [:tsmith] from comment #4)

Try with xvfb. Also Grizzly[1] has a replay mode now which makes it even easier to repo issues (handles ffpuppet and prefpicker for you). You can pip install grizzly-framework and use python3 -m grizzly.replay <firefix_bin> <testcase> --xvfb. It also has rr support and --repeat to handle retrying stubborn test cases.

Hi Ting-Yu, Have you had any luck reproducing this bug given Tyson's suggestions in comment 4? This is a sec-high, and Platform is actively trying to reduce it's overall sec bug count (especially sec-highs and sec-crits). Thanks!

Flags: needinfo?(aethanyc)

Yes, I can reproduce it with the command Tyson suggested in comment 4, and the --xvfb argument is necessary.

This problem is that a poisoned nsIFrame is being used, so I believe it's not exploitable. Also, given the testcase doesn't crash in normal release and debug build, I'll change the sec rating to sec-low. If this is not appropriate, please let me know.

Flags: needinfo?(aethanyc)
Keywords: sec-high → csectype-framepoisoning, sec-low

The testcase is a little bit tricky to reproduce. The exact commands I used are:

  1. Download the asan debug build: python3 -m fuzzfetch --asan -n firefox-asan-debug -d
  2. Reproduce: python3 -m grizzly.replay firefox-asan-debug/firefox-bin testcase.html --xvfb -l log

By looking at log/reports/42f53dc1_2020-10-04_21-42-57_logs/log_stderr.txt, I see assertions like

[Child 75531, Main Thread] ###!!! ASSERTION: frame not in line: 'line->Contains(aDeletedFrame)', file /home/aethanyc/Projects/gecko/layout/generic/nsBlockFrame.cpp, line 6226
[Child 75531, Main Thread] ###!!! ASSERTION: value should always be stored and non-empty when state set: 'prop && !prop->mLines.empty() && prop->mLines.front()->GetChildCount() == 0 ? prop->mFrames.IsEmpty() : prop->mLines.front()->mFirstChild == prop->mFrames.FirstChild()', file /home/aethanyc/Projects/gecko/layout/generic/nsBlockFrame.cpp, line 5382
[Child 75531, Main Thread] ###!!! ASSERTION: bad overflow frames / lines: 'overflowLines->mLines.front()->mFirstChild == overflowLines->mFrames.FirstChild()', file /home/aethanyc/Projects/gecko/layout/generic/nsBlockFrame.cpp, line 7955
[Child 75531, Main Thread] ###!!! ASSERTION: value should always be stored and non-empty when state set: 'prop && !prop->mLines.empty() && prop->mLines.front()->GetChildCount() == 0 ? prop->mFrames.IsEmpty() : prop->mLines.front()->mFirstChild == prop->mFrames.FirstChild()', file /home/aethanyc/Projects/gecko/layout/generic/nsBlockFrame.cpp, line 5382
[Child 75531, Main Thread] ###!!! ASSERTION: Unexpected last frame: 'pulledFrames.LastChild() == pulledLine->LastChild()', file /home/aethanyc/Projects/gecko/layout/generic/nsBlockFrame.cpp, line 2970

The first assertion frame not in line[1] is in nsBlockFrame::DoRemoveFrameInternal(). That's a method for deleting a frame and all its continuation. The assertion is triggered when a continuation has the block frame as its parent, the block frame is going to delete it, but it cannot find the line containing the continuation.

[1] https://searchfox.org/mozilla-central/rev/4352fb7b0d17c1febff9569ed311e0e42c93093e/layout/generic/nsBlockFrame.cpp#6220

The bug happens when a block frame wants to remove a child's
next-continuation [1] whose parent is still the same block frame (if a
next-continuation is not in the same block, the loop would break). In
this case, the block either has advanced to the end of its normal line,
or it cannot find the to-be-deleted next-continuation in the rest of the
normal lines. Either way, the block would switch to overflow lines and
search in the them.

The old code assumes the to-be-deleted next-continuation is in the first
overflow line, but this is assumption doesn't hold. We should search the
overflow lines for it (similar to what we do near the beginning of
nsBlockFrame::DoRemoveFrameInternal in searching aDeletedFrame).

[1] The child in the testcase is a ::-moz-column-span-wrapper.

My patch can fix Bug 1670336, too. I've added the testcase into my patch here as it can be reproduce in ordinary debug build.

Crash Signature: [@ nsLineBox::DeleteLineList]

Making them in-flow continuations is valid but then we can't just
append them ot mFrames since that'll put them on the last line which may
result in some lines in-between the pulled child and its continuation.
That violates the invariant that in-flow continuations in the same
parent must be on consecutive lines (unless they are overflow containers
in which case they need to be on the [Excess]OverflowContainers lists).

(The crash happens because if we push the lines in-between the two
continuations to the OverflowLines, then we won't find the 2nd
continuation on the first line of OverflowLines.)

This patch relaxes the assertion that we only have one of the
[Excess]OverflowContainers lists when draining those lists,
but that's just an inconvenience without much value. That assertion
only holds there anyway - once the parent is reflowed it may
have both lists temporarily before its next-in-flow picks up
the ExcessOverflowContainers children.

Attachment #9179501 - Attachment is obsolete: true

Hmm, why did a bot set the signature field on this bug to "[@ nsLineBox::DeleteLineList] "? I don't see that method implicated anywhere in this bug.

Assignee: aethanyc → mats
Crash Signature: [@ nsLineBox::DeleteLineList]
OS: Unspecified → All
Hardware: Unspecified → All

That signature was in the duplicate bug 1670336, the bot copied it from there.

Ah, that makes sense.

Crash Signature: [@ nsLineBox::DeleteLineList ]

Move any overflow-container continuations on the OverflowContainers list for the children we pull from our prev-in-flow to the ExcessOverflowContainers to keep frames ordered. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/4e07cf6a753615eb882847e1a57705c443cb2c5f

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED

Looks like a low-volume crash on ESR78. Is this worth backporting?

status-firefox-esr78: --- → affected
Flags: needinfo?(mats)

Meh, I don't think so. It's low-volume and not exploitable.

Depends on: 1680406
Flags: needinfo?(mats)
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Whiteboard: [adv-main86+r]
Regressions: 1703999
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: