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

VERIFIED FIXED in mozilla1.9alpha8

Status

()

defect
--
critical
VERIFIED FIXED
12 years ago
8 years ago

People

(Reporter: martijn.martijn, Assigned: mats)

Tracking

(4 keywords)

Trunk
mozilla1.9alpha8
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.8 +
wanted1.8.1.x +
blocking1.8.0.14 +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse dos] infinite stack recursion, crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

9.77 KB, text/html
Details
4.26 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review
378 bytes, application/vnd.mozilla.xul+xml
Details
Posted file testcase (obsolete) —
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.
Assignee: nobody → mats.palmgren
OS: Windows XP → All
Whiteboard: infinite stack recusion
Posted file stack
Posted patch Patch rev. 1Splinter Review
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.
Attachment #269144 - Flags: review?(neil)
I'm a bit worried that there is nsDocElementBoxFrame created for the popup, 
though what should be done in this case...
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?
The patch in bug 279703 already adds a check for the XULPopupListener change, but the tooltip change looks OK too.
Attachment #269144 - Flags: review?(neil) → review+
Attachment #269144 - Flags: superreview?(roc)
Attachment #269144 - Flags: superreview?(roc) → superreview+
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
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta1
Flags: in-testsuite?
This still affects the branch. Requesting blocking.
Flags: blocking1.8.1.6?
Whiteboard: infinite stack recusion → infinite stack recursion
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
Comment on attachment 269144 [details] [diff] [review]
Patch rev. 1

Low-risk crash fix.
Attachment #269144 - Flags: approval1.8.1.6?
Attachment #269144 - Flags: approval1.8.0.13?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: infinite stack recursion → [sg:nse dos] infinite stack recursion
Attachment #269144 - Flags: approval1.8.0.13? → approval1.8.0.14?
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
Attachment #269144 - Flags: approval1.8.1.7?
Attachment #269144 - Flags: approval1.8.1.7+
Attachment #269144 - Flags: approval1.8.0.14?
Attachment #269144 - Flags: approval1.8.0.14+
Flags: blocking1.8.1.7?
Flags: blocking1.8.1.7+
Flags: blocking1.8.0.14?
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 
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
Group: security
Flags: blocking1.8.0.14? → blocking1.8.0.14+
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.
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets
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?
Posted file 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
Attachment #268784 - Attachment is obsolete: true
crash test added
http://hg.mozilla.org/mozilla-central/rev/cbef28dd9944
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsPopupSetFrame::Destroy]
You need to log in before you can comment on or make changes to this bug.