Bug 468771 (CVE-2009-3981)

"ASSERTION: the pointer to this sibling will be overwritten" with -moz-column, table

RESOLVED FIXED

Status

()

Core
Layout
P3
critical
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: roc)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
assertion, crash, fixed1.9.0.16, fixed1.9.1, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +
blocking1.9.0.16 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
Created attachment 352264 [details]
testcase

###!!! ASSERTION: the pointer to this sibling will be overwritten: '!aNewFrame->GetNextSibling()', file /Users/jruderman/central/layout/generic/nsFrameList.cpp, line 176

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/central/layout/base/nsPresShell.cpp, line 675

Likely exploitable.  The testcase doesn't cause a crash, but a variant (created with the usual trick) makes Firefox dereference 0xdadadaf6.

Comment 1

9 years ago
The usual suspects, the block reflow reports incompleteness, the table starts to split, but as it is not in pagination mode, this goes wrong.

Comment 2

9 years ago
the crash is preceeded by:
###!!! ASSERTION: Shouldn't be incomplete if availableHeight is UNCONSTRAINED.:
'aReflowState.availableHeight != NS_UNCONSTRAINEDSIZE', file d:/moz_src/src/layo
ut/generic/nsBlockFrame.cpp, line 1413

Comment 3

9 years ago
Created attachment 352271 [details]
testcase for the incomplete assert
(Reporter)

Updated

9 years ago
Whiteboard: [sg:critical?]
Should be fairly easy to fix once we figure out where the incompleteness is being triggered.
Flags: blocking1.9.1?

Comment 5

9 years ago
we trigger it by asking to reisze to a very small width of the column frame

ColumnSet(div)(0) 05CCE828 Reflow a=60,UC c=60,UC h-resize v-resize cnt=10999
            block 05CCE774 Reflow a=60,1200 c=60,UC dirty v-resize nif=05CCF8A4 cnt=11000
             text 05CCE9F0 Reflow a=60,UC c=UC,UC dirty v-resize nif=05CCF838 cnt=11001
             text 05CCE9F0 Reflow d=600,1140 status=0x1
             text 05CCF838 Reflow a=60,UC c=UC,UC dirty v-resize pif=05CCE9F0 cnt=11002
             text 05CCF838 Reflow d=660,1140
            block 05CCE774 Reflow d=60,1200 status=0x3o=(0,0) 600 x 1200sto=(0,0) 600 x 1200
           ColumnSet(div)(0) 05CCE828 Reflow d=60,1200 status=0x3o=(0,0) 600 x 1200sto=(0,0) 600 x 1200
###!!! ASSERTION: Shouldn't be incomplete if availableHeight is UNCONSTRAINED.:'aReflowState.availableHeight != NS_UNCONSTRAINEDSIZE', file d:/moz_src/src/layout/generic/nsBlockFrame.cpp, line 1413
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P3
FWIW: I hit the assertions from comment 0 and comment 2 on the first testcase, and the incomplete assert on the second testcase, using my linux mozilla-central debug build (updated yesterday).

Platform --> ALL/ALL
OS: Mac OS X → All
Hardware: x86 → All
FWIW, the testcase on bug 435664 triggers this bug's assertion ("the pointer to this sibling will be overwritten") when printed / print-previewed.  (It also crashes, which is what that bug is primarily about.)  See bug 435664 comment 13.  Possibly related to this bug?
I think the problem here is that the column set is propagating incompleteness that's ok inside of it to outside of it... although maybe incompleteness isn't ok inside the last column of a column set.
Assignee: nobody → roc
Blocks: 430569
Flags: wanted1.9.1+
Flags: blocking1.9.1-
Flags: blocking1.9.1+
Blocks: 476579
Created attachment 361492 [details] [diff] [review]
fix

The comment should help explain what's going on.

Basically we're just changing the number of columns from 2 to 1. This should mean that the first column pulls all the content out of the second column, but that doesn't happen because we skip reflowing the first column. We need to tweak the incremental reflow test so that we always reflow the last column if we're giving it unbounded height; it will want to pull content from its next-sibling.
Attachment #361492 - Flags: review?(dbaron)
Whiteboard: [sg:critical?] → [sg:critical?][needs review]
Comment on attachment 361492 [details] [diff] [review]
fix

r+sr=dbaron
Attachment #361492 - Flags: superreview+
Attachment #361492 - Flags: review?(dbaron)
Attachment #361492 - Flags: review+
Whiteboard: [sg:critical?][needs review] → [sg:critical?][needs landing]
This fixes bug 476579 too.
Pushed http://hg.mozilla.org/mozilla-central/rev/c0dbc2a40bb1. I withheld the testcase.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?][needs landing] → [sg:critical?][needs 191 landing]
Attachment #361492 - Flags: approval1.9.1?
Duplicate of this bug: 476579
(Reporter)

Updated

9 years ago
Depends on: 478131
Comment on attachment 361492 [details] [diff] [review]
fix

a1.9.1=dbaron
Attachment #361492 - Flags: approval1.9.1? → approval1.9.1+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6f907ad90647
Keywords: fixed1.9.1
Whiteboard: [sg:critical?][needs 191 landing] → [sg:critical?]
Although the assertion doesn't occur on the 1.9.0 branch the code being patched looks the same. Do we need this fix on the 1.9.0 branch as well?
Flags: blocking1.9.0.16?
Blocking 1.9.0 on the assumption the answer to comment 16 is "yes, we do".
Flags: blocking1.9.0.16? → blocking1.9.0.16+
Code-freeze for 1.9.0.16 is next Tuesday. If this patch works as-is please request approval, else we need a back-port patch. Thanks.
Whiteboard: [sg:critical?] → [sg:critical?][needs 1.9.0 patch]
Comment on attachment 361492 [details] [diff] [review]
fix

Patch applies as-is.
Attachment #361492 - Flags: approval1.9.0.16?
Comment on attachment 361492 [details] [diff] [review]
fix

Approved for 1.9.0.16, a=dveditz for release-drivers
Attachment #361492 - Flags: approval1.9.0.16? → approval1.9.0.16+
Whiteboard: [sg:critical?][needs 1.9.0 patch] → [sg:critical?][needs 1.9.0 landing]
Checked into 1.9.0.
Keywords: fixed1.9.0.16
Whiteboard: [sg:critical?][needs 1.9.0 landing] → [sg:critical?]
Neither of the two testcases trigger the assert in my debug 1.9.0.15 build on OS X. Have we seen these asserts on the 1.9.0 branch?
Alias: CVE-2009-3981
Group: core-security
(Reporter)

Comment 23

7 years ago
Crashtest added: http://hg.mozilla.org/mozilla-central/rev/d3e7c18f58fc
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.