899022 - crash in nsCOMPtr_base::assign_with_AddRef | nsWindowRoot::PreHandleEvent

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Scoobidiver (away)]

It's #10 browser crasher on Mac OS X in 22.0.

Signature 	nsCOMPtr_base::assign_with_AddRef(nsISupports*) | nsWindowRoot::PreHandleEvent(nsEventChainPreVisitor&) More Reports Search
UUID 	b7fb3d22-1570-4eb8-8c15-e7d7a2130729
Date Processed	2013-07-29 04:07:12.795335
Uptime	35480
Install Age 	94426 since version was first installed.
Install Time 	2013-07-28 01:53:08
Product 	Firefox
Version 	23.0
Build ID 	20130725195523
Release Channel 	beta
OS 	Mac OS X
OS Version 	10.8.4 12E55
Build Architecture 	amd64
Build Architecture Info 	family 6 model 23 stepping 10 | 2
Crash Reason 	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address 	0x8
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x 863GL Layers? GL Context? GL Context+ GL Layers+ 

Frame 	Module 	Signature 	Source
0 	XUL 	nsCOMPtr_base::assign_with_AddRef(nsISupports*) 	obj-firefox/x86_64/xpcom/build/nsCOMPtr.cpp
1 	XUL 	nsWindowRoot::PreHandleEvent(nsEventChainPreVisitor&) 	obj-firefox/x86_64/dist/include/nsCOMPtr.h
2 	XUL 	nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) 	content/events/src/nsEventDispatcher.cpp
3 	XUL 	nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) 	content/events/src/nsEventDispatcher.cpp
4 	XUL 	nsINode::DispatchEvent(nsIDOMEvent*, bool*) 	content/base/src/nsINode.cpp
5 	XUL 	nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) 	content/base/src/nsContentUtils.cpp
6 	XUL 	nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) 	content/base/src/nsContentUtils.cpp
7 	XUL 	nsGlobalWindow::FireOfflineStatusEvent() 	dom/base/nsGlobalWindow.cpp
8 	XUL 	nsGlobalWindow::Observe(nsISupports*, char const*, unsigned short const*) 	dom/base/nsGlobalWindow.cpp
9 	XUL 	nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) 	xpcom/ds/nsObserverList.cpp
10 	XUL 	nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) 	xpcom/ds/nsObserverService.cpp
11 	XUL 	nsIOService::SetOffline(bool) 	netwerk/base/src/nsIOService.cpp
12 	XUL 	nsIOService::Observe(nsISupports*, char const*, unsigned short const*) 	netwerk/base/src/nsIOService.cpp
13 	XUL 	_ZThn8_N11nsIOService7ObserveEP11nsISupportsPKcPKt 	netwerk/base/src/nsIOService.cpp
14 	XUL 	nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) 	xpcom/ds/nsObserverList.cpp
15 	XUL 	nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) 	xpcom/ds/nsObserverService.cpp
16 	XUL 	nsXREDirProvider::DoShutdown() 	toolkit/xre/nsXREDirProvider.cpp
17 	XUL 	ScopedXPCOMStartup::~ScopedXPCOMStartup() 	toolkit/xre/nsAppRunner.cpp
18 	XUL 	XREMain::XRE_main(int, char**, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
19 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp
20 	firefox 	main 	browser/app/nsBrowserApp.cpp
21 	firefox 	start 	

More reports at:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=nsCOMPtr_base%3A%3Aassign_with_AddRef%28nsISupports*%29+|+nsWindowRoot%3A%3APreHandleEvent%28nsEventChainPreVisitor%26%29
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=%400x0+|+nsCOMPtr_base%3A%3Aassign_with_AddRef%28nsISupports*%29+|+nsWindowRoot%3A%3APreHandleEvent%28nsEventChainPreVisitor%26%29

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 786050 [details] [diff] [review]
patch

Review of attachment 786050 [details] [diff] [review]:
-----------------------------------------------------------------

:(

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: esr uses different CC macros here

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 786050 [details] [diff] [review]
patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
It is easy to see what the problem is but I don't know how to reproduce the crash

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Check-in comment will be about cycle collecting the window.

Which older supported branches are affected by this flaw?
All since bug 54203 (year 2000)

Do you have backports for the affected branches? Yes

How likely is this patch to cause regressions; how much testing does it need?
It makes one reference strong, so would be good to get some testing in order to make sure
there are no memory leaks.
(even if the strong reference is added to the cycle collection)

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 786050 [details] [diff] [review]
patch

Sec-approval+ for trunk.

Once this is in, please also check it into Aurora and Beta. Tiny patch!

I can't approve for b2g or ESR on my own without release management though.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b67f2733685

Comment 7

[Al Billings [:abillings - ex-MoCo]]

Should we take this on ESR and B2G?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Probably.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6b67f2733685

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/69d2e4db243b
https://hg.mozilla.org/releases/mozilla-beta/rev/80c22c20bde5

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 786050 [details] [diff] [review]
patch

This wouldn't drive a 23.0.2 so minusing for m-r branch, it will be in 24 and ESR 24.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr17/rev/59b913e20d8c

Comment 13

[Matt Wobensmith [:mwobensmith][:matt:]]

In the absence of a test case, comment 4 says:

"It makes one reference strong, so would be good to get some testing in order to make sure there are no memory leaks."

If there's anything you can suggest for QA to do - to verify that this is a good fix - please let us know. At the moment, it doesn't appear that there's anything we can do for verification.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

With "some testing" I meant enough mochitest runs and me running about:cc and checking whether
there are leaks. Haven't seen anything.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

b2g18 is frozen for non-leo+ blockers. Please request blocking status if you feel that this must land there.

Comment 16

[Alex Keybl [:akeybl]]

Comment on attachment 786050 [details] [diff] [review]
patch

[Triage Comment]

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #15)
> b2g18 is frozen for non-leo+ blockers. Please request blocking status if you
> feel that this must land there.

I'm sorry, I fucked this up. I fed RyanVM bad information. The v1.1 branch will be open until 3/3/2014, as it's being maintained for security until that time.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/025638d3f504

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/025638d3f504