Closed Bug 1459670 Opened 7 years ago Closed 6 years ago

use-after-poison in [@ AnyContentAncestorModified]

Categories

(Core :: Web Painting, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 + fixed
firefox62 + fixed

People

(Reporter: tsmith, Assigned: mattwoodrow)

References

(Blocks 2 open bugs)

Details

(4 keywords)

Crash Data

Attachments

(3 files, 1 obsolete file)

Attached file testcase.html (obsolete) (deleted) —
Found with m-c: BuildID=20180507140941 SourceStamp=93443d36d4bd53dba004f7b73430879f96daa681 ==4110==ERROR: AddressSanitizer: use-after-poison on address 0x625000332a56 at pc 0x7f7d46f3ede0 bp 0x7ffc1d8a6390 sp 0x7ffc1d8a6388 READ of size 2 at 0x625000332a56 thread T0 (file:// Content) #0 0x7f7d46f3eddf in IsFrameModified src/layout/generic/nsIFrame.h:4135:35 #1 0x7f7d46f3eddf in AnyContentAncestorModified src/layout/painting/RetainedDisplayListBuilder.cpp:193 #2 0x7f7d46f3eddf in HasModifiedFrame src/layout/painting/RetainedDisplayListBuilder.cpp:332 #3 0x7f7d46f3eddf in MergeState::ProcessOldNode(Index<OldListUnits>, nsTArray<Index<MergedListUnits> >&&) src/layout/painting/RetainedDisplayListBuilder.cpp:362 #4 0x7f7d46e37838 in MergeState::Finalize() src/layout/painting/RetainedDisplayListBuilder.cpp:308:7 #5 0x7f7d46e35ea2 in RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList*, RetainedDisplayList*, RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&) src/layout/painting/RetainedDisplayListBuilder.cpp:490:26 #6 0x7f7d46e3dccc in RetainedDisplayListBuilder::AttemptPartialUpdate(unsigned int, mozilla::DisplayListChecker*) src/layout/painting/RetainedDisplayListBuilder.cpp:1097:7 #7 0x7f7d465fd93b in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) src/layout/base/nsLayoutUtils.cpp:3679:40 #8 0x7f7d464f0305 in mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) src/layout/base/PresShell.cpp:6351:5 #9 0x7f7d45e7ea9a in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) src/view/nsViewManager.cpp:480:19 #10 0x7f7d45e7d89c in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) src/view/nsViewManager.cpp:412:33 #11 0x7f7d45e82ef6 in nsViewManager::ProcessPendingUpdates() src/view/nsViewManager.cpp:1102:5 #12 0x7f7d464679d4 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2067:11 #13 0x7f7d46474f60 in TickDriver src/layout/base/nsRefreshDriver.cpp:337:13 #14 0x7f7d46474f60 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:307 #15 0x7f7d46474b26 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:329:5 #16 0x7f7d4647789e in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:770:5 #17 0x7f7d4647789e in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:683 #18 0x7f7d4647749e in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:584:9 #19 0x7f7d46d1f0bf in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:68:16 #20 0x7f7d3faaedc4 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20 #21 0x7f7d3f986cd3 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1988:28 #22 0x7f7d3f4f6b6e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2141:25 #23 0x7f7d3f4f3b36 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2071:17 #24 0x7f7d3f4f52ec in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1917:5 #25 0x7f7d3f4f5948 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1950:15 #26 0x7f7d3e604ec9 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14 #27 0x7f7d3e620900 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10 #28 0x7f7d3f4fe6c6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:125:5 #29 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #30 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #31 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #32 0x7f7d45f0c57a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27 #33 0x7f7d4a180f7b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22 #34 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #35 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #36 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #37 0x7f7d4a180940 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34 #38 0x4f1875 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30 #39 0x4f1875 in main src/browser/app/nsBrowserApp.cpp:280 #40 0x7f7d5dd8a82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291 #41 0x420f48 in _start (firefox+0x420f48) 0x625000332a56 is located 4438 bytes inside of 8192-byte region [0x625000331900,0x625000333900) allocated by thread T0 (file:// Content) here: #0 0x4c1c93 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3 #1 0x7f7d3e5b3ec3 in AllocateChunk src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:193:15 #2 0x7f7d3e5b3ec3 in InternalAllocate src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:228 #3 0x7f7d3e5b3ec3 in Allocate src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:75 #4 0x7f7d3e5b3ec3 in mozilla::ArenaAllocator<8192ul, 8ul>::Allocate(unsigned long) src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:80 #5 0x7f7d466d0ecf in AllocateByFrameID src/layout/base/nsPresArena.h:39:12 #6 0x7f7d466d0ecf in AllocateFrame src/obj-firefox/dist/include/nsIPresShell.h:208 #7 0x7f7d466d0ecf in operator new src/layout/generic/ViewportFrame.cpp:34 #8 0x7f7d466d0ecf in NS_NewViewportFrame(nsIPresShell*, mozilla::ComputedStyle*) src/layout/generic/ViewportFrame.cpp:31 #9 0x7f7d46560eca in nsCSSFrameConstructor::ConstructRootFrame() src/layout/base/nsCSSFrameConstructor.cpp:2705:5 #10 0x7f7d464bb554 in mozilla::PresShell::Initialize() src/layout/base/PresShell.cpp:1800:36 #11 0x7f7d416c1172 in nsContentSink::StartLayout(bool) src/dom/base/nsContentSink.cpp:1270:26 #12 0x7f7d405a2f36 in nsHtml5TreeOpExecutor::StartLayout(bool*) src/parser/html/nsHtml5TreeOpExecutor.cpp:674:18 #13 0x7f7d4059e48b in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*) src/parser/html/nsHtml5TreeOperation.cpp:1199:17 #14 0x7f7d4059b416 in nsHtml5TreeOpExecutor::RunFlushLoop() src/parser/html/nsHtml5TreeOpExecutor.cpp:490:17 #15 0x7f7d405a74cb in nsHtml5ExecutorFlusher::Run() src/parser/html/nsHtml5StreamParser.cpp:121:18 #16 0x7f7d3e5e60d1 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:337:32 #17 0x7f7d3e604ec9 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14 #18 0x7f7d3e620900 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10 #19 0x7f7d3f4fe6da in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21 #20 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #21 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #22 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #23 0x7f7d45f0c57a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27 #24 0x7f7d4a180f7b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22 #25 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #26 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #27 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #28 0x7f7d4a180940 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34 #29 0x4f1875 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30 #30 0x4f1875 in main src/browser/app/nsBrowserApp.cpp:280 #31 0x7f7d5dd8a82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
Flags: in-testsuite?
Attached file prefs.js (deleted) —
Helps with repro
This also only seems to repro with GNOME_ACCESSIBILITY=1
Calling it sec-high out of caution. Can't tell from just this stack whether it's a simple safely frame-poisoned frame, or if the display list is pointing at something that isn't a frame at all. Should be easy to tell in a debugger though and then we can downgrade the severity if it's not that bad.
Keywords: sec-high
Blocks: RDLbugs
Matt, have you had a chance to look at this further?
Flags: needinfo?(matt.woodrow)
I can't get this one to reproduce unfortunately.
Flags: needinfo?(matt.woodrow)
Attached file testcase.html (deleted) —
Give this a shot. I could reproduce it consistently with testcase but it did take up to a minute sometimes. I was using m-c: BuildID=20180529095249 SourceStamp=f01bb6245db1ea2a87e5360104a4110571265137
Attachment #8973746 - Attachment is obsolete: true
Flags: needinfo?(matt.woodrow)
Perfect, thanks! Got a good understanding of this now, will try get a patch up before bed.
Flags: needinfo?(matt.woodrow)
Assignee: nobody → matt.woodrow
Attached patch table-frame-lifetimes (deleted) — Splinter Review
Basically the same as bug 1452464, just handling the table frame pointers.
Attachment #8982148 - Flags: review?(mikokm)
(In reply to Daniel Veditz [:dveditz] from comment #3) > Calling it sec-high out of caution. Can't tell from just this stack whether > it's a simple safely frame-poisoned frame, or if the display list is > pointing at something that isn't a frame at all. Should be easy to tell in a > debugger though and then we can downgrade the severity if it's not that bad. It is indeed a guaranteed to be a frame pointer that's UAF, so should always be poisoned (or reused for another frame of the exact same type). I think the sec rating can be lowered based on that.
Flags: needinfo?(dveditz)
Comment on attachment 8982148 [details] [diff] [review] table-frame-lifetimes Review of attachment 8982148 [details] [diff] [review]: ----------------------------------------------------------------- This pattern with multiple invalidation frames seems to be repeating in quite a few places. Maybe we could refactor this later to use SmallPointerArray instead.
Attachment #8982148 - Flags: review?(mikokm) → review+
Group: layout-core-security
Flags: needinfo?(dveditz)
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6a768ef5b02e Add frame tracking for the nsDisplayTable* classes. r=miko
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Please request Beta approval on this when you get a chance.
Flags: needinfo?(matt.woodrow)
Flags: in-testsuite?
Flags: in-testsuite-
Comment on attachment 8982148 [details] [diff] [review] table-frame-lifetimes Approval Request Comment [Feature/Bug causing the regression]: Retained-dl [User impact if declined]: Possible crashes/UAF from table mutations. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: Tested with the fuzzer test, can't reproduce the issue. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Hooks up the extra nsIFrame references to the frame tracking infrastructure that we've used without issue for similar bugs in the past. [String changes made/needed]: None.
Flags: needinfo?(matt.woodrow)
Attachment #8982148 - Flags: approval-mozilla-beta?
Comment on attachment 8982148 [details] [diff] [review] table-frame-lifetimes RDL crash fix. Approved for 61.0b12.
Attachment #8982148 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: