Last Comment Bug 387460 - [FIX]Crash [@ nsCellMapColumnIterator::GetNextFrame] with DOMAttrModified and changing rowspan
: [FIX]Crash [@ nsCellMapColumnIterator::GetNextFrame] with DOMAttrModified and...
[sg:critical?] possibly exploitable
: crash, fixed1.8.0.14, regression, testcase, verified1.8.1.8
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
Depends on:
Blocks: stirtable 352461
  Show dependency treegraph
Reported: 2007-07-09 13:20 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2011-06-13 10:01 PDT (History)
11 users (show)
dveditz: blocking1.8.1.8+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (492 bytes, application/xhtml+xml)
2007-07-09 13:20 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Trunk fix (3.81 KB, patch)
2007-07-09 23:05 PDT, Boris Zbarsky [:bz] (still a bit busy)
jonas: review+
jonas: superreview+
Details | Diff | Splinter Review
Branch patch (3.82 KB, patch)
2007-07-09 23:12 PDT, Boris Zbarsky [:bz] (still a bit busy)
jonas: review+
jonas: superreview+
dveditz: approval1.8.1.8+
dveditz: approval1.8.0.14+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2007-07-09 13:20:45 PDT
Created attachment 271557 [details]

See testcase which crashes current trunk build after 200ms.

This regressed between 2007-01-21 and 2007-01-22:
I guess a regression from bug 352461.
0  	nsCellMapColumnIterator::GetNextFrame(int *,int *)
1 	BasicTableLayoutStrategy::ComputeColumnIntrinsicWidths(nsIRenderingContext *)
2 	BasicTableLayoutStrategy::ComputeIntrinsicWidths(nsIRenderingContext *)
3 	BasicTableLayoutStrategy::GetMinWidth(nsIRenderingContext *)
4 	nsTableFrame::TableShrinkWidthToFit(nsIRenderingContext *,int)
5 	nsTableFrame::ComputeAutoSize(nsIRenderingContext *,nsSize,int,nsSize,nsSize,nsSize,int)
6 	nsFrame::ComputeSize(nsIRenderingContext *,nsSize,int,nsSize,nsSize,nsSize,int)
7 	nsTableFrame::ComputeSize(nsIRenderingContext *,nsSize,int,nsSize,nsSize,nsSize,int)
8 	nsHTMLReflowState::InitConstraints(nsPresContext *,int,int,nsMargin const *,nsMargin const *)
9 	nsHTMLReflowState::Init(nsPresContext *,int,int,nsMargin const *,nsMargin const *)
Comment 1 Bernd 2007-07-09 21:39:39 PDT
Bug 352461 is seldom the cause but rather exposing a underlying problem in the cellmap. Why does the testcase need  
window.addEventListener('DOMAttrModified', function(e){document.getElementById('a').focus();}, true); 
Is that a good old layout breakpoint? Or can it be replaced by one?

Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2007-07-09 22:30:42 PDT
Anything that triggers a layout flush in there will cause a crash.  document.body.offsetWidth does.

I should have a patch soon; doing some CVS archeology.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2007-07-09 23:05:35 PDT
Created attachment 271620 [details] [diff] [review]
Trunk fix

This has been broken ever since mutation events landed.  Basically, we're firing the mutation event after changing the attr value but before calling AttributeChanged, so some data structures (in particular the cellmap) are not getting updated as they should before the mutation event runs arbitrary script.

This might well be exploitable if one tries hard enough...
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2007-07-09 23:12:24 PDT
Created attachment 271622 [details] [diff] [review]
Branch patch
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2007-07-10 18:15:16 PDT
Fixed on trunk.
Comment 6 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-07-19 14:19:33 PDT
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre
Comment 7 Daniel Veditz [:dveditz] 2007-08-28 10:52:02 PDT
Comment on attachment 271622 [details] [diff] [review]
Branch patch

approved for and, a=dveditz for release-drivers
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2007-08-29 13:43:01 PDT
Fixed on branches.
Comment 9 Carsten Book [:Tomcat] 2007-08-30 10:21:30 PDT
verified fixed using Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/2007083003 BonEcho/ and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070830 BonEcho/ ID:2007083003 and the testcase this bug.

No crash on testcase - adding verified keyword.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2007-08-31 00:00:08 PDT
Filed bug 394418 on a similar issue.
Comment 11 Al Billings [:abillings] 2007-12-10 18:02:20 PST
I can't repro the crash with the released Firefox (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070508 Firefox/

Because of this, I can't verify that the fix works in the current 1.8.0 branch builds. Is there an alternative test case?
Comment 12 Al Billings [:abillings] 2007-12-11 17:44:55 PST
It doesn't crash in Windows FF either: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20070508 Firefox/
Comment 13 Bob Clary [:bc:] 2009-04-24 10:39:45 PDT
crash test landed

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