Bug 812893 (CVE-2013-0780)

Heap-use-after-free in nsOverflowContinuationTracker::Finish, with -moz-columns

RESOLVED FIXED in Firefox 19

Status

()

Core
Layout
--
critical
RESOLVED FIXED
5 years ago
6 months ago

People

(Reporter: Abhishek Arya, Assigned: mats)

Tracking

(4 keywords)

Trunk
mozilla21
crash, csectype-uaf, sec-critical, testcase
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
in-testsuite +

Firefox Tracking Flags

(firefox18 affected, firefox19+ verified, firefox20+ verified, firefox21+ verified, firefox-esr1719+ verified, b2g1819+ fixed, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [asan][bug 724978 might fix][adv-main19+][adv-esr1703+])

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 682899 [details]
Testcase

Reproduces on trunk.

=================================================================
==3869== ERROR: AddressSanitizer: heap-use-after-free on address 0x7facf171c940 at pc 0x7fad10189afe bp 0x7fff43a32a90 sp 0x7fff43a32a88
READ of size 8 at 0x7facf171c940 thread T0
    #0 0x7fad10189afd in nsFrameList::FirstChild() const src/layout/base/../generic/nsFrameList.h:214
    #1 0x7fad10a06005 in nsOverflowContinuationTracker::Finish(nsIFrame*) src/layout/generic/nsContainerFrame.cpp:1719
    #2 0x7fad109a7a57 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) src/layout/generic/nsBlockReflowContext.cpp:306
    #3 0x7fad10949e9d in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3097
    #4 0x7fad1093ebe9 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2476
    #5 0x7fad1092a670 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:2284
    #6 0x7fad10918fe3 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1041
    #7 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #8 0x7fad109e6ad3 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:649
    #9 0x7fad109eeb11 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsColumnSetFrame.cpp:1032
    #10 0x7fad109a7400 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) src/layout/generic/nsBlockReflowContext.cpp:268
    #11 0x7fad10949e9d in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3097
    #12 0x7fad1093ebe9 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2476
    #13 0x7fad10925cf7 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:1996
    #14 0x7fad10918fe3 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1041
    #15 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #16 0x7fad109e6ad3 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:649
    #17 0x7fad109eeb11 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsColumnSetFrame.cpp:1032
    #18 0x7fad109a7400 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) src/layout/generic/nsBlockReflowContext.cpp:268
    #19 0x7fad10949e9d in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3097
    #20 0x7fad1093ebe9 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2476
    #21 0x7fad10925cf7 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:1996
    #22 0x7fad10918fe3 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1041
    #23 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #24 0x7fad109e6ad3 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:649
    #25 0x7fad109eeb11 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsColumnSetFrame.cpp:1032
    #26 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #27 0x7fad10bd7066 in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsCanvasFrame.cpp:473
    #28 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #29 0x7fad10b4e948 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) src/layout/generic/nsGfxScrollFrame.cpp:433
    #30 0x7fad10b534a6 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) src/layout/generic/nsGfxScrollFrame.cpp:533
    #31 0x7fad10b57b40 in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsGfxScrollFrame.cpp:774
    #32 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #33 0x7fad10f4692a in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsViewportFrame.cpp:202
    #34 0x7fad106799bb in PresShell::DoReflow(nsIFrame*, bool) src/layout/base/nsPresShell.cpp:7502
    #35 0x7fad106a906d in PresShell::ProcessReflowCommands(bool) src/layout/base/nsPresShell.cpp:7643
    #36 0x7fad106a7bbe in PresShell::FlushPendingNotifications(mozFlushType) src/layout/base/nsPresShell.cpp:3883
    #37 0x7fad1074e155 in nsRefreshDriver::Notify(nsITimer*) src/layout/base/nsRefreshDriver.cpp:403
    #38 0x7fad1cc44123 in nsTimerImpl::Fire() src/xpcom/threads/nsTimerImpl.cpp:485
    #39 0x7fad1cc454c1 in nsTimerEvent::Run() src/xpcom/threads/nsTimerImpl.cpp:565
    #40 0x7fad1cc081be in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:627
    #41 0x7fad1c87de0f in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:221
    #42 0x7fad1afe3516 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
    #43 0x7fad1cef0cce in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:215
    #44 0x7fad1cef0b15 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:208
    #45 0x7fad1cef09fb in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:182
    #46 0x7fad1a3e84d4 in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
    #47 0x7fad18f4de12 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:290
    #48 0x7fad0e437f24 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3823
    #49 0x7fad0e43dbf9 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3890
    #50 0x7fad0e440970 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4084
    #51 0x40c0c6 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
    #52 0x409900 in main src/browser/app/nsBrowserApp.cpp:279
    #53 0x7fad2e64e76c in ?? ??:0
0x7facf171c940 is located 0 bytes inside of 16-byte region [0x7facf171c940,0x7facf171c950)
freed by thread T0 here:
    #0 0x4c3750 in __interceptor_free ??:?
    #1 0x7fad2c1dc405 in moz_free src/memory/mozalloc/mozalloc.cpp:48
    #2 0x7fad10a0da58 in operator delete(void*) src/../../dist/include/mozilla/mozalloc.h:224
    #3 0x7fad10a0cef3 in nsContainerFrame::StealFrame(nsPresContext*, nsIFrame*, bool) src/layout/generic/nsContainerFrame.cpp:1212
    #4 0x7fad10984dac in nsBlockFrame::StealFrame(nsPresContext*, nsIFrame*, bool) src/layout/generic/nsBlockFrame.cpp:5538
    #5 0x7fad10a0f51c in nsContainerFrame::DeleteNextInFlowChild(nsPresContext*, nsIFrame*, bool) src/layout/generic/nsContainerFrame.cpp:1359
    #6 0x7fad10986c30 in nsBlockFrame::DeleteNextInFlowChild(nsPresContext*, nsIFrame*, bool) src/layout/generic/nsBlockFrame.cpp:5625
    #7 0x7fad109a7ba2 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) src/layout/generic/nsBlockReflowContext.cpp:307
    #8 0x7fad10949e9d in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3097
    #9 0x7fad1093ebe9 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2476
    #10 0x7fad10925cf7 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:1996
    #11 0x7fad10918fe3 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1041
    #12 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #13 0x7fad109e6ad3 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:649
    #14 0x7fad109eeb11 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsColumnSetFrame.cpp:1032
previously allocated by thread T0 here:
    #0 0x4c3810 in malloc ??:?
    #1 0x7fad2c1dc541 in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:54
    #2 0x7fad10a0b40f in operator new(unsigned long) src/../../dist/include/mozilla/mozalloc.h:200
    #3 0x7fad1094dbbe in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3266
    #4 0x7fad1093ebe9 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2476
    #5 0x7fad1092a670 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:2284
    #6 0x7fad10918fe3 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1041
    #7 0x7fad10a0566c in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:952
    #8 0x7fad109e6ad3 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:649
Shadow byte and word:
  0x1ff59e2e3928: fd
  0x1ff59e2e3928: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ff59e2e3908: 00 00 00 00 00 fb fb fb
  0x1ff59e2e3910: fa fa fa fa fa fa fa fa
  0x1ff59e2e3918: fd fd fd fd fd fd fd fd
  0x1ff59e2e3920: fa fa fa fa fa fa fa fa
=>0x1ff59e2e3928: fd fd fd fd fd fd fd fd
  0x1ff59e2e3930: fa fa fa fa fa fa fa fa
  0x1ff59e2e3938: 00 00 00 00 00 00 00 00
  0x1ff59e2e3940: fa fa fa fa fa fa fa fa
  0x1ff59e2e3948: fd fd fd fd fd fd fd fd
Stats: 252M malloced (231M for red zones) by 340183 calls
Stats: 46M realloced by 17894 calls
Stats: 221M freed by 224349 calls
Stats: 186M really freed by 177540 calls
Stats: 235M (60247 full pages) mmaped in 447 calls
  mmaps   by size class: 7:102375; 8:45034; 9:13299; 10:5110; 11:8670; 12:1280; 13:960; 14:544; 15:224; 16:784; 17:460; 18:36; 19:35; 20:21;
  mallocs by size class: 7:196151; 8:83366; 9:22929; 10:9146; 11:19278; 12:2496; 13:1908; 14:1548; 15:413; 16:1467; 17:1349; 18:70; 19:40; 20:22;
  frees   by size class: 7:122554; 8:54394; 9:16555; 10:5987; 11:17304; 12:1710; 13:1495; 14:1365; 15:281; 16:1256; 17:1334; 18:57; 19:38; 20:19;
  rfrees  by size class: 7:99289; 8:43459; 9:10392; 10:4360; 11:13903; 12:1343; 13:996; 14:1255; 15:215; 16:939; 17:1297; 18:49; 19:37; 20:6;
Stats: malloc large: 3361 small slow: 4876
==3869== ABORTING

Updated

5 years ago
Component: General → Layout
Product: Firefox → Core
(Assignee)

Comment 1

5 years ago
The underlying issue is probably bug 724978.
Other related crash bugs: bug 772320, bug 783228, bug 772306.
Severity: normal → critical
Depends on: 724978
Keywords: crash, testcase
Hardware: x86_64 → All
Summary: Heap-use-after-free in nsOverflowContinuationTracker::Finish → Heap-use-after-free in nsOverflowContinuationTracker::Finish, with -moz-columns
Whiteboard: [asan]
Mats, how bad is this?
If this bad pointer is being read in framelist code why aren't we getting a poisoned value instead of a UAF?
Keywords: sec-critical
Flags: needinfo?(matspal)

Updated

5 years ago
Assignee: nobody → matspal
(Assignee)

Comment 4

5 years ago
I'm pretty sure the underlying problem is bug 724978 comment 1.
(I have no intention of taking that bug.)
Assignee: matspal → nobody
Flags: needinfo?(matspal)

Updated

5 years ago
Assignee: nobody → bugs
If the underlying issue is bug 724978 then this may well affect Firefox 17 or even Fx10. We'll have to test since other intervening changes may have masked the problem in the older releases.
Assignee: bugs → nobody
status-firefox19: --- → affected
status-firefox20: --- → affected
tracking-firefox19: --- → +
tracking-firefox20: --- → +
Flags: sec-bounty?
Assigning to Ehsan since he has made some progress on bug 724978.
Assignee: nobody → ehsan
Sorry I don't have any idea what's going on here...
Assignee: ehsan → nobody

Updated

5 years ago
Whiteboard: [asan] → [asan][bug 724978 might fix]
Scott can you own this one for now and let us know if/when bug 724978 might help us?
Assignee: nobody → sjohnson
(In reply to David Bolter [:davidb] from comment #8)
> Scott can you own this one for now and let us know if/when bug 724978 might
> help us?

Ok... I don't know when bug 724978 will land, though, or even when I'll have a chance to work on it. 

What is the priority of this as compared to bug 724978? If I understand correctly, bug 724978 is not a high priority bug at the moment, but I could be wrong...
(In reply to Scott Johnson (:jwir3) from comment #9)
> (In reply to David Bolter [:davidb] from comment #8)
> > Scott can you own this one for now and let us know if/when bug 724978 might
> > help us?
> 
> Ok... I don't know when bug 724978 will land, though, or even when I'll have
> a chance to work on it. 
> 
> What is the priority of this as compared to bug 724978? If I understand
> correctly, bug 724978 is not a high priority bug at the moment, but I could
> be wrong...

Unless we discover this is not a sec-critical, this bug is very high priority indeed.
mats, one thing that I'm not clear on is why you think this would be fixed by 724978... it doesn't seem like there is abspos stuff in this testcase at all. Then again, I'm not sure how -moz-shape-inside is interacting with the columns.
What are the STR to reproduce this? I don't seem to see it on my trunk builds...


> Shadow byte and word:
>   0x1ff59e2e3928: fd
>   0x1ff59e2e3928: fd fd fd fd fd fd fd fd
> More shadow bytes:
>   0x1ff59e2e3908: 00 00 00 00 00 fb fb fb
>   0x1ff59e2e3910: fa fa fa fa fa fa fa fa
>   0x1ff59e2e3918: fd fd fd fd fd fd fd fd
>   0x1ff59e2e3920: fa fa fa fa fa fa fa fa
> =>0x1ff59e2e3928: fd fd fd fd fd fd fd fd
>   0x1ff59e2e3930: fa fa fa fa fa fa fa fa
>   0x1ff59e2e3938: 00 00 00 00 00 00 00 00
>   0x1ff59e2e3940: fa fa fa fa fa fa fa fa
>   0x1ff59e2e3948: fd fd fd fd fd fd fd fd
> Stats: 252M malloced (231M for red zones) by 340183 calls
> Stats: 46M realloced by 17894 calls
> Stats: 221M freed by 224349 calls
> Stats: 186M really freed by 177540 calls
> Stats: 235M (60247 full pages) mmaped in 447 calls
>    mmaps   by size class: 7:102375; 8:45034; 9:13299; 10:5110; 11:8670; 12:1280; 
> 13:960; 14:544; 15:224; 16:784; 17:460; 18:36; 19:35; 20:21;
>   mallocs by size class: 7:196151; 8:83366; 9:22929; 10:9146; 11:19278; 12:2496; 
> 13:1908; 14:1548; 15:413; 16:1467; 17:1349; 18:70; 19:40; 20:22;
>   frees   by size class: 7:122554; 8:54394; 9:16555; 10:5987; 11:17304; 12:1710; 
> 13:1495; 14:1365; 15:281; 16:1256; 17:1334; 18:57; 19:38; 20:19;
>   rfrees  by size class: 7:99289; 8:43459; 9:10392; 10:4360; 11:13903; 12:1343; 
> 13:996; 14:1255; 15:215; 16:939; 17:1297; 18:49; 19:37; 20:6;
> Stats: malloc large: 3361 small slow: 4876
> ==3869== ABORTING

Is this valgrind output?
(Assignee)

Comment 13

5 years ago
(In reply to Scott Johnson (:jwir3) from comment #11)
> mats, one thing that I'm not clear on is why you think this would be fixed
> by 724978...

I don't recall exactly, but these frames in the reported stack dump:
    #1 0x7fad10a06005 in nsOverflowContinuationTracker::Finish(nsIFrame*)
    #9 0x7fad109eeb11 in nsColumnSetFrame::Reflow
seems to indicate it's the problem we discussed in bug 724978 comment 1.

(In reply to Scott Johnson (:jwir3) from comment #12)
> What are the STR to reproduce this? I don't seem to see it on my trunk
> builds...

Load the testcase in an Address Sanitizer (ASan) build.

> Is this valgrind output?

It's ASan output.  See this link for details:
https://developer.mozilla.org/en-US/docs/Building_Firefox_with_Address_Sanitizer
Any updates on this?
(In reply to Al Billings [:abillings] from comment #14)
> Any updates on this?

Not yet, sorry. I'm trying to determine if this is a regression from something recent (and thus we can fix it by backing this out), or if it's something that's been around for a while.

Based on my bisecting so far, it seems like the former is likely, but it's slow going.
(In reply to Scott Johnson (:jwir3) from comment #15)
> (In reply to Al Billings [:abillings] from comment #14)
> > Any updates on this?
> 
> Based on my bisecting so far, it seems like the former is likely, but it's
> slow going.

Matt should be able to help out. Assigning him as the QA contact.
QA Contact: mwobensmith
I'm not able to see a crash. Using m-c ASan builds from 11/16, 11/19 and 1/7 (most current) all three builds exhibit the same behavior. I'm getting 1200+ warnings and asserts when loading this bug file, but no crash. 

WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 130
nsBlockReflowContext: ColumnSet(form)(4)@0x1212f10f8 metrics=2147482818,1152!
###!!! ASSERTION: Doing nscoord addition with values > nscoord_MAX: 'a < nscoord_MAX && b < nscoord_MAX', file ../../dist/include/nsCoord.h, line 195
WARNING: nscoord addition capped to nscoord_MAX: '(int64_t)a + (int64_t)b < (int64_t)nscoord_MAX', file ../../dist/include/nsCoord.h, line 201

Our ASan archive only goes back a few weeks. I just happened to have the above older builds on my local machine.
(Assignee)

Comment 18

4 years ago
That's odd.  I can reproduce this in my local m-c ASan debug Linux64 build...
(Assignee)

Comment 19

4 years ago
I've been debugging this a bit and I think I know what the problem is --
nsOverflowContinuationTracker::Insert screws up the frame lists when it
moves a frame with continuations to the overflow containers list.

Scott, are you working on this or should I take it?
(In reply to Mats Palmgren [:mats] from comment #19)
> I've been debugging this a bit and I think I know what the problem is --
> nsOverflowContinuationTracker::Insert screws up the frame lists when it
> moves a frame with continuations to the overflow containers list.
> 
> Scott, are you working on this or should I take it?

Nope, feel free to take it. I would like to speak with you about this, when you have a chance, though, just so I can get an idea of how you went about debugging this issue. (For learning purposes).
Assignee: sjohnson → matspal
(Assignee)

Updated

4 years ago
Blocks: 821126
(Assignee)

Comment 21

4 years ago
Created attachment 705378 [details]
bogus frame tree
(Assignee)

Comment 22

4 years ago
Created attachment 705397 [details] [diff] [review]
fix

https://tbpl.mozilla.org/?tree=Try&rev=5f49535956e9
Attachment #705397 - Flags: review?(roc)
(Assignee)

Comment 23

4 years ago
A few notes:
nsOverflowContinuationTracker::Insert is kind of weird in that it
expects a non-NS_FRAME_IS_OVERFLOW_CONTAINER frame to not be on any
frame list and thus no StealFrame is required in that case.

Also, it has the problem that it recurse on OVERFLOW_CONTAINER
continuations and thus can cause stack overflow.

nsOverflowContinuationTracker can point to either the
ExcessOverflowContinuations list or the OverflowContinuations on the
next-in-flow (stored in mParent).  This seems overly complicated.
I think it would be simpler to always add the frames to the
ExcessOverflowContinuations list and then the next-in-flow can
drain that at the start of its ReflowOverflowContainerChildren.
Attachment #705397 - Flags: review?(roc) → review+
(In reply to Mats Palmgren [:mats] from comment #23)
> A few notes:
> nsOverflowContinuationTracker::Insert is kind of weird in that it
> expects a non-NS_FRAME_IS_OVERFLOW_CONTAINER frame to not be on any
> frame list and thus no StealFrame is required in that case.
> 
> Also, it has the problem that it recurse on OVERFLOW_CONTAINER
> continuations and thus can cause stack overflow.
> 
> nsOverflowContinuationTracker can point to either the
> ExcessOverflowContinuations list or the OverflowContinuations on the
> next-in-flow (stored in mParent).  This seems overly complicated.
> I think it would be simpler to always add the frames to the
> ExcessOverflowContinuations list and then the next-in-flow can
> drain that at the start of its ReflowOverflowContainerChildren.

Can you work on these issues? I think it's super-important to simplify this code as much as possible.
(Assignee)

Comment 25

4 years ago
I filed bug 834097 and bug 834096.  I'll try to find some time inbetween other work.
(Assignee)

Comment 26

4 years ago
Comment on attachment 705397 [details] [diff] [review]
fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
I don't think the patch gives any clues at all.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.

Which older supported branches are affected by this flaw?
I'm guessing all, but I haven't checked.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I think the same patch would apply, this code doesn't change much.

How likely is this patch to cause regressions; how much testing does it need?
This code is complicated so I would say moderate risk.
Attachment #705397 - Flags: sec-approval?
status-b2g18: --- → affected
status-firefox21: --- → affected
status-firefox-esr17: --- → affected
tracking-b2g18: --- → ?
tracking-firefox21: --- → +
tracking-firefox-esr17: --- → ?
I'm giving sec-approval but please check to see how far back we're affected.
Attachment #705397 - Flags: sec-approval? → sec-approval+
(Assignee)

Comment 28

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed2ed42808ca
Please nominate for uplift to Aurora/Beta no later than Monday. Please also prepare patches for ESR17 or B2G (if the phone is affected).
https://hg.mozilla.org/mozilla-central/rev/ed2ed42808ca
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox21: affected → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(Assignee)

Comment 31

4 years ago
Comment on attachment 705397 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: sec-critical crash
Testing completed (on m-c, etc.): on m-c since 2013-01-26
Risk to taking this patch (and alternatives if risky): medium risk, no alt.
String or UUID changes made by this patch: none
Attachment #705397 - Flags: approval-mozilla-esr17?
Attachment #705397 - Flags: approval-mozilla-beta?
Attachment #705397 - Flags: approval-mozilla-b2g18?
Attachment #705397 - Flags: approval-mozilla-aurora?

Updated

4 years ago
tracking-b2g18: ? → 19+
tracking-firefox-esr17: ? → 19+
Comment on attachment 705397 [details] [diff] [review]
fix

Approving on branches as this is sec-high and baked well on m-c.Considering the risk is moderate but since we do not have any alt, please involve Matt Wobensmith for any additional testing/verfication that may be helpful here.

Please land no later than tomorrow morning for this to make it into 19.0b4 to get good beta testing.
Attachment #705397 - Flags: approval-mozilla-esr17?
Attachment #705397 - Flags: approval-mozilla-esr17+
Attachment #705397 - Flags: approval-mozilla-beta?
Attachment #705397 - Flags: approval-mozilla-beta+
Attachment #705397 - Flags: approval-mozilla-b2g18?
Attachment #705397 - Flags: approval-mozilla-b2g18+
Attachment #705397 - Flags: approval-mozilla-aurora?
Attachment #705397 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 821126
Flags: sec-bounty? → sec-bounty+
(Assignee)

Comment 35

4 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c99a75cf84ff
https://hg.mozilla.org/releases/mozilla-beta/rev/fc6e52d846fb
https://hg.mozilla.org/releases/mozilla-esr17/rev/82941f956b85
https://hg.mozilla.org/releases/mozilla-b2g18/rev/a3fe771af156
status-b2g18: affected → fixed
status-firefox19: affected → fixed
status-firefox20: affected → fixed
status-firefox-esr17: affected → fixed
As mentioned above in comment 17, I've not been able to see a crash. However, I can see that the assert is gone in the latest builds. The warnings are still present.

Verified on build 2013-01-31, m-c
Verified on build 2013-01-31, Aurora
Verified on beta 19b4
Verified on build 2013-01-30, 17.0.2esr
status-firefox19: fixed → verified
status-firefox20: fixed → verified
status-firefox21: fixed → verified
status-firefox-esr17: fixed → verified
Batch edit: Bugs fixed on b2g18 after 1/25 merge to v1.0 branch are fixed on v1.0.1 branch.
status-b2g18-v1.0.1: --- → fixed
status-firefox18: --- → affected
Whiteboard: [asan][bug 724978 might fix] → [asan][bug 724978 might fix][adv-main19+][adv-esr1703+]
Alias: CVE-2013-0780
Group: core-security
(Assignee)

Comment 38

3 years ago
Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/88df072cf29b
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/88df072cf29b
Keywords: csectype-uaf
You need to log in before you can comment on or make changes to this bug.