384877 - Crash [@ nsPopupSetFrame::Destroy] when clicking on popup id="d" popup="d"

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

When clicking on the document of the testcase, Mozilla crashes.

https://crash-reports.mozilla.com/reports/report/index/88bd6778-1d8c-11dc-99b8-001a4bd43ef6
0  	nsPopupSetFrame::Destroy()
1 	nsFrameList::DestroyFrames()
2 	nsContainerFrame::Destroy()
3 	nsBoxFrame::RemoveFrame(nsIAtom *,nsIFrame *)
4 	nsRootBoxFrame::RemoveFrame(nsIAtom *,nsIFrame *)
5 	nsFrameManager::RemoveFrame(nsIFrame *,nsIAtom *,nsIFrame *)
6 	nsCSSFrameConstructor::ContentRemoved(nsIContent *,nsIContent *,int,int)
7 	PresShell::ContentRemoved(nsIDocument *,nsIContent *,nsIContent *,int)
8 	nsBindingManager::ContentRemoved(nsIDocument *,nsIContent *,nsIContent *,int)
9 	nsNodeUtils::ContentRemoved(nsINode *,nsIContent *,int)

It also crashes branch builds, so I'm marking it security sensitive for now.
I guess this depends on the fix for bug 279703.

Comment 1

[Mats Palmgren (inactive)]

Attachment: stack

Comment 2

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Return early in LaunchPopup() if the popup id refers to the popup itself.
Tooltips are probably not affected (at least I couldn't find a testcase
that triggered it) but I added a similar check to ShowTooltip() just in case.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I'm a bit worried that there is nsDocElementBoxFrame created for the popup, 
though what should be done in this case...

Comment 4

[[e-mail]]

I think Enn is best placed to comment on this... I'm curious as to whether there's a way to avoid opening an open popup; might there be an exploit caused by having a popup be the context menu for a menuitem in a submenu of itself?

Comment 5

[Neil Deakin]

The patch in bug 279703 already adds a check for the XULPopupListener change, but the tooltip change looks OK too.

Comment 6

[Mats Palmgren (inactive)]

mozilla/content/xul/content/src/nsXULPopupListener.cpp 	1.146 	mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.167 	mozilla/layout/xul/base/src/nsXULTooltipListener.cpp 	1.63 

-> FIXED

Comment 7

[Samuel Sidler (old account; do not CC)]

This still affects the branch. Requesting blocking.

Comment 8

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre

Comment 9

[Mats Palmgren (inactive)]

Comment on attachment 269144 [details] [diff] [review]
Patch rev. 1

Low-risk crash fix.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 269144 [details] [diff] [review]
Patch rev. 1

approved for 1.8.1.7 and 1.8.0.14, a=dveditz for release-drivers

Comment 11

[Mats Palmgren (inactive)]

MOZILLA_1_8_BRANCH
mozilla/content/xul/content/src/nsXULPopupListener.cpp 	1.125.12.6
mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.123.10.11
mozilla/layout/xul/base/src/nsXULTooltipListener.cpp 	1.43.4.4 

MOZILLA_1_8_0_BRANCH
mozilla/content/xul/content/src/nsXULPopupListener.cpp 	1.125.16.4
mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.123.10.2.2.8
mozilla/layout/xul/base/src/nsXULTooltipListener.cpp 	1.43.14.4 

Comment 12

[Carsten Book [:Tomcat]]

verified fixed 1.8.1.7 using the testcase from this bug and:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.7pre) Gecko/2007090308 BonEcho/2.0.0.7pre

and : Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.7pre) Gecko/20070903 BonEcho/2.0.0.7pre ID:2007090304

no crash on testcase - adding verified keyword

Comment 13

[Al Billings [:abillings - ex-MoCo]]

The test case doesn't crash the FF 1.5.0.12 release: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre.

Comment 14

[Bob Clary [:bc] (inactive)]

I tried synthesizing the mouse click to automate the crash but have failed. Is there any trick to dispatch a mouse click to the popup that will cause the crash?

Comment 15

[Martijn Wargers (dead)]

Attachment: testcase

This crashes automatically, without having to click in the document in builds without the patch that fixed this.
In Firefox 3 and beyond there is now openPopup instead of showPopup, that's why the difference in code paths.
I also tried to do a check on the state property of the popup, but that thing doesn't seem to work correctly at all in this case.
https://developer.mozilla.org/en/XUL/menupopup#p-state

Comment 16

[Bob Clary [:bc] (inactive)]

crash test added
http://hg.mozilla.org/mozilla-central/rev/cbef28dd9944