978862 - (CVE-2014-1540) heap-use-after-free in nsEventListenerManager::CompileEventHandlerInternal

Status: VERIFIED FIXED

(Core :: DOM: Events, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: BC7FAF8C-242032.html

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

==5540==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0000d75f8 at pc 0x7fcb6095eb42 bp 0x7fffa2779990 sp 0x7fffa2779988
READ of size 8 at 0x60c0000d75f8 thread T0
    #0 0x7fcb6095eb41 (libxul.so!nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct*, nsAString_internal const*, mozilla::dom::Element*)+0x2b91)
	Line 793 of "../../dist/include/nsCOMPtr.h"
    #1 0x7fcb6095f59d (libxul.so!nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEvent*, mozilla::dom::EventTarget*)+0x14d)
	Line 946 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventListenerManager.cpp"
    #2 0x7fcb6096074f (libxul.so!nsEventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*)+0x92f)
	Line 1021 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventListenerManager.cpp"
    #3 0x7fcb60a10ac0 (libxul.so!nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&)+0x810)
	Line 303 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventDispatcher.cpp"
    #4 0x7fcb609e758a (libxul.so!nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*)+0x292a)
	Line 592 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventDispatcher.cpp"
...

Comment 1

[Tyson Smith [:tsmith]]

Attachment: asan_output.txt

Comment 2

[Benjamin Smedberg]

Crash from release nightly (free poisoning enabled): bp-e0e01f5c-53ab-4133-8a22-b28f82140303

Comment 3

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

Attachment: be more careful when to touch aListenerStruct

Unfortunately the patch doesn't show the problematic
WrapNewBindingObject which ends up re-entering listenermanager.

Comment 4

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

Comment on attachment 8384851 [details] [diff] [review]
be more careful when to touch aListenerStruct

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The patch does spot where the issue is, but I'd say it is still a bit hard to figure out that one needs to use
certain element type to trigger the issue.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The patch has effectively kungfuDeathGrip, so yes, there is a bulls-eye.

Which older supported branches are affected by this flaw?
29 should be, although I couldn't reproduce the crash there. The testcase might need some tweak.

If not all supported branches, which bug introduced the flaw?
bug 941876

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The same patch should apply with some --fuzz

How likely is this patch to cause regressions; how much testing does it need?
Should be safe.

Comment 5

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

Comment on attachment 8384851 [details] [diff] [review]
be more careful when to touch aListenerStruct

sec-approval+ for trunk.

Please make an Aurora patch as well and nominate it.

Comment 6

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

Comment on attachment 8384851 [details] [diff] [review]
be more careful when to touch aListenerStruct

I thought I asked approval for Aurora too... oh well.

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

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/15229e2a01b1

Comment 8

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

Backout
https://hg.mozilla.org/integration/mozilla-inbound/rev/b112bbbc923f
And reland with the right bug number.
https://hg.mozilla.org/integration/mozilla-inbound/rev/373012140ff7

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/373012140ff7

Comment 10

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

Confirmed crash on 2014-02-11 Fx30.
Verified fixed on 2014-05-07 Fx30.

Comment 11

[Justin Wood (:Callek)]

Transplanted to SEA_2_26_1_RELBRANCH

https://hg.mozilla.org/releases/mozilla-release/rev/769d705970fc

Comment 12

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

Adding bounty request per Tyson's communication with Christoph.