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]
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]
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 User image Martijn Wargers [:mwargers] 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 User image 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 User image 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 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 2007-07-09 23:12:24 PDT
Created attachment 271622 [details] [diff] [review]
Branch patch
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2007-07-10 18:15:16 PDT
Fixed on trunk.
Comment 6 User image Martijn Wargers [:mwargers] 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 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 2007-08-29 13:43:01 PDT
Fixed on branches.
Comment 9 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 2007-08-31 00:00:08 PDT
Filed bug 394418 on a similar issue.
Comment 11 User image 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 User image 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 User image 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.