774548 - (CVE-2012-3957) Heap-buffer-overflow in nsBlockFrame::MarkLineDirty

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Reproduces on trunk.

=================================================================
==25348== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7f07b3382ab4 at pc 0x7f07e1f45e62 bp 0x7fff73ddf550 sp 0x7fff73ddf548
READ of size 4 at 0x7f07b3382ab4 thread T0
    #0 0x7f07e1f45e62 in nsLineBox::IsInline() const asn1cmn.c:0
    #1 0x7f07e1f45647 in nsBlockFrame::MarkLineDirty(nsLineList_iterator, nsLineList const*) layout/generic/nsBlockFrame.cpp:1562
    #2 0x7f07e1f82d34 in nsBlockFrame::AddFrames(nsFrameList&, nsIFrame*) layout/generic/nsBlockFrame.cpp:4990
    #3 0x7f07e1f8089d in nsBlockFrame::InsertFrames(mozilla::layout::FrameChildListID, nsIFrame*, nsFrameList&) layout/generic/nsBlockFrame.cpp:4838
    #4 0x7f07e1b38e92 in nsFrameManager::InsertFrames(nsIFrame*, mozilla::layout::FrameChildListID, nsIFrame*, nsFrameList&) layout/base/nsFrameManager.cpp:463
    #5 0x7f07e18a76f0 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool) layout/base/nsCSSFrameConstructor.cpp:7332
    #6 0x7f07e18a06a9 in nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent*, nsILayoutHistoryState*, bool) layout/base/nsCSSFrameConstructor.cpp:6814
    #7 0x7f07e1890a69 in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, bool) layout/base/nsCSSFrameConstructor.cpp:9263
    #8 0x7f07e18c0772 in nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList&) layout/base/nsCSSFrameConstructor.cpp:8008
    #9 0x7f07e18c6afd in nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, bool) layout/base/nsCSSFrameConstructor.cpp:8162
    #10 0x7f07e181129d in mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint) layout/base/RestyleTracker.cpp:125
    #11 0x7f07e180b8d6 in mozilla::css::RestyleTracker::DoProcessRestyles() layout/base/RestyleTracker.cpp:210
    #12 0x7f07e18ee7ba in mozilla::css::RestyleTracker::ProcessRestyles() layout/base/RestyleTracker.h:70
    #13 0x7f07e18ee2ee in nsCSSFrameConstructor::ProcessPendingRestyles() layout/base/nsCSSFrameConstructor.cpp:11996
    #14 0x7f07e1cc088e in PresShell::FlushPendingNotifications(mozFlushType) layout/base/nsPresShell.cpp:3814
    #15 0x7f07e21a5d0c in nsGfxScrollFrameInner::AsyncScrollPortEvent::Run() layout/generic/nsGfxScrollFrame.cpp:2990
    #16 0x7f07e1c4ea65 in nsRootPresContext::FlushWillPaintObservers() layout/base/nsPresContext.cpp:2807
    #17 0x7f07e1c621b9 in nsRootPresContext::RunWillPaintObservers::Run() layout/base/nsPresContext.h:1381
    #18 0x7f07ecd040dd in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:625
    #19 0x7f07ec992e8d in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:217
    #20 0x7f07eb980ea6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
    #21 0x7f07ecfb80ba in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:209
    #22 0x7f07ecfb7f03 in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:202
    #23 0x7f07ecfb7de8 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:176
    #24 0x7f07eae9161e in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:165
    #25 0x7f07e9ae0f78 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:257
    #26 0x7f07e025e2a0 in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3787
    #27 0x7f07e0264c42 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3864
    #28 0x7f07e0268112 in XRE_main toolkit/xre/nsAppRunner.cpp:3940
    #29 0x40c28f in do_main(int, char**) browser/app/nsBrowserApp.cpp:160
    #30 0x409cbd in main browser/app/nsBrowserApp.cpp:298
    #31 0x7f07fcc21c4d in ?? ??:0
0x7f07b3382ab4 is located 20 bytes to the right of 32-byte region [0x7f07b3382a80,0x7f07b3382aa0)
allocated by thread T0 here:
    #0 0x4a4452 in __interceptor_malloc ??:0
    #1 0x7f07f9aad717 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54
    #2 0x7f07e1f66f4b in nsBlockFrame::PushLines(nsBlockReflowState&, nsLineList_iterator) layout/generic/nsBlockFrame.cpp:4414
    #3 0x7f07e1f7630c in nsBlockFrame::PlaceLine(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFloatManager::SavedState*, nsRect&, int&, bool*) layout/generic/nsBlockFrame.cpp:4325
    #4 0x7f07e1f6cb5e in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3764
    #5 0x7f07e1f5d897 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3482
    #6 0x7f07e1f4c24c in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:2570
    #7 0x7f07e1f316b1 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2020
    #8 0x7f07e1f2514f in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1069
    #9 0x7f07e2014c27 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:906
    #10 0x7f07e1ff6b35 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) layout/generic/nsColumnSetFrame.cpp:665
    #11 0x7f07e1ffdcd7 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsColumnSetFrame.cpp:920
    #12 0x7f07e1fb53bb in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) layout/generic/nsBlockReflowContext.cpp:262
    #13 0x7f07e1f566b7 in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3206
    #14 0x7f07e1f4bda6 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:2514
    #15 0x7f07e1f316b1 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2020
    #16 0x7f07e1f2514f in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1069
    #17 0x7f07e2014c27 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:906
    #18 0x7f07e1ff6b35 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) layout/generic/nsColumnSetFrame.cpp:665
    #19 0x7f07e1ffdcd7 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsColumnSetFrame.cpp:920
    #20 0x7f07e2014c27 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:906
    #21 0x7f07e21e5eb7 in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsCanvasFrame.cpp:429
    #22 0x7f07e2014c27 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:906
    #23 0x7f07e215f54e in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:518
    #24 0x7f07e2159c72 in nsHTMLScrollFrame::TryLayout(ScrollReflowState*, nsHTMLReflowMetrics*, bool, bool, bool, unsigned int*) layout/generic/nsGfxScrollFrame.cpp:363
