Last Comment Bug 326864 - Crash [@ XULPopupListenerImpl::PreLaunchPopup] involving "menupopup.menu = null;"
: Crash [@ XULPopupListenerImpl::PreLaunchPopup] involving "menupopup.menu = nu...
Status: RESOLVED FIXED
[sg:moderate?]
: crash, testcase, verified1.8.0.10, verified1.8.1.2
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- critical (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
: Neil Deakin
Mentors:
Depends on:
Blocks: 326633
  Show dependency treegraph
 
Reported: 2006-02-11 21:30 PST by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
4 users (show)
dveditz: wanted1.8.1.x+
dveditz: wanted1.8.0.x+
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (626 bytes, application/vnd.mozilla.xul+xml)
2006-02-11 21:30 PST, Jesse Ruderman
no flags Details
stack trace (mac debug) (6.16 KB, text/plain)
2006-02-11 21:32 PST, Jesse Ruderman
no flags Details
Add null check. (865 bytes, patch)
2007-01-11 17:50 PST, Johnny Stenback (:jst, jst@mozilla.com)
enndeakin: review+
jonas: superreview+
jaymoz: approval1.8.1.2+
jaymoz: approval1.8.0.10+
Details | Diff | Splinter Review

Description Jesse Ruderman 2006-02-11 21:30:09 PST
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 2006-02-11 21:30:40 PST
Created attachment 211557 [details]
testcase
Comment 2 Jesse Ruderman 2006-02-11 21:32:28 PST
Created attachment 211558 [details]
stack trace (mac debug)
Comment 3 Daniel Veditz [:dveditz] 2007-01-09 17:11:54 PST
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 2007-01-09 23:00:09 PST
WFM Mac trunk debug.  Should the Windows issue become a separate bug report?
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-11 17:48:01 PST
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, jst@mozilla.com) 2007-01-11 17:50:41 PST
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.
Comment 7 Neil Deakin 2007-01-12 06:30:18 PST
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 2007-01-22 16:02:42 PST
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] 2007-01-23 14:26:33 PST
Comment on attachment 251241 [details] [diff] [review]
Add null check.

Approved for both branches, a=jay for drivers.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-23 16:10:17 PST
Fix landed on the branches, marking bug FIXED (as this is not an issue on the trunk).
Comment 11 Marcia Knous [:marcia - use ni] 2007-01-25 11:05:54 PST
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 - use ni] 2007-01-25 11:09:26 PST
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 2007-12-18 16:18:32 PST
Crashtest checked in.

Note You need to log in before you can comment on or make changes to this bug.