Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061205 Minefield/3.0a1 Steps to reproduce: 1. Drag a .zip file from Finder to a Firefox browser window. (This should produce a "What should Minefield do with this file?" dialog.) 2. Close the browser window. 3. Click "cancel" in the dialog. Result: Minefield crashes, badly. Security-sensitive because a clever attacker might be able to substitute step 1 with something that doesn't involve dragging and might be able to get you to do step 2 at just the right time. Stack traces vary a bit, but usually have something bad happening above nsCocoaWindow::Show. Thread 0 Crashed: 0 <<00000000>> 0x07351514 0 + 120919316 1 libxpcom_core.dylib 0x2c00227c nsCOMPtr_base::assign_from_qi(nsQueryInterface, nsID const&) + 40 2 org.mozilla.firefox 0x002bcef0 nsCocoaWindow::Show(int) + 68 3 org.mozilla.firefox 0x00392d70 nsXULWindow::Destroy() + 284 4 org.mozilla.firefox 0x00391b40 nsWebShellWindow::Destroy() + 280 5 org.mozilla.firefox 0x0045b328 nsGlobalWindow::ReallyCloseWindow() + 428 6 org.mozilla.firefox 0x00998f6c MOZ_Z__length_code + 167680 7 libxpcom_core.dylib 0x2c043fc8 nsThread::ProcessNextEvent(int, int*) + 280 8 libxpcom_core.dylib 0x2c00a130 NS_ProcessNextEvent_P(nsIThread*, int) + 76 9 org.mozilla.firefox 0x005bea28 nsBaseAppShell::Run() + 80 10 org.mozilla.firefox 0x002ad6bc non-virtual thunk [nv:-4] to nsAppShell::AfterProcessNextEvent(nsIThreadInternal*, unsigned) + 416 11 com.apple.Foundation 0x9296bbf8 __NSFireDelayedPerform + 304 12 com.apple.CoreFoundation 0x907f0550 __CFRunLoopDoTimer + 184 13 com.apple.CoreFoundation 0x907dcec8 __CFRunLoopRun + 1680 14 com.apple.CoreFoundation 0x907dc47c CFRunLoopRunSpecific + 268 15 com.apple.HIToolbox 0x93208740 RunCurrentEventLoopInMode + 264 16 com.apple.HIToolbox 0x93207dd4 ReceiveNextEventCommon + 380 17 com.apple.HIToolbox 0x93207c40 BlockUntilNextEventMatchingListInMode + 96 18 com.apple.AppKit 0x9370bae4 _DPSNextEvent + 384 19 com.apple.AppKit 0x9370b7a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116 20 org.mozilla.firefox 0x002ad2e8 nsAppShell::ProcessNextNativeEvent(int) + 188 21 org.mozilla.firefox 0x005be9c0 nsBaseAppShell::DoProcessNextNativeEvent(int) + 48 22 org.mozilla.firefox 0x005beb90 nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, int, unsigned) + 100 23 libxpcom_core.dylib 0x2c043f4c nsThread::ProcessNextEvent(int, int*) + 156 24 libxpcom_core.dylib 0x2c00a030 NS_ProcessPendingEvents_P(nsIThread*, unsigned) + 84 25 org.mozilla.firefox 0x005be944 nsBaseAppShell::NativeEventCallback() + 80 26 org.mozilla.firefox 0x002ad0f8 nsAppShell::ProcessGeckoEvents() + 172 27 org.mozilla.firefox 0x002ad66c non-virtual thunk [nv:-4] to nsAppShell::AfterProcessNextEvent(nsIThreadInternal*, unsigned) + 336 28 com.apple.Foundation 0x92959918 __NSFireMachPort + 276 29 com.apple.CoreFoundation 0x907ea820 __CFMachPortPerform + 176 30 com.apple.CoreFoundation 0x907ea734 __CFRunLoopDoSource1 + 152 31 com.apple.CoreFoundation 0x907dce4c __CFRunLoopRun + 1556 32 com.apple.CoreFoundation 0x907dc47c CFRunLoopRunSpecific + 268 33 com.apple.HIToolbox 0x93208740 RunCurrentEventLoopInMode + 264 34 com.apple.HIToolbox 0x93207d4c ReceiveNextEventCommon + 244 35 com.apple.HIToolbox 0x93207c40 BlockUntilNextEventMatchingListInMode + 96 36 com.apple.AppKit 0x9370bae4 _DPSNextEvent + 384 37 com.apple.AppKit 0x9370b7a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116 38 com.apple.AppKit 0x93707cec -[NSApplication run] + 472 39 org.mozilla.firefox 0x002ad3f8 nsAppShell::Run() + 104 40 org.mozilla.firefox 0x003207d8 nsAppStartup::Run() + 88 41 org.mozilla.firefox 0x00012cac XRE_main + 4700 42 org.mozilla.firefox 0x0000dbd4 start + 456 43 dyld 0x8fe01048 _dyld_start + 60

I assume you tried simply loading an application/octet-stream file from somewhere by clicking on a link for step 1 of this bug and couldn't reproduce. That'd be easier for an attacker. As is the current steps move this out of the "normal browsing behavior" category and reduce the potential severity to "moderate" IMO.

I should have tried that earlier. I tried just now and got the same crash.

When any nsIWidget object dies, it should set the parent of its children to null. We are doing this for views under cocoa widgets, but not top-level windows. That's why we have this bug. The fix is to notify children of top-level windows on destruction like we do for views.

Comment on attachment 248229 [details] [diff] [review] fix v1.0 r=mano

*** Bug 361419 has been marked as a duplicate of this bug. ***

dveditz - to make this easier to review, look at the top of the following functions: widget/src/cocoa/nsChildView.mm - nsChildView::~nsChildView() widget/src/mac/nsWindow.cpp - nsWindow::~nsWindow() My patch is exactly the same code (different variable names). Somebody just forgot to put it in the cocoa top-level window nsIWidget implementation.

Comment on attachment 248229 [details] [diff] [review] fix v1.0 r=smorgan

Comment on attachment 248229 [details] [diff] [review] fix v1.0 I am substituting another peer review for sr here because I want this in faster, it is low risk, and this code already exists in other nsIWidget impls.

landed on trunk