Closed Bug 362952 Opened 18 years ago Closed 18 years ago

Crash [@ nsCocoaWindow::Show] dismissing file-handling dialog after closing browser window

Categories

(Core :: Widget: Cocoa, defect)

PowerPC
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: jaas)

References

Details

(Keywords: crash, Whiteboard: [sg:critical?] post 1.8-branch)

Crash Data

Attachments

(1 file)

909 bytes, patch
asaf
: review+
stuart.morgan+bugzilla
: review+
Details | Diff | Splinter Review
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
Whiteboard: [sg:critical?]
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.
Whiteboard: [sg:critical?] → [sg:moderate?]
I should have tried that earlier.  I tried just now and got the same crash.
Whiteboard: [sg:moderate?] → [sg:critical?]
Attached patch fix v1.0Splinter Review
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.
Attachment #248229 - Flags: review?(mano)
Comment on attachment 248229 [details] [diff] [review]
fix v1.0

r=mano
Attachment #248229 - Flags: review?(mano) → review+
Attachment #248229 - Flags: superreview?(dveditz)
*** 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
Attachment #248229 - Flags: review+
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.
Attachment #248229 - Flags: superreview?(dveditz)
landed on trunk
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?] → [sg:critical?] post 1.8-branch
Group: security
Flags: wanted1.8.1.x-
Crash Signature: [@ nsCocoaWindow::Show]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: