Last Comment Bug 774548 - (CVE-2012-3957) Heap-buffer-overflow in nsBlockFrame::MarkLineDirty
(CVE-2012-3957)
: Heap-buffer-overflow in nsBlockFrame::MarkLineDirty
Status: RESOLVED FIXED
[asan][advisory-tracking+][qa?]
: crash, csectype-bounds, sec-critical
Product: Core
Classification: Components
Component: Layout: Block and Inline (show other bugs)
: Trunk
: x86_64 All
: -- critical (vote)
: mozilla17
Assigned To: Mats Palmgren (:mats)
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-16 18:12 PDT by Abhishek Arya
Modified: 2016-12-01 13:38 PST (History)
5 users (show)
rforbes: sec‑bounty+
mats: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
+
fixed
15+
fixed


Attachments
Testcase (745 bytes, text/html)
2012-07-16 18:12 PDT, Abhishek Arya
no flags Details
stack + frame dump (11.81 KB, text/html)
2012-07-17 07:46 PDT, Mats Palmgren (:mats)
no flags Details
fix (871 bytes, patch)
2012-07-17 07:54 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
fix, v2 (1.67 KB, patch)
2012-07-17 11:50 PDT, Mats Palmgren (:mats)
roc: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Abhishek Arya 2012-07-16 18:12:31 PDT
Created attachment 642817 [details]
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] 2012-07-16 23:16:52 PDT
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 (:mats) 2012-07-17 07:46:50 PDT
Created attachment 642948 [details]
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 (:mats) 2012-07-17 07:54:59 PDT
Created attachment 642952 [details] [diff] [review]
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 (:mats) 2012-07-17 07:59:25 PDT
(Note: I haven't tested the patch in an ASAN build yet)
Comment 5 Mats Palmgren (:mats) 2012-07-17 11:50:32 PDT
Created attachment 643064 [details] [diff] [review]
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 (:mats) 2012-07-17 12:59:14 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0842d651cfa8

Filed follow-up bug 774794.
Comment 7 Mats Palmgren (:mats) 2012-07-17 15:13:49 PDT
(Tested ok in Linux64 ASAN build remotely with the testcase)
Comment 8 Ed Morley [:emorley] 2012-07-18 06:13:12 PDT
https://hg.mozilla.org/mozilla-central/rev/0842d651cfa8
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-18 06:38:47 PDT
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 (:mats) 2012-07-20 11:46:37 PDT
Comment on attachment 643064 [details] [diff] [review]
fix, v2

Low risk sec-critical crash fix.
Comment 12 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-08-14 15:24:50 PDT
Does the attached testcase need to go in-testsuite?
Comment 13 Mats Palmgren (:mats) 2012-08-14 18:02:45 PDT
Yes, we should land the test when the bug is public, thanks.

Note You need to log in before you can comment on or make changes to this bug.