1061028 - Type confusion in nsCSSFrameConstructor::RemoveFirstLetterFrames

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Build with latest clang with ubsan vptr (-fsanitize=vptr support and enable rtti (ac_add_options --enable-cpp-rtti, ignore startup false positives). Incorrect type can be checked in regular build as well. UBSAN vptr build is just useful for fuzzing. 

/build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:10554:31: runtime error: downcast of address 0x625000519988 which does not point to an object of type 'nsContainerFrame'
0x625000519988: note: object is of type 'nsTextFrame'
 00 00 00 00  10 61 93 5a 5b 7f 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  50 46 17 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'nsTextFrame'
    #0 0x7f5b53a44a03 in nsCSSFrameConstructor::RemoveFirstLetterFrames(nsPresContext*, nsIPresShell*, nsContainerFrame*, nsContainerFrame*, bool*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:10553:7
    #1 0x7f5b53a26a8e in nsCSSFrameConstructor::RemoveLetterFrames(nsPresContext*, nsIPresShell*, nsContainerFrame*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:10582:12
    #2 0x7f5b53a2cc52 in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags, bool*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:7838:7
    #3 0x7f5b53a1311e in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, bool) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:8963:10
    #4 0x7f5b53a296da in nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState&, nsIFrame*, nsIFrame*, nsCSSFrameConstructor::FrameConstructionItemList&, bool, nsIFrame*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:11349:7
    #5 0x7f5b53a224eb in nsCSSFrameConstructor::ContentAppended(nsIContent*, nsIContent*, bool) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6952:7
    #6 0x7f5b53a1c422 in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6618:5
    #7 0x7f5b53a1c4cc in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6625:7
    #8 0x7f5b53a1c4cc in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6625:7
    #9 0x7f5b53a1c4cc in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6625:7
    #10 0x7f5b53a1c4cc in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6625:7
    #11 0x7f5b53a1c4cc in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6625:7
    #12 0x7f5b53a1c4cc in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6625:7
    #13 0x7f5b53a238c4 in nsCSSFrameConstructor::CreateNeededFrames() /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:6640:5
    #14 0x7f5b5397f22b in mozilla::RestyleManager::ProcessPendingRestyles() /build/firefox/src/layout/base/RestyleManager.cpp:1491:3
    #15 0x7f5b538a4b74 in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) /build/firefox/src/layout/base/nsPresShell.cpp:4224:9
    #16 0x7f5b5391031d in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /build/firefox/src/layout/base/nsRefreshDriver.cpp:1190:11
    #17 0x7f5b5391ae74 in TickDriver /build/firefox/src/layout/base/nsRefreshDriver.cpp:171:5
    #18 0x7f5b5391ae74 in mozilla::RefreshDriverTimer::Tick() /build/firefox/src/layout/base/nsRefreshDriver.cpp:162
    #19 0x7f5b4d259f2d in nsTimerImpl::Fire() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:618:7
    #20 0x7f5b4d25b01b in nsTimerEvent::Run() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:711:3
    #21 0x7f5b4d24a3aa in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:823:7
    #22 0x7f5b4d2be632 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/glue/nsThreadUtils.cpp:265:10
    #23 0x7f5b4def2204 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:99:21
    #24 0x7f5b4de81a10 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:229:3
    #25 0x7f5b4de81a10 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:222
    #26 0x7f5b4de81a10 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:196
    #27 0x7f5b51f2a2c2 in nsBaseAppShell::Run() /build/firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:164:3
    #28 0x7f5b555f5b52 in nsAppStartup::Run() /build/firefox/src/toolkit/components/startup/nsAppStartup.cpp:280:19
    #29 0x7f5b5572bce9 in XREMain::XRE_mainRun() /build/firefox/src/toolkit/xre/nsAppRunner.cpp:4101:10
    #30 0x7f5b5572ce02 in XREMain::XRE_main(int, char**, nsXREAppData const*) /build/firefox/src/toolkit/xre/nsAppRunner.cpp:4172:8
    #31 0x7f5b5572dd9c in XRE_main /build/firefox/src/toolkit/xre/nsAppRunner.cpp:4386:16
    #32 0x4bdedd in do_main /build/firefox/src/browser/app/nsBrowserApp.cpp:282:12
    #33 0x4bdedd in main /build/firefox/src/browser/app/nsBrowserApp.cpp:643
    #34 0x7f5b61ba076c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #35 0x4bd42c in _start

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix

I think this is harmless too.  The recursive call with the text frame
as aFrame will call GetFirstPrincipalChild() which wraps GetChildList
which is virtual.  nsTextFrame inherits nsFrame's implementation
which returns an empty nsFrameList, so 'kid' will be null and we'll
just return NS_OK.

http://hg.mozilla.org/mozilla-central/annotate/532b5fb77ba1/layout/base/nsCSSFrameConstructor.cpp#l10492

Comment 2

[Mats Palmgren (inactive)]

The static_cast<nsContainerFrame*> was added by bug 508665 Part 4.

Comment 3

[Mats Palmgren (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=9c3dff1b08d2
https://tbpl.mozilla.org/?tree=Try&rev=ace5f4c3b47f

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d88bb355ca22

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d88bb355ca22

Comment 6

[Mats Palmgren (inactive)]

Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ef6a1da4eeb

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5ef6a1da4eeb