Closed Bug 1061028 Opened 6 years ago Closed 6 years ago

Type confusion in nsCSSFrameConstructor::RemoveFirstLetterFrames

Categories

(Core :: Layout, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox32 --- wontfix
firefox33 --- affected
firefox34 --- affected
firefox35 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: inferno, Assigned: mats)

References

Details

(Keywords: regression, sec-other, testcase, Whiteboard: [ubsan][adv-main35-])

Attachments

(2 files)

Attached file 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
Attached patch fixSplinter Review
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
Assignee: nobody → mats
Attachment #8482411 - Flags: review?(roc)
Component: CSS Parsing and Computation → Layout
The static_cast<nsContainerFrame*> was added by bug 508665 Part 4.
Blocks: 508665
Keywords: regression, testcase
Depends on: 1062907
Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ef6a1da4eeb
Group: core-security
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
No longer depends on: 1062907
Whiteboard: [ubsan] → [ubsan][adv-main35-]
You need to log in before you can comment on or make changes to this bug.