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

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Jesse Ruderman]

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.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Jesse Ruderman]

Attachment: stack trace (mac debug)

Comment 3

[Daniel Veditz [:dveditz]]

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?

Comment 4

[Jesse Ruderman]

WFM Mac trunk debug.  Should the Windows issue become a separate bug report?

Comment 5

[Johnny Stenback (:jst)]

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).

Comment 6

[Johnny Stenback (:jst)]

Attachment: Add null check.

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

Comment 7

[Neil Deakin]

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?

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 9

[Jay Patel [:jay]]

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

Approved for both branches, a=jay for drivers.

Comment 10

[Johnny Stenback (:jst)]

Fix landed on the branches, marking bug FIXED (as this is not an issue on the trunk).

Comment 11

[Marcia Knous [:marcia]]

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.

Comment 12

[Marcia Knous [:marcia]]

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.

Comment 13

[Jesse Ruderman]

Crashtest checked in.