[FIX]Crash [@ nsCellMapColumnIterator::GetNextFrame] with DOMAttrModified and changing rowspan

VERIFIED FIXED

Status

()

Core
Layout: Tables
--
critical
VERIFIED FIXED
10 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: bz)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
crash, fixed1.8.0.14, regression, testcase, verified1.8.1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.8 +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] possibly exploitable, crash signature)

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
Created attachment 271557 [details]
testcase

See testcase which crashes current trunk build after 200ms.

This regressed between 2007-01-21 and 2007-01-22:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-21+04&maxdate=2007-01-22+09&cvsroot=%2Fcvsroothttp://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-21+04&maxdate=2007-01-22+09&cvsroot=%2Fcvsroot
I guess a regression from bug 352461.

http://crash-stats.mozilla.com/report/index/6a2e1307-2e54-11dc-92f0-001a4bd43ef6
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

10 years ago
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?



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.
Group: security
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...
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #271620 - Flags: superreview?(jonas)
Attachment #271620 - Flags: review?(jonas)
Created attachment 271622 [details] [diff] [review]
Branch patch
Attachment #271622 - Flags: superreview?(jonas)
Attachment #271622 - Flags: review?(jonas)
Attachment #271622 - Flags: approval1.8.1.6?
Attachment #271622 - Flags: approval1.8.0.13?
Flags: blocking1.8.1.6?
Flags: blocking1.8.0.13?
OS: Windows XP → All
Hardware: PC → All
Summary: Crash [@ nsCellMapColumnIterator::GetNextFrame] with DOMAttrModified and changing rowspan → [FIX]Crash [@ nsCellMapColumnIterator::GetNextFrame] with DOMAttrModified and changing rowspan
Attachment #271622 - Flags: superreview?(jonas)
Attachment #271622 - Flags: superreview+
Attachment #271622 - Flags: review?(jonas)
Attachment #271622 - Flags: review+
Attachment #271620 - Flags: superreview?(jonas)
Attachment #271620 - Flags: superreview+
Attachment #271620 - Flags: review?(jonas)
Attachment #271620 - Flags: review+
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Comment 6

10 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre
Status: RESOLVED → VERIFIED
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?] possibly exploitable
Flags: blocking1.8.0.13? → blocking1.8.0.14?
Attachment #271622 - Flags: approval1.8.0.13? → approval1.8.0.14?
Comment on attachment 271622 [details] [diff] [review]
Branch patch

approved for 1.8.1.7 and 1.8.0.14, a=dveditz for release-drivers
Attachment #271622 - Flags: approval1.8.1.7?
Attachment #271622 - Flags: approval1.8.1.7+
Attachment #271622 - Flags: approval1.8.0.14?
Attachment #271622 - Flags: approval1.8.0.14+
Flags: blocking1.8.1.7? → blocking1.8.1.7+
Fixed on branches.
Keywords: fixed1.8.0.14, fixed1.8.1.7
(Reporter)

Updated

10 years ago
Flags: blocking1.8.0.14?
verified fixed 1.8.1.7 using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.7pre) Gecko/2007083003 BonEcho/2.0.0.7pre and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.7pre) Gecko/20070830 BonEcho/2.0.0.7pre ID:2007083003 and the testcase this bug.

No crash on testcase - adding verified keyword.
Keywords: fixed1.8.1.7 → verified1.8.1.7
Flags: in-testsuite?
Filed bug 394418 on a similar issue.
Group: security
I can't repro the crash with the released Firefox 1.5.0.12 (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.12) Gecko/20070508 Firefox/1.5.0.12).

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?
It doesn't crash in Windows FF 1.5.0.12 either: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.12) Gecko/20070508 Firefox/1.5.0.12.

Comment 13

8 years ago
crash test landed
http://hg.mozilla.org/mozilla-central/rev/9fa246f8f121
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsCellMapColumnIterator::GetNextFrame]
You need to log in before you can comment on or make changes to this bug.