The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in mozilla1.9alpha8

Status

()

Core
XUL
--
critical
VERIFIED FIXED
10 years ago
6 years ago

People

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

Tracking

(4 keywords)

Trunk
mozilla1.9alpha8
x86
All
crash, testcase, verified1.8.0.14, verified1.8.1.8
Points:
---
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
Neil Deakin
: review+
Details | Diff | Splinter Review
378 bytes, application/vnd.mozilla.xul+xml
Details
(Reporter)

Description

10 years ago
Created attachment 268784 [details]
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.
(Assignee)

Updated

10 years ago
Assignee: nobody → mats.palmgren
OS: Windows XP → All
Whiteboard: infinite stack recusion
(Assignee)

Comment 1

10 years ago
Created attachment 269143 [details]
stack
(Assignee)

Comment 2

10 years ago
Created attachment 269144 [details] [diff] [review]
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.
Attachment #269144 - Flags: review?(neil)

Comment 3

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

Comment 4

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

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

Updated

10 years ago
Attachment #269144 - Flags: review?(neil) → review+
(Assignee)

Updated

10 years ago
Attachment #269144 - Flags: superreview?(roc)
Attachment #269144 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 6

10 years ago
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
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta1
(Assignee)

Updated

10 years ago
Flags: in-testsuite?
This still affects the branch. Requesting blocking.
Flags: blocking1.8.1.6?
Whiteboard: infinite stack recusion → infinite stack recursion
(Reporter)

Comment 8

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
(Assignee)

Comment 9

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

Comment 11

10 years ago
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 
Keywords: fixed1.8.0.14, fixed1.8.1.7
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
Keywords: fixed1.8.1.7 → verified1.8.1.7
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.
Keywords: fixed1.8.0.14 → verified1.8.0.14

Updated

9 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets

Comment 14

8 years ago
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?
(Reporter)

Comment 15

8 years ago
Created attachment 370631 [details]
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

Comment 16

8 years ago
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.