938341 - (CVE-2013-5616) heap-use-after-free in libxul.so!nsEventListenerManager::HandleEventSubType

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Jesse Schwartzentruber (:truber)]

Attachment: 1C072E5F.html

Found by the BlackBerry Security Automated Analysis Team's fuzzing framework ALF.

==24554==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000255a58 at pc 0x7fb832e41778 bp 0x7fff0b8b91f0 sp 0x7fff0b8b91e8
READ of size 8 at 0x60c000255a58 thread T0
    #0 0x7fb832e41777 (libxul.so!nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*)+0x367)
	Line 268 of "../../../dist/include/mozilla/dom/CallbackObject.h"
    #1 0x7fb832e42516 (libxul.so!nsEventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*)+0xa36)
	Line 1038 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventListenerManager.cpp"
    #2 0x7fb832e33303 (libxul.so!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*)+0x4d3)
	Line 325 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventListenerManager.h"
    #3 0x7fb832e325a9 (libxul.so!nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&, nsCxPusher*)+0x829)
	Line 313 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventDispatcher.cpp"
    #4 0x7fb832e36274 (libxul.so!nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*)+0x2974)
	Line 605 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventDispatcher.cpp"
    #5 0x7fb833009c78 (libxul.so!nsGenericHTMLElement::Click()+0x718)
	Line 2738 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/html/content/src/nsGenericHTMLElement.cpp"
    #6 0x7fb83558dd50 (libxul.so!mozilla::dom::HTMLElementBinding::click(JSContext*, JS::Handle<JSObject*>, nsGenericHTMLElement*, JSJitMethodCallArgs const&)+0x30)
	Line 582 of "./HTMLElementBinding.cpp"
    #7 0x7fb83558d5da (libxul.so!mozilla::dom::HTMLElementBinding::genericMethod(JSContext*, unsigned int, JS::Value*)+0x68a)
	Line 4767 of "./HTMLElementBinding.cpp"
    #8 0x7fb837c257c7 (libxul.so!js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)+0x5f7)
	Line 220 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/jscntxtinlines.h"
    #9 0x7fb837c1a70b (libxul.so!Interpret(JSContext*, js::RunState&)+0x18c9b)
	Line 2502 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/vm/Interpreter.cpp"
    #10 0x7fb837c01993 (libxul.so!js::RunScript(JSContext*, js::RunState&)+0x3f3)
	Line 420 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/vm/Interpreter.cpp"
    #11 0x7fb837c25a03 (libxul.so!js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)+0x833)
	Line 482 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/vm/Interpreter.cpp"
    #12 0x7fb837c26778 (libxul.so!js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)+0x568)
	Line 513 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/vm/Interpreter.cpp"
    #13 0x7fb83792bbd7 (libxul.so!JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*)+0x117)
	Line 4920 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/jsapi.cpp"
    #14 0x7fb83537f02f (libxul.so!mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JSObject*>, nsDOMEvent&, mozilla::ErrorResult&)+0x23f)
	Line 36 of "./EventHandlerBinding.cpp"
    #15 0x7fb83365bc53 (libxul.so!nsJSEventListener::HandleEvent(nsIDOMEvent*)+0xab3)
	Line 58 of "../../../dist/include/mozilla/dom/EventHandlerBinding.h"
    #16 0x7fb832e41622 (libxul.so!nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*)+0x212)
	Line 960 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventListenerManager.cpp"
    #17 0x7fb832e42516 (libxul.so!nsEventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*)+0xa36)
	Line 1038 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventListenerManager.cpp"
    #18 0x7fb832e33303 (libxul.so!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*)+0x4d3)
	Line 325 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventListenerManager.h"
    #19 0x7fb832e321de (libxul.so!nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&, nsCxPusher*)+0x45e)
	Line 292 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventDispatcher.cpp"
    #20 0x7fb832e36274 (libxul.so!nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*)+0x2974)
	Line 605 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/events/src/nsEventDispatcher.cpp"
    #21 0x7fb8322c410b (libxul.so!nsDocumentViewer::LoadComplete(tag_nsresult)+0x91b)
	Line 997 of "/builds/slave/m-in-l64-asan-0000000000000000/build/layout/base/nsDocumentViewer.cpp"
    #22 0x7fb83644079a (libxul.so!nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, tag_nsresult)+0x5ca)
	Line 6776 of "/builds/slave/m-in-l64-asan-0000000000000000/build/docshell/base/nsDocShell.cpp"
    #23 0x7fb83643da29 (libxul.so!nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult)+0x1499)
	Line 6573 of "/builds/slave/m-in-l64-asan-0000000000000000/build/docshell/base/nsDocShell.cpp"
    #24 0x7fb83643df6c (libxul.so!non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult)+0xc)
	Line 6579 of "/builds/slave/m-in-l64-asan-0000000000000000/build/docshell/base/nsDocShell.cpp"
    #25 0x7fb836490fbf (libxul.so!nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, tag_nsresult)+0x47f)
	Line 1331 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp"
    #26 0x7fb836490313 (libxul.so!nsDocLoader::doStopDocumentLoad(nsIRequest*, tag_nsresult)+0x263)
	Line 865 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp"
    #27 0x7fb83648dfaf (libxul.so!nsDocLoader::DocLoaderIsEmpty(bool)+0x7ef)
	Line 755 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp"
    #28 0x7fb83648f528 (libxul.so!nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult)+0x5b8)
	Line 639 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp"
    #29 0x7fb83648fdc9 (libxul.so!non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult)+0x9)
	Line 642 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp"
    #30 0x7fb831a22d79 (libxul.so!nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, tag_nsresult)+0x799)
	Line 688 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/base/src/nsLoadGroup.cpp"
    #31 0x7fb832b1c8b6 (libxul.so!nsDocument::DoUnblockOnload()+0x226)
	Line 7988 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/base/src/nsDocument.cpp"
    #32 0x7fb832b1c56b (libxul.so!nsDocument::UnblockOnload(bool)+0x55b)
	Line 7916 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/base/src/nsDocument.cpp"
    #33 0x7fb832afb232 (libxul.so!nsDocument::DispatchContentLoadedEvents()+0xb02)
	Line 4702 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/base/src/nsDocument.cpp"
    #34 0x7fb832b3fa0c (libxul.so!nsRunnableMethodImpl<void (nsDocument::*)(), void, true>::Run()+0x6c)
	Line 382 of "../../../dist/include/nsThreadUtils.h"
    #35 0x7fb835f7cba9 (libxul.so!nsThread::ProcessNextEvent(bool, bool*)+0xaa9)
	Line 622 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/threads/nsThread.cpp"
    #36 0x7fb835ea8cd1 (libxul.so!NS_ProcessNextEvent(nsIThread*, bool)+0xb1)
	Line 251 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsThreadUtils.cpp"
    #37 0x7fb834a162d1 (libxul.so!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)+0x311)
	Line 85 of "/builds/slave/m-in-l64-asan-0000000000000000/build/ipc/glue/MessagePump.cpp"
    #38 0x7fb836084b53 (libxul.so!MessageLoop::Run()+0x1c3)
	Line 220 of "/builds/slave/m-in-l64-asan-0000000000000000/build/ipc/chromium/src/base/message_loop.cc"
    #39 0x7fb8347eea4c (libxul.so!nsBaseAppShell::Run()+0x5c)
	Line 161 of "/builds/slave/m-in-l64-asan-0000000000000000/build/widget/xpwidgets/nsBaseAppShell.cpp"
    #40 0x7fb8341f5a0e (libxul.so!nsAppStartup::Run()+0xbe)
	Line 267 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/components/startup/nsAppStartup.cpp"
    #41 0x7fb83174e655 (libxul.so!XREMain::XRE_mainRun()+0x1e05)
	Line 3976 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #42 0x7fb83174f58a (libxul.so!XREMain::XRE_main(int, char**, nsXREAppData const*)+0x4fa)
	Line 4044 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #43 0x7fb8317504bb (libxul.so!XRE_main+0x3ab)
	Line 4246 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #44 0x459dcd (firefox!main+0x94d)
	Line 275 of "/builds/slave/m-in-l64-asan-0000000000000000/build/browser/app/nsBrowserApp.cpp"
    #45 0x7fb840e9776c (libc.so.6!__libc_start_main+0xec)
	Line 226 of "libc-start.c"
    #46 0x45934c (firefox!_start+0x28)
