Closed Bug 1544670 (CVE-2019-11692) Opened 6 years ago Closed 6 years ago

heap-use-after-free in mozilla::dom::WakeLock::Release

Categories

(Core :: DOM: Core & HTML, defect, P1)

Product:
Core
Component:
DOM: Core & HTML
Version:
68 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox-esr60 67+ fixed
firefox66 --- wontfix
firefox67 + fixed
firefox68 + fixed

People

(Reporter: nils, Assigned: smaug)

Details

(4 keywords, Whiteboard: [adv-main67+][adv-esr60.7+])

Attachments

(1 file, 1 obsolete file)

Bug 1544670, don't let one to reuse unlinked CallbackObjectHolder, r=mccr8
47 bytes, text/x-phabricator-request
abillings
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr60+
abillings
: sec-approval+
Details | Review

I have seen the following crash a few times while fuzzing Firefo 68.0a1, however currently am unable to reproduce reliably. I will continue to try to minimize a testcase.

=================================================================
==6682==ERROR: AddressSanitizer: heap-use-after-free on address 0x608000018378 at pc 0x7fb01ea64c44 bp 0x7fffce9e4e60 sp 0x7fffce9e4e58
READ of size 8 at 0x608000018378 thread T0 (Web Content)
#0 0x7fb01ea64c43 in ~nsCOMPtr_base /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:330:9
#1 0x7fb01ea64c43 in mozilla::dom::WakeLock::~WakeLock() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:42
#2 0x7fb01ea656a8 in mozilla::dom::WakeLock::~WakeLock() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:39:23
#3 0x7fb01ea643fc in mozilla::dom::WakeLock::Release() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:32:1
#4 0x7fb01d6bb736 in UnlinkSelf /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:528:5
#5 0x7fb01d6bb736 in ~CallbackObjectHolder /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:406
#6 0x7fb01d6bb736 in ~Listener /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventListenerManager.h:233
#7 0x7fb01d6bb736 in Destruct /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:525
#8 0x7fb01d6bb736 in DestructRange /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2183
#9 0x7fb01d6bb736 in nsTArray_Impl<mozilla::EventListenerManager::Listener, nsTArrayInfallibleAllocator>::RemoveElementsAtUnsafe(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2242
#10 0x7fb01d65d24e in RemoveElementsAt /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2236:3
#11 0x7fb01d65d24e in RemoveElementAt /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1698
#12 0x7fb01d65d24e in RemoveElementAt /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:208
#13 0x7fb01d65d24e in mozilla::EventListenerManager::RemoveAllListeners() /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1717
#14 0x7fb01db9c059 in nsHTMLDocument::Open(mozilla::dom::Optional<nsTSubstring<char16_t> > const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1269:12
#15 0x7fb01db9eef3 in nsHTMLDocument::WriteCommon(nsTSubstring<char16_t> const&, bool, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1530:5
#16 0x7fb01db9e11d in nsHTMLDocument::WriteCommon(mozilla::dom::Sequence<nsTString<char16_t> > const&, bool, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1456:5
#17 0x7fb01c8205e1 in mozilla::dom::HTMLDocument_Binding::write(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:397:9
#18 0x7fb01cdd3ac1 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3153:13
#19 0x7fb0245f5bf7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
#20 0x7fb0245f5bf7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
#21 0x7fb0245de3ef in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
#22 0x7fb0245de3ef in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3072
#23 0x7fb0245c0368 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
#24 0x7fb0245fb449 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:781:13
#25 0x7fb0246f1a37 in EvalKernel(JSContext*, JS::Handle<JS::Value>, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:326:10
#26 0x7fb0246f3e04 in js::DirectEval(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:440:10
#27 0x7fb0257f51e4 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jit/BaselineIC.cpp:3866:10
#28 0x287384819887 (<unknown module>)

