Crash [@ XULPopupListenerImpl::PreLaunchPopup] involving "menupopup.menu = null;"

RESOLVED FIXED

Status

()

Core
XUL
--
critical
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: jst)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
PowerPC
Mac OS X
crash, testcase, verified1.8.0.10, verified1.8.1.2
Points:
---
Bug Flags:
wanted1.8.1.x +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate?], crash signature)

Attachments

(3 attachments)

626 bytes, application/vnd.mozilla.xul+xml
Details
6.16 KB, text/plain
Details
865 bytes, patch
Neil Deakin (not available until Aug 9)
: review+
sicking
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
Loading the testcase crashes Firefox.

Filing as security-sensitive because I don't know what would happen if I set it to something other than null, but controlled by content.
(Reporter)

Comment 1

12 years ago
Created attachment 211557 [details]
testcase
(Reporter)

Comment 2

12 years ago
Created attachment 211558 [details]
stack trace (mac debug)
Completely different stack trace from windows on 2.0.0.2pre debug, cx->fp->pc is 0xCCCCCCCC (uninitialized memory). On an opt build this could potentially grab values from recently freed memory filled by an attacker. Memory would have to be filled with values that were addresses that pointed to something that made sense as JS opcodes, exploitation might be difficult.

>	js_GetProperty() Line 3418	C
 	nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject() Line 243	C++
 	nsXPCWrappedJSClass::DelegatedQueryInterface() Line 595	C++
 	nsXPCWrappedJS::QueryInterface() Line 106	C++
 	DispatchToInterface() Line 141	C++
 	nsEventListenerManager::HandleEvent() Line 1752	C++
 	nsXULDocument::HandleDOMEvent() Line 1234	C++
 	nsXULElement::HandleDOMEvent() Line 2254	C++
 	PresShell::HandleEventInternal() Line 6425	C++
 	PresShell::HandleEvent() Line 6261	C++
 	PresShell::RetargetEventToParent() Line 6037	C++
 	PresShell::HandleEvent() Line 6168	C++
 	nsViewManager::HandleEvent() Line 2512	C++
 	nsViewManager::DispatchEvent() Line 2246	C++
 	HandleEvent() Line 171	C++
 	nsWindow::DispatchEvent() Line 1389	C++
 	nsWindow::DispatchWindowEvent() Line 1409	C++
 	nsWindow::DispatchKeyEvent() Line 3707	C++
 	nsWindow::OnKeyDown() Line 3727	C++
 	nsWindow::ProcessMessage() Line 4883	C++
 	nsWindow::WindowProc() Line 1577	C++
 	77d48744	
 	77d48826	
 	77d489dd	
 	77d48a20	
 	nsAppShell::Run() Line 133	C++
 	nsAppStartup::Run() Line 151	C++
 	XRE_main() Line 2444	C++
 	main() Line 61	C++
 	mainCRTStartup() Line 398	C

Hit a JS assert in 1.5.0.9, but still looking like JS frame corruption of some kind. 
Assertion failure: JS_UPTRDIFF(fp->sp, fp->spbase) <= depthdiff, at c:/dev/ff15/mozilla/js/src/jsinterp.c:386

On trunk this WFM on windows. Mac only crash? Fixed along the way?
Assignee: nobody → jst
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:moderate?]
(Reporter)

Comment 4

11 years ago
WFM Mac trunk debug.  Should the Windows issue become a separate bug report?
(Assignee)

Comment 5

11 years ago
I still see this on Linux, but branches only, not trunk. I've got a trivial fix for the crash I'm seeing (same stack as the mac stack trace in this bug).
Status: NEW → ASSIGNED
(Assignee)

Comment 6

11 years ago
Created attachment 251241 [details] [diff] [review]
Add null check.

In the case I'm seeing the crash, the target is a document, not something that successfully QIs to nsIContent.
(Assignee)

Updated

11 years ago
Attachment #251241 - Flags: superreview?(bugmail)
Attachment #251241 - Flags: review?(enndeakin)
Comment on attachment 251241 [details] [diff] [review]
Add null check.

Is this a branch crash only? The event listener should always be a XUL element, so why is the target of the event a document?
Attachment #251241 - Flags: review?(enndeakin) → review+
Comment on attachment 251241 [details] [diff] [review]
Add null check.

Since this is branch-only it's less important that the fix is the RightThing(tm). But it'd be good to know that things work as they should on trunk.
Attachment #251241 - Flags: superreview?(jonas) → superreview+
(Assignee)

Updated

11 years ago
Attachment #251241 - Flags: approval1.8.1.2?
Attachment #251241 - Flags: approval1.8.0.10?

Comment 9

11 years ago
Comment on attachment 251241 [details] [diff] [review]
Add null check.

Approved for both branches, a=jay for drivers.
Attachment #251241 - Flags: approval1.8.1.2?
Attachment #251241 - Flags: approval1.8.1.2+
Attachment #251241 - Flags: approval1.8.0.10?
Attachment #251241 - Flags: approval1.8.0.10+
(Assignee)

Comment 10

11 years ago
Fix landed on the branches, marking bug FIXED (as this is not an issue on the trunk).
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.0.10, fixed1.8.1.2
Resolution: --- → FIXED
verified on the 1.8 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.2pre) Gecko/20070125 BonEcho/2.0.0.2pre. No crash using Testcase. adding keyword.
Keywords: fixed1.8.1.2 → verified1.8.1.2
verified on the 1.8.0. branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10pre) Gecko/20070125 Firefox/1.5.0.10pre. No crash with testcase, adding keyword.
Keywords: fixed1.8.0.10 → verified1.8.0.10
Group: security
(Reporter)

Comment 13

10 years ago
Crashtest checked in.
Flags: in-testsuite+

Updated

9 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets
Crash Signature: [@ XULPopupListenerImpl::PreLaunchPopup]
You need to log in before you can comment on or make changes to this bug.