0x60c000255a58 is located 88 bytes inside of 128-byte region [0x60c000255a00,0x60c000255a80)
freed by thread T0 here:
    #0 0x44653b (firefox!realloc+0x5b)
	Line 95 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc"
    #1 0x7fb83e04e65e (libmozalloc.so!moz_xrealloc+0xe)
	Line 84 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp"
previously allocated by thread T0 here:
    #0 0x446395 (firefox!malloc+0x55)
	Line 74 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc"
    #1 0x7fb83e04e5c8 (libmozalloc.so!moz_xmalloc+0x8)
	Line 52 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp"
Shadow bytes around the buggy address:
  0x0c1880042af0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1880042b00: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
  0x0c1880042b10: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1880042b20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1880042b30: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
=>0x0c1880042b40: fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd
  0x0c1880042b50: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1880042b60: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c1880042b70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1880042b80: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1880042b90: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:     fa
  Heap right redzone:    fb
  Freed heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==24554==ABORTING

Comment 1

[Benjamin Smedberg]

<applet contenteditable="true">. I can reproduce a crash with Java in click-to-play mode. Jesse, do you know if Java was installed/enabled on the machine which experienced this error?

Nightly crash report: https://crash-stats.mozilla.com/report/index/85a797dc-99dd-49aa-a455-c33622131113