0x608000018378 is located 88 bytes inside of 96-byte region [0x608000018320,0x608000018380)
freed by thread T0 (Web Content) here:
#0 0x55acbf8359e2 in free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
#1 0x7fb01ea643fc in mozilla::dom::WakeLock::Release() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:32:1
#2 0x7fb01d64c7d7 in UnlinkSelf /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:528:5
#3 0x7fb01d64c7d7 in ~CallbackObjectHolder /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:406
#4 0x7fb01d64c7d7 in mozilla::EventListenerManager::RemoveEventListenerByType(mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener>, nsTSubstring<char16_t> const&, mozilla::EventListenerFlags const&) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:696
#5 0x7fb01d666eaf in RemoveEventListenerByType /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventListenerManager.h:300:5
#6 0x7fb01d666eaf in mozilla::dom::EventTarget::RemoveSystemEventListener(nsTSubstring<char16_t> const&, nsIDOMEventListener*, bool) /builds/worker/workspace/build/src/dom/events/EventTarget.cpp:133
#7 0x7fb01ea650af in mozilla::dom::WakeLock::DetachEventListener() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:174:12
#8 0x7fb01ea64b2b in mozilla::dom::WakeLock::~WakeLock() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:41:3
#9 0x7fb01ea656a8 in mozilla::dom::WakeLock::~WakeLock() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:39:23
#10 0x7fb01ea643fc in mozilla::dom::WakeLock::Release() /builds/worker/workspace/build/src/dom/power/WakeLock.cpp:32:1
#11 0x7fb01d6bb736 in UnlinkSelf /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:528:5
#12 0x7fb01d6bb736 in ~CallbackObjectHolder /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:406
#13 0x7fb01d6bb736 in ~Listener /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventListenerManager.h:233
#14 0x7fb01d6bb736 in Destruct /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:525
#15 0x7fb01d6bb736 in DestructRange /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2183
#16 0x7fb01d6bb736 in nsTArray_Impl<mozilla::EventListenerManager::Listener, nsTArrayInfallibleAllocator>::RemoveElementsAtUnsafe(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2242
#17 0x7fb01d65d24e in RemoveElementsAt /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2236:3
#18 0x7fb01d65d24e in RemoveElementAt /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1698
#19 0x7fb01d65d24e in RemoveElementAt /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:208
#20 0x7fb01d65d24e in mozilla::EventListenerManager::RemoveAllListeners() /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1717
#21 0x7fb01db9c059 in nsHTMLDocument::Open(mozilla::dom::Optional<nsTSubstring<char16_t> > const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1269:12
#22 0x7fb01db9eef3 in nsHTMLDocument::WriteCommon(nsTSubstring<char16_t> const&, bool, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1530:5
#23 0x7fb01db9e11d in nsHTMLDocument::WriteCommon(mozilla::dom::Sequence<nsTString<char16_t> > const&, bool, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1456:5
#24 0x7fb01c8205e1 in mozilla::dom::HTMLDocument_Binding::write(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:397:9
#25 0x7fb01cdd3ac1 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3153:13
#26 0x7fb0245f5bf7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
#27 0x7fb0245f5bf7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
#28 0x7fb0245de3ef in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
#29 0x7fb0245de3ef in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3072
#30 0x7fb0245c0368 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
#31 0x7fb0245fb449 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:781:13
#32 0x7fb0246f1a37 in EvalKernel(JSContext*, JS::Handle<JS::Value>, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:326:10
#33 0x7fb0246f3e04 in js::DirectEval(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:440:10
#34 0x7fb0257f51e4 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jit/BaselineIC.cpp:3866:10
#35 0x287384819887 (<unknown module>)
#36 0x621000538877 (<unknown module>)
#37 0x28738483e9d0 (<unknown module>)
#38 0x62100051b25f (<unknown module>)
#39 0x2873848174de (<unknown module>)
#40 0x7fb025d7d6ca in EnterJit /builds/worker/workspace/build/src/js/src/jit/Jit.cpp:103:5
#41 0x7fb025d7d6ca in js::jit::MaybeEnterJit(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/jit/Jit.cpp:168
#42 0x7fb0245c025e in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:407:32
#43 0x7fb0245f6568 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:562:13

previously allocated by thread T0 (Web Content) here:
#0 0x55acbf835d63 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
#1 0x55acbf86a5fd in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:68:15
#2 0x7fb01ea627bc in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
#3 0x7fb01ea627bc in mozilla::dom::power::PowerManagerService::NewWakeLock(nsTSubstring<char16_t> const&, nsPIDOMWindowInner*, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/power/PowerManagerService.cpp:118
#4 0x7fb01da34608 in mozilla::dom::HTMLMediaElement::CreateAudioWakeLockIfNeeded() /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:3885:28
#5 0x7fb01da341ae in mozilla::dom::HTMLMediaElement::UpdateWakeLock() /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:3872:5
#6 0x7fb01db1812c in mozilla::dom::HTMLVideoElement::UpdateWakeLock() /builds/worker/workspace/build/src/dom/html/HTMLVideoElement.cpp:350:21
#7 0x7fb01da32b94 in mozilla::dom::HTMLMediaElement::PlayInternal(bool) /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:3793:3
#8 0x7fb01da31b14 in mozilla::dom::HTMLMediaElement::Play(mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/html/HTMLMediaElement.cpp:3721:5
#9 0x7fb01ca14bdd in play /builds/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLMediaElementBinding.cpp:1236:45
#10 0x7fb01ca14bdd in mozilla::dom::HTMLMediaElement_Binding::play_promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLMediaElement*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLMediaElementBinding.cpp:1250
#11 0x7fb01cdd4aad in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3153:13
#12 0x7fb0245f5bf7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
#13 0x7fb0245f5bf7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
#14 0x7fb0245f81b2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:605:8
#15 0x7fb0253433bf in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:162:10
#16 0x7fb0252fc601 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:237:19
#17 0x7fb025322d00 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:504:19
#18 0x7fb0245f6c4b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:508:14
#19 0x7fb0245de3ef in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
#20 0x7fb0245de3ef in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3072
#21 0x7fb0245c0368 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
#22 0x7fb0245fb449 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:781:13
#23 0x7fb0246f1a37 in EvalKernel(JSContext*, JS::Handle<JS::Value>, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:326:10
#24 0x7fb0246f0ffb in js::IndirectEval(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:424:10
#25 0x7fb0245f5bf7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
#26 0x7fb0245f5bf7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
#27 0x7fb0245f81b2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:605:8
#28 0x7fb0253433bf in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:162:10
#29 0x7fb0252fc601 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:237:19
#30 0x7fb025322d00 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:504:19
#31 0x7fb0245f6c4b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:508:14
#32 0x7fb0245de3ef in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
#33 0x7fb0245de3ef in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3072
#34 0x7fb0245c0368 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
#35 0x7fb0245fb449 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:781:13

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:330:9 in ~nsCOMPtr_base
Shadow bytes around the buggy address:
0x0c107fffb010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c107fffb060: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd[fd]
0x0c107fffb070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb090: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb0a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c107fffb0b0: fa fa fa fa fa fa fa fa 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
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==6682==ABORTING

Group: core-security → dom-core-security
Keywords: csectype-uaf

Without a testcase this requires a bit more code auditing.

Priority: -- → P1

The use and free stacks are almost the same, starting at mozilla::dom::WakeLock::Release() (the second one in the case of the free stack)

Maybe the use stack is calling EventListenerManager::RemoveAllListeners() on the same ELM and listener that is having EventListenerManager::RemoveEventListenerByType() called on it in the free stack? It is an observer array, so it is supposed to be able to deal with at least some reentrance, but it looks like nsTArray_Impl::RemoveElementsAtUnsafe() calls destroy on the elements before it removes them from the array, so I think you could end up calling the dtor on an object twice.

This was added by the document.open changes in bug 1489308.

status-firefox67: --- → affected
tracking-firefox67: --- → ?
tracking-firefox68: --- → ?
Keywords: regression, sec-high
Regressed by: 1489308
Assignee: nobody → bugs

There are other places in ELM that remove things from mListeners, which feel like they could also be affected, so maybe it isn't actually a regression.

The issue is when we explicitly remove all the listeners. That changed recently.
(there can be other similar cases. looking)

Actually, UnlinkSelf is odd. It calls Release and only then clears mPtrBits.
Changing that should be enough to fix this, since RemoveEventListenerByType would become no-op.

Comment on attachment 9058652 [details]
Bug 1544670, don't let one to reuse unlinked CallbackObjectHolder, r=mccr8

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: None
  • User impact if declined:
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
  • String changes made/needed:

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: unclear, but I think the patch does kind of pinpoint where the issue is.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which older supported branches are affected by this flaw?: all
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: Based on https://searchfox.org/mozilla-central/rev/d33d470140ce3f9426af523eaa8ecfa83476c806/dom/bindings/CallbackObject.h#525 the code has been the same since 2013
  • How likely is this patch to cause regressions; how much testing does it need?: I would be surprised to see regressions. Just clearing a member variable a bit earlier.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined:
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
  • String or UUID changes made by this patch:
Attachment #9058652 - Flags: sec-approval?
Attachment #9058652 - Flags: approval-mozilla-esr60?
Attachment #9058652 - Flags: approval-mozilla-beta?

If esr60 is affected I guess that means 1489308 isn't actually the cause for this?

status-firefox66: unaffected → wontfix
status-firefox-esr60: --- → affected
tracking-firefox67: ? → +
tracking-firefox68: ? → +
tracking-firefox-esr60: --- → 67+
No longer regressed by: 1489308

Bug 1489308 added a way this could be reached, but there might have been other ways it could be reached too.

That said, I don't understand why ~WakeLock calls WakeLock::DetachEventListener(). If it's still an event listener on some event target, that will hold a ref to it and hence ~WakeLock won't get reached, right? Can we just remove that call (in a separate bug)?

Flags: needinfo?(bugs)

WakeLock is the event listener, and when it's refcnt drops to 0 because ELM's listener are removed, its dtor is called (synchronously since it isn't CCable object), and then dtor tries to re-remove the listener.

Flags: needinfo?(bugs)

Right, but why is the dtor trying to remove anything? If it's registered as a listener, the ELM will hold a ref and its refcnt can't drop to 0. So if the dtor is running, there's no need to do any removals, unless I'm missing something.

Flags: needinfo?(bugs)

oh, sure, dtor doesn't need to remove anything. But if one tries to do that, it shouldn't crash.

Flags: needinfo?(bugs)

Comment on attachment 9058652 [details]
Bug 1544670, don't let one to reuse unlinked CallbackObjectHolder, r=mccr8

sec-approval+ for mozilla-central and beta approval given.

Attachment #9058652 - Flags: sec-approval?
Attachment #9058652 - Flags: sec-approval+
Attachment #9058652 - Flags: approval-mozilla-beta?
Attachment #9058652 - Flags: approval-mozilla-beta+

Comment on attachment 9058652 [details]
Bug 1544670, don't let one to reuse unlinked CallbackObjectHolder, r=mccr8

Approved for 60.7.0esr as well.

Attachment #9058652 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

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

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox68: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

Nils, did you manage to create a minimized testcase for this issue? If not, could you please help us with some STR for manual verification?

Flags: needinfo?(nils)

Brindusa, I haven't been able to create a minimized testcase. I have tried to reproduce this many times on a current build and was unable to do so.

Flags: needinfo?(nils)

Thanks Nils!

Whiteboard: [adv-main67+][adv-esr60.7+]
Alias: CVE-2019-11692

(In reply to Brindusa Tot[:brindusat] from comment #21)

Nils, did you manage to create a minimized testcase for this issue? If not, could you please help us with some STR for manual verification?(In reply to Nils from comment #22)

Brindusa, I haven't been able to create a minimized testcase. I have tried to reproduce this many times on a current build and was unable to do so.

Based on this, I will remove the qe-verify+ flag.

Flags: qe-verify+ → qe-verify-
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Group: core-security-release

Comment on attachment 9158785 [details]
Bug 1544670 - Set minimum time delta value to avoid division by zero. r=zombie

Revision D80805 was moved to bug 1544570. Setting attachment 9158785 [details] to obsolete.

Attachment #9158785 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: