Closed Bug 468771 (CVE-2009-3981) Opened 16 years ago Closed 16 years ago

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


(Core :: Layout, defect, P3)






(Reporter: jruderman, Assigned: roc)



(5 keywords, Whiteboard: [sg:critical?])


(3 files)

Attached file 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.
The usual suspects, the block reflow reports incompleteness, the table starts to split, but as it is not in pagination mode, this goes wrong.
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
Whiteboard: [sg:critical?]
Should be fairly easy to fix once we figure out where the incompleteness is being triggered.
Flags: blocking1.9.1?
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
Flags: wanted1.9.1+
Flags: blocking1.9.1-
Flags: blocking1.9.1+
Attached patch fixSplinter Review
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]

Attachment #361492 - Flags: superreview+
Attachment #361492 - Flags: review?(dbaron)
Attachment #361492 - Flags: review+
Whiteboard: [sg:critical?][needs review] → [sg:critical?][needs landing]
Pushed I withheld the testcase.
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?][needs landing] → [sg:critical?][needs 191 landing]
Depends on: 478131
Comment on attachment 361492 [details] [diff] [review]

Attachment #361492 - Flags: approval1.9.1? → approval1.9.1+
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 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]

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

Approved for, 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 build on OS X. Have we seen these asserts on the 1.9.0 branch?
Alias: CVE-2009-3981
Group: core-security
Crashtest added:
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.