==25348== ABORTING
Stats: 152M malloced (168M for red zones) by 363291 calls
Stats: 39M realloced by 19415 calls
Stats: 109M freed by 231030 calls
Stats: 0M really freed by 0 calls
Stats: 344M (88112 full pages) mmaped in 86 calls
  mmaps   by size class: 8:294894; 9:49146; 10:20475; 11:18423; 12:3072; 13:2048; 14:1536; 15:384; 16:576; 17:128; 18:176; 19:40; 20:12;
  mallocs by size class: 8:274461; 9:47542; 10:16571; 11:17710; 12:2504; 13:1857; 14:1429; 15:321; 16:571; 17:101; 18:172; 19:40; 20:12;
  frees   by size class: 8:162364; 9:36580; 10:13056; 11:14376; 12:1553; 13:942; 14:1230; 15:269; 16:470; 17:91; 18:52; 19:38; 20:9;
  rfrees  by size class:
Stats: malloc large: 325 small slow: 1903
Shadow byte and word:
  0x1fe0f6670556: fb
  0x1fe0f6670550: 00 00 00 00 fb fb fb fb
More shadow bytes:
  0x1fe0f6670530: fd fd fd fd fd fd fd fd
  0x1fe0f6670538: fd fd fd fd fd fd fd fd
  0x1fe0f6670540: fa fa fa fa fa fa fa fa
  0x1fe0f6670548: fa fa fa fa fa fa fa fa
=>0x1fe0f6670550: 00 00 00 00 fb fb fb fb
  0x1fe0f6670558: fb fb fb fb fb fb fb fb
  0x1fe0f6670560: fa fa fa fa fa fa fa fa
  0x1fe0f6670568: fa fa fa fa fa fa fa fa
  0x1fe0f6670570: 00 00 00 00 00 00 00 00

Comment 1

[Daniel Veditz [:dveditz]]

The code where the problem was flagged hasn't changed in forever, the closest was a change a couple lines away to fix a security bug in Firefox 3.0.4 (bug 443528). Something may have changed elsewhere recently that may be part of the problem, though.

In a debug build I get

###!!! ABORT: running past end: 'mCurrent != mListLink', file /Users/daniel/dev/ffcentral/mozilla/layout/base/../generic/nsLineBox.h, line 681

... so you shouldn't need an ASan build to figure this one out or verify it later.

Comment 2

[Mats Palmgren (inactive)]

Attachment: stack + frame dump

The prev sibling is on an overflow line and MarkLineDirty
assumes it's on a normal line unless an explicit aLineList
is provided.

nsBlockFrame::AddFrames already has the correct line list
and propagating it fixes the crash.

Comment 3

[Mats Palmgren (inactive)]

Attachment: fix

We should probably just require the caller to provide
the line list for aLine,  or add a bit to lines saying
which list they belong to (I think we have pretty good
control over when lines are pushed/pulled and we only
do one at a time iirc).

https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=b0bb71a96909

Comment 4

[Mats Palmgren (inactive)]

(Note: I haven't tested the patch in an ASAN build yet)

Comment 5

[Mats Palmgren (inactive)]

Attachment: fix, v2

The first Try run was green.  There's a call earlier in the same method
that we also should fix.  This patch is intended also for branches.

https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=7747fcf62833

I'll file a follow-up bug on making aLineList mandatory for MarkLineDirty and
removing the "nsnull means mLines" association, but nsBlockInFlowLineIterator
depends on the latter (b/c nsBlockInFlowLineIterator::GetLineList() returns
nsnull when mLine is a normal line).

Comment 6

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0842d651cfa8

Filed follow-up bug 774794.

Comment 7

[Mats Palmgren (inactive)]

(Tested ok in Linux64 ASAN build remotely with the testcase)

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0842d651cfa8

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 643064 [details] [diff] [review]
fix, v2

Review of attachment 643064 [details] [diff] [review]:
-----------------------------------------------------------------

We should take this security fix on Aurora and Beta.

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 643064 [details] [diff] [review]
fix, v2

Low risk sec-critical crash fix.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a989cf247c69
https://hg.mozilla.org/releases/mozilla-beta/rev/5d5f3c4cd2d7
https://hg.mozilla.org/releases/mozilla-esr10/rev/b86437f03200

Comment 12

[u279076]

Does the attached testcase need to go in-testsuite?

Comment 13

[Mats Palmgren (inactive)]

Yes, we should land the test when the bug is public, thanks.