Last Comment Bug 468771 - (CVE-2009-3981) "ASSERTION: the pointer to this sibling will be overwritten" with -moz-column, table
(CVE-2009-3981)
: "ASSERTION: the pointer to this sibling will be overwritten" with -moz-column...
Status: RESOLVED FIXED
[sg:critical?]
: assertion, crash, fixed1.9.0.16, fixed1.9.1, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: P3 critical (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
Mentors:
: 476579 (view as bug list)
Depends on: 478131
Blocks: stirtable framedest 430569 476579
  Show dependency treegraph
 
Reported: 2008-12-09 21:04 PST by Jesse Ruderman
Modified: 2010-04-11 17:01 PDT (History)
12 users (show)
roc: blocking1.9.1-
roc: wanted1.9.1+
dveditz: blocking1.9.0.16+
jruderman: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (624 bytes, application/xhtml+xml)
2008-12-09 21:04 PST, Jesse Ruderman
no flags Details
testcase for the incomplete assert (460 bytes, application/xhtml+xml)
2008-12-09 22:20 PST, Bernd
no flags Details
fix (2.68 KB, patch)
2009-02-10 03:36 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
dbaron: review+
dbaron: superreview+
dbaron: approval1.9.1+
dveditz: approval1.9.0.16+
Details | Diff | Splinter Review

Description Jesse Ruderman 2008-12-09 21:04:23 PST
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 Bernd 2008-12-09 22:10:48 PST
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 Bernd 2008-12-09 22:12:08 PST
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 Bernd 2008-12-09 22:20:12 PST
Created attachment 352271 [details]
testcase for the incomplete assert
Comment 4 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-12-09 23:13:25 PST
Should be fairly easy to fix once we figure out where the incompleteness is being triggered.
Comment 5 Bernd 2008-12-10 12:24:19 PST
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
Comment 6 Daniel Holbert [:dholbert] 2008-12-26 11:34:51 PST
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
Comment 7 Daniel Holbert [:dholbert] 2009-01-07 15:22:23 PST
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?
Comment 8 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2009-01-25 12:21:29 PST
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.
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-10 03:36:50 PST
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.
Comment 10 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2009-02-10 09:12:31 PST
Comment on attachment 361492 [details] [diff] [review]
fix

r+sr=dbaron
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-10 12:12:34 PST
This fixes bug 476579 too.
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-11 01:18:05 PST
Pushed http://hg.mozilla.org/mozilla-central/rev/c0dbc2a40bb1. I withheld the testcase.
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-11 01:19:30 PST
*** Bug 476579 has been marked as a duplicate of this bug. ***
Comment 14 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2009-02-25 17:10:33 PST
Comment on attachment 361492 [details] [diff] [review]
fix

a1.9.1=dbaron
Comment 15 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-26 12:58:18 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6f907ad90647
Comment 16 Daniel Veditz [:dveditz] 2009-10-11 00:09:04 PDT
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?
Comment 17 Daniel Veditz [:dveditz] 2009-10-14 15:51:00 PDT
Blocking 1.9.0 on the assumption the answer to comment 16 is "yes, we do".
Comment 18 Daniel Veditz [:dveditz] 2009-11-04 15:50:14 PST
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.
Comment 19 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-11-10 16:33:01 PST
Comment on attachment 361492 [details] [diff] [review]
fix

Patch applies as-is.
Comment 20 Daniel Veditz [:dveditz] 2009-11-10 16:36:31 PST
Comment on attachment 361492 [details] [diff] [review]
fix

Approved for 1.9.0.16, a=dveditz for release-drivers
Comment 21 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-11-10 18:06:55 PST
Checked into 1.9.0.
Comment 22 Al Billings [:abillings] 2009-11-20 17:01:23 PST
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?
Comment 23 Jesse Ruderman 2010-04-11 17:01:43 PDT
Crashtest added: http://hg.mozilla.org/mozilla-central/rev/d3e7c18f58fc

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