Comment 2

[Mats Palmgren (inactive)]

Also crashes a non-Asan debug build on Linux64 (without Java installed).

Comment 3

[Jesse Schwartzentruber (:truber)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Jesse, do you know if Java was installed/enabled on the
> machine which experienced this error?
> 
Java was not installed. The machine was Ubuntu server 12.04.3 with minimum deps for Xvfb and Firefox

Comment 4

[Mats Palmgren (inactive)]

Attachment: stack

We're removing event listeners from the mListeners array of the ELM in
frame #30, deallocating memory for the items. Not a problem per se, since
we anticipated this and made a 'kungFuDeathGrip' for the one we're using.
The problem is the second param to HandleEventSubType is *by reference* and
we pass 'ls->mListener' so this is a pointer into the memory we're currently
deallocating ('ls').
http://hg.mozilla.org/mozilla-central/annotate/7b014f0f3b03/content/events/src/nsEventListenerManager.cpp#l1039

We should just pass in 'kungFuDeathGrip' instead.  Note that 'ls' suffers
the same problem but it's only used before calling HandleEvent.  We should
null it out and make a comment about it though to avoid future mistakes.
http://hg.mozilla.org/mozilla-central/annotate/7b014f0f3b03/content/events/src/nsEventListenerManager.cpp#l932

Comment 5

[Mats Palmgren (inactive)]

Attachment: stack #2

This is a better stack showing when the actual corruption occurs.

Comment 6

[Mats Palmgren (inactive)]

Attachment: fix

Comment 7

[Mats Palmgren (inactive)]

Attachment: crashtest patch (DO NOT LAND BEFORE BUG IS PUBLIC)

Comment 8

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

Comment on attachment 832499 [details] [diff] [review]
fix

>-          EventListenerHolder kungFuDeathGrip(ls->mListener);
>-          if (NS_FAILED(HandleEventSubType(ls, ls->mListener, *aDOMEvent,
>-                                           aCurrentTarget, aPusher))) {
>+          // NOTE: HandleEventSubType may run script and destroy the world,
>+          // e.g. 'ls' or anything else in mListeners.
>+          if (NS_FAILED(HandleEventSubType(ls, *aDOMEvent, aCurrentTarget,
>+                                           aPusher))) {
I wouldn't add the comment


>+++ b/content/events/src/nsEventListenerManager.h
>@@ -411,18 +411,20 @@ public:
> protected:
>   void HandleEventInternal(nsPresContext* aPresContext,
>                            mozilla::WidgetEvent* aEvent,
>                            nsIDOMEvent** aDOMEvent,
>                            mozilla::dom::EventTarget* aCurrentTarget,
>                            nsEventStatus* aEventStatus,
>                            nsCxPusher* aPusher);
> 
>+  /**
>+   * @note calling this might run script that destroys the world
>+   */
>   nsresult HandleEventSubType(nsListenerStruct* aListenerStruct,

this is rather obvious, since event listener is usually a script. So perhaps no worth to add the comment.

Comment 9

[Mats Palmgren (inactive)]

Attachment: fix (without code comments)

(In reply to Olli Pettay [:smaug] from comment #8)
> I wouldn't add the comment

OK, I'm splitting out the code comments into a separate patch for later landing
when the bug is public.

> this is rather obvious, since event listener is usually a script. So perhaps
> no worth to add the comment.

OK, removed this comment altogether.

Comment 10

[Mats Palmgren (inactive)]

Attachment: code comments (DO NOT LAND BEFORE BUG IS PUBLIC)

Comment 11

[Mats Palmgren (inactive)]

It seems b2g18 might also be affected; we're passing 'ls->mListener' by reference here:
https://hg.mozilla.org/releases/mozilla-b2g18/file/3d28e6cbacce/content/events/src/nsEventListenerManager.cpp#l961
and then using it, after CompileEventHandlerInternal, on line 889:
https://hg.mozilla.org/releases/mozilla-b2g18/file/3d28e6cbacce/content/events/src/nsEventListenerManager.cpp#l866

Comment 12

[Mats Palmgren (inactive)]

Nope, sorry, it's not by reference but the actual pointer value, so it should be
independent of 'ls'.

Comment 13

[Mats Palmgren (inactive)]

Comment on attachment 832554 [details] [diff] [review]
fix (without code comments)

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Seems hard to me.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No comments, no tests.

Which older supported branches are affected by this flaw?
All except b2g18*

If not all supported branches, which bug introduced the flaw?
Don't know, I'll check and add a dependency.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I expect the same patch will apply.

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely to cause regressions.  No special testing needed.

Comment 14

[Mats Palmgren (inactive)]

Looking at "hg blame" I'm guessing this cset is the culprit (landed in mozilla23):
http://hg.mozilla.org/mozilla-central/rev/55cad36868d8

Comment 15

[Boris Zbarsky [:bzbarsky]]

Yes, definitely.  Sorry about that.  :(

Comment 16

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

Comment on attachment 832554 [details] [diff] [review]
fix (without code comments)

sec-approval+ for trunk.

Please either prepare appropriate patches or just nominate this for Aurora, Beta, and ESR24.

Comment 17

[Mats Palmgren (inactive)]

Comment on attachment 832554 [details] [diff] [review]
fix (without code comments)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 835643
User impact if declined: sec-critical crash
Testing completed (on m-c, etc.): not landed yet
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none

Comment 18

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/44fe639256f8

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/44fe639256f8

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0395ab4e357b
https://hg.mozilla.org/releases/mozilla-beta/rev/55acf28131b7

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/55acf28131b7

Al, can you approve this for esr24?

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr24/rev/0d25979d3001

Comment 24

[Mats Palmgren (inactive)]

Restoring flags. (hmm, why doesn't history say anything about how they got to "affected" again?)

Comment 25

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

Confirmed crash on FF28 2013-11-13.
Verified fixed on ASan builds of FF24esr, FF26, FF27 and FF28, 2013-11-20.

Comment 27

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb01c6ccd716

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/eb01c6ccd716