Closed Bug 311710 Opened 19 years ago Closed 19 years ago

Evil xul testcase, using display:-moz-grid-group causes crash [@ nsGridRow::IsCollapsed]

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8rc1

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

Details

(Keywords: fixed1.8, testcase, verified1.7.13, Whiteboard: [sg:critical])

Attachments

(2 files)

See upcoming testcase, which causes a crash when clicking on the button.
Mozilla1.7 is also crashing with the testcase.

Talkback ID: TB10399249G
nsGridRow::IsCollapsed 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRow.cpp,
line 102]
nsGrid::GetPrefRowHeight 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGrid.cpp,
line 905]
nsGridRowGroupLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRowGroupLayout.cpp,
line 128]
nsBoxFrame::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 948]
nsSprocketLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp,
line 1346]
nsGridRowGroupLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRowGroupLayout.cpp,
line 113]
nsBoxFrame::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 948]
nsStackLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsStackLayout.cpp,
line 95]
nsGridLayout2::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridLayout2.cpp,
line 151]
nsBoxFrame::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 948]
nsSprocketLayout::PopulateBoxSizes 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp,
line 822]
nsSprocketLayout::Layout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp,
line 265]
nsBoxFrame::DoLayout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 1106]
nsBoxFrame::DoLayout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 1106]
nsRootBoxFrame::Reflow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsRootBoxFrame.cpp,
line 226]
nsContainerFrame::ReflowChild 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 891]
ViewportFrame::Reflow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsViewportFrame.cpp,
line 239]
IncrementalReflow::Dispatch 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 859]
PresShell::ProcessReflowCommands 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6492]
PresShell::WillPaint 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6136]
SHELL32.dll + 0x520c24 (0x778b0c24)
Attached file testcase
Attached patch Proposed fixSplinter Review
Looks like a logic bug in grid to me.  And the assert change is to just make
them fire when they should (like this case).
Attachment #199020 - Flags: superreview?(roc)
Attachment #199020 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #199020 - Flags: superreview?(roc) → superreview+
Comment on attachment 199020 [details] [diff] [review]
Proposed fix

Quickly plussing this before my eyes glaze over again ;-)
Attachment #199020 - Flags: review?(neil.parkwaycc.co.uk) → review+
Assignee: nobody → bzbarsky
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8rc1
Group: security
Comment on attachment 199020 [details] [diff] [review]
Proposed fix

We should probably take this on branch.  The reason we crash here is an
out-of-bounds access on an array, followed by a method call on what we think is
the object there.  So this is probably exploitable...

The fix is quite safe -- the only affected case is when we go from having more
columns than we have explicitly defined to having exactly the right number, and
without this patch this case is guaranteed to lead us to dereference -1 on an
array....  So the only case affected is the case when we have a problem.
Attachment #199020 - Flags: approval1.8rc1?
Flags: blocking1.8rc1?
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8+
Whiteboard: [sg:critical]
Daniel, should I request 1.7/aviary branch approval?
Attachment #199020 - Flags: approval1.8rc1? → approval1.8rc1+
Verified FIXED using a self-built SeaMonkey trunk as of PDT 5:00pm this evening
on testcase: https://bugzilla.mozilla.org/attachment.cgi?id=198952 with Windows XP.
Status: RESOLVED → VERIFIED
Fixed on branch
Keywords: fixed1.8
Flags: blocking1.8rc1?
Flags: testcase+
Comment on attachment 199020 [details] [diff] [review]
Proposed fix

aviary101/moz17 landing approval: a=dveditz for drivers. Please add the fixed1.7.13 and fixed-aviary1.0.8 keywords when landed.
Attachment #199020 - Flags: approval1.7.13+
Attachment #199020 - Flags: approval-aviary1.0.8+
*** Committing to MOZILLA_1_7_BRANCH... 
new revision: 1.14.50.1; previous revision: 1.14

*** Committing layout/xul/base/src/grid/nsGrid.cpp on AVIARY_1_0_1_20050124_BRANCH
new revision: 1.14.64.1; previous revision: 1.14
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20060209 Firefox/1.0.7
verified fixed on 1.7 branch using Mozilla 1.7.13 Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.13) Gecko/20060309. No crash with testcase.
Group: security
Flags: in-testsuite+ → in-testsuite?
crash test landed
http://hg.mozilla.org/mozilla-central/rev/e7bb6f9cd175
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: