Headline

CVE-2019-17010: 1581084 - (CVE-2019-17010) mozilla::EventListenerManager::AddEventListenerInternal

Under certain conditions, when checking the Resist Fingerprinting preference during device orientation checks, a race condition could have caused a use-after-free and a potentially exploitable crash. This vulnerability affects Thunderbird < 68.3, Firefox ESR < 68.3, and Firefox < 71.

4 years ago

CVE

Open in Source

#vulnerability #web #linux #js

Closed Bug 1581084 (CVE-2019-17010) Opened 3 years ago Closed 3 years ago

I’m seeing this crash quite regularly while fuzzing, however the testcases are not reproducible. If we can find a way to reliable trigger a situation where nsDocShell::CreateAboutBlankContentViewer is called in EnsureContentViewer I should be to craft a testcase. I am seeing this on ondevicemotion and ondeviceorientation events and also with a slightly different stack with onstorage event on the window object.

=================================================================
==8535==ERROR: AddressSanitizer: heap-use-after-free on address 0x611001e24504 at pc 0x7ff429b5b3e7 bp 0x7fff90876a10 sp 0x7fff90876a08
READ of size 1 at 0x611001e24504 thread T0 (Web Content)
    #0 0x7ff429b5b3e6 in IsApzAwareListener /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1680:29
    #1 0x7ff429b5b3e6 in mozilla::EventListenerManager::AddEventListenerInternal(mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener>, mozilla::EventMessage, nsAtom*, mozilla::EventListenerFlags const&, bool, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:396
    #2 0x7ff429b5eb97 in mozilla::EventListenerManager::SetEventHandlerInternal(nsAtom*, mozilla::TypedEventHandler const&, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:728:5
    #3 0x7ff429b6a5a7 in mozilla::EventListenerManager::SetEventHandler(nsAtom*, mozilla::dom::EventHandlerNonNull*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1529:3
    #4 0x7ff4287d5d5b in SetOndeviceorientation /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventNameList.h:326:1
    #5 0x7ff4287d5d5b in mozilla::dom::Window_Binding::set_ondeviceorientation(JSContext*, JS::Handle<JSObject*>, nsGlobalWindowInner*, JSJitSetterCallArgs) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:6780
    #6 0x7ff4293cb266 in bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::MaybeGlobalThisPolicy>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3121:8
    #7 0x7ff42ffb33a7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:447:13
    #8 0x7ff42ffb33a7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:539
    #9 0x7ff42ffb943d in InternalCall /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:594:10
    #10 0x7ff42ffb943d in Call /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:610
    #11 0x7ff42ffb943d in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:748
    #12 0x7ff4304fca53 in SetExistingProperty(JSContext*, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2932:8
    #13 0x7ff4304f5521 in bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2961:14
    #14 0x7ff4301fea57 in SetProperty /builds/worker/workspace/build/src/js/src/vm/ObjectOperations-inl.h:283:10
    #15 0x7ff4301fea57 in js::ForwardingProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:149
    #16 0x7ff426807ac0 in nsOuterWindowProxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp:944:23
    #17 0x7ff4301d38c1 in setInternal /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:395:19
    #18 0x7ff4301d38c1 in js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:403
    #19 0x7ff42ff91878 in SetProperty /builds/worker/workspace/build/src/js/src/vm/ObjectOperations-inl.h:280:12
    #20 0x7ff42ff91878 in SetPropertyOperation /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:269
    #21 0x7ff42ff91878 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2849
    #22 0x7ff42ff7ccef in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:424:10
    #23 0x7ff42ffb9c6f 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:786:13
    #24 0x7ff43009eb33 in js::DirectEvalStringFromIon(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Handle<JS::Value>, JS::Handle<JSString*>, unsigned char*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:422:10
    #25 0x308de91b36ac  (<unknown module>)

0x611001e24504 is located 196 bytes inside of 256-byte region [0x611001e24440,0x611001e24540)
freed by thread T0 (Web Content) here:
    #0 0x55b630990a92 in __interceptor_free /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7ff422678916 in Free /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:197:34
    #2 0x7ff422678916 in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::ShrinkCapacity(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray-inl.h:229
    #3 0x7ff429b58319 in Clear /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:248:12
    #4 0x7ff429b58319 in RemoveAllListenersSilently /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:147
    #5 0x7ff429b58319 in mozilla::EventListenerManager::Disconnect() /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1315
    #6 0x7ff4267a8b60 in nsGlobalWindowInner::FreeInnerObjects() /builds/worker/workspace/build/src/dom/base/nsGlobalWindowInner.cpp:1102:23
    #7 0x7ff42681240b in nsGlobalWindowOuter::SetNewDocument(mozilla::dom::Document*, nsISupports*, bool, mozilla::dom::WindowGlobalChild*) /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp:2231:19
    #8 0x7ff42c4ebb35 in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::dom::WindowGlobalChild*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool) /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:983:22
    #9 0x7ff42c4eaefa in nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::dom::WindowGlobalChild*) /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:743:10
    #10 0x7ff42f136c96 in nsDocShell::SetupNewViewer(nsIContentViewer*, mozilla::dom::WindowGlobalChild*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:8438:7
    #11 0x7ff42f135b79 in nsDocShell::Embed(nsIContentViewer*, mozilla::dom::WindowGlobalChild*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6217:17
    #12 0x7ff42f140bcc in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIPrincipal*, nsIContentSecurityPolicy*, nsIURI*, bool, bool, mozilla::dom::WindowGlobalChild*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7052:14
    #13 0x7ff42f0eb329 in nsDocShell::EnsureContentViewer() /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6887:17
    #14 0x7ff42f117ba7 in GetDocument /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:3590:3
    #15 0x7ff42f117ba7 in non-virtual thunk to nsDocShell::GetDocument() /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp
    #16 0x7ff4266f3ace in nsContentUtils::ShouldResistFingerprinting(nsIDocShell*) /builds/worker/workspace/build/src/dom/base/nsContentUtils.cpp:1967:48
    #17 0x7ff42b374fe2 in nsDeviceSensors::IsSensorAllowedByPref(unsigned int, nsIDOMWindow*) /builds/worker/workspace/build/src/dom/system/nsDeviceSensors.cpp:574:11
    #18 0x7ff42b37550c in nsDeviceSensors::AddWindowListener(unsigned int, nsIDOMWindow*) /builds/worker/workspace/build/src/dom/system/nsDeviceSensors.cpp:156:8
    #19 0x7ff4267ed998 in nsGlobalWindowInner::EnableDeviceSensor(unsigned int) /builds/worker/workspace/build/src/dom/base/nsGlobalWindowInner.cpp:5973:9
    #20 0x7ff429b59449 in EnableDevice /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:478:15
    #21 0x7ff429b59449 in mozilla::EventListenerManager::AddEventListenerInternal(mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener>, mozilla::EventMessage, nsAtom*, mozilla::EventListenerFlags const&, bool, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:290
    #22 0x7ff429b5eb97 in mozilla::EventListenerManager::SetEventHandlerInternal(nsAtom*, mozilla::TypedEventHandler const&, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:728:5
    #23 0x7ff429b6a5a7 in mozilla::EventListenerManager::SetEventHandler(nsAtom*, mozilla::dom::EventHandlerNonNull*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1529:3
    #24 0x7ff4287d5d5b in SetOndeviceorientation /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventNameList.h:326:1
    #25 0x7ff4287d5d5b in mozilla::dom::Window_Binding::set_ondeviceorientation(JSContext*, JS::Handle<JSObject*>, nsGlobalWindowInner*, JSJitSetterCallArgs) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:6780
    #26 0x7ff4293cb266 in bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::MaybeGlobalThisPolicy>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3121:8
    #27 0x7ff42ffb33a7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:447:13
    #28 0x7ff42ffb33a7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:539
    #29 0x7ff42ffb943d in InternalCall /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:594:10
    #30 0x7ff42ffb943d in Call /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:610
    #31 0x7ff42ffb943d in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:748
    #32 0x7ff4304fca53 in SetExistingProperty(JSContext*, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2932:8
    #33 0x7ff4304f5521 in bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2961:14
    #34 0x7ff4301fea57 in SetProperty /builds/worker/workspace/build/src/js/src/vm/ObjectOperations-inl.h:283:10
    #35 0x7ff4301fea57 in js::ForwardingProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:149
    #36 0x7ff426807ac0 in nsOuterWindowProxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp:944:23
    #37 0x7ff4301d38c1 in setInternal /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:395:19
    #38 0x7ff4301d38c1 in js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:403
    #39 0x7ff42ff91878 in SetProperty /builds/worker/workspace/build/src/js/src/vm/ObjectOperations-inl.h:280:12
    #40 0x7ff42ff91878 in SetPropertyOperation /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:269
    #41 0x7ff42ff91878 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2849
    #42 0x7ff42ff7ccef in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:424:10

previously allocated by thread T0 (Web Content) here:
    #0 0x55b63099122f in __interceptor_realloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:165:3
    #1 0x55b6309c668d in moz_xrealloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:72:18
    #2 0x7ff4226777a9 in Realloc /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:204:12
    #3 0x7ff4226777a9 in nsTArrayInfallibleAllocator::ResultTypeProxy nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray-inl.h:191
    #4 0x7ff429b588db in ExtendCapacity<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray-inl.h:117:16
    #5 0x7ff429b588db in AppendElements<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1691
    #6 0x7ff429b588db in AppendElement<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1716
    #7 0x7ff429b588db in AppendElement /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:192
    #8 0x7ff429b588db in mozilla::EventListenerManager::AddEventListenerInternal(mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener>, mozilla::EventMessage, nsAtom*, mozilla::EventListenerFlags const&, bool, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:236
    #9 0x7ff429b5eb97 in mozilla::EventListenerManager::SetEventHandlerInternal(nsAtom*, mozilla::TypedEventHandler const&, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:728:5
    #10 0x7ff429b6a5a7 in mozilla::EventListenerManager::SetEventHandler(nsAtom*, mozilla::dom::EventHandlerNonNull*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1529:3
    #11 0x7ff42880a06b in SetOndragover /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventNameList.h:173:1
    #12 0x7ff42880a06b in mozilla::dom::Window_Binding::set_ondragover(JSContext*, JS::Handle<JSObject*>, nsGlobalWindowInner*, JSJitSetterCallArgs) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:13423
    #13 0x7ff4293cb266 in bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::MaybeGlobalThisPolicy>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3121:8
    #14 0x7ff42ffb33a7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:447:13
    #15 0x7ff42ffb33a7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:539
    #16 0x7ff42ffb943d in InternalCall /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:594:10
    #17 0x7ff42ffb943d in Call /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:610
    #18 0x7ff42ffb943d in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:748
    #19 0x7ff4304fca53 in SetExistingProperty(JSContext*, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2932:8
    #20 0x7ff4304f5521 in bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2961:14
    #21 0x7ff4301fea57 in SetProperty /builds/worker/workspace/build/src/js/src/vm/ObjectOperations-inl.h:283:10
    #22 0x7ff4301fea57 in js::ForwardingProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:149
    #23 0x7ff426807ac0 in nsOuterWindowProxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp:944:23
    #24 0x7ff4301d38c1 in setInternal /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:395:19
    #25 0x7ff4301d38c1 in js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:403
    #26 0x7ff42ff91878 in SetProperty /builds/worker/workspace/build/src/js/src/vm/ObjectOperations-inl.h:280:12
    #27 0x7ff42ff91878 in SetPropertyOperation /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:269
    #28 0x7ff42ff91878 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2849
    #29 0x7ff42ff7ccef in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:424:10
    #30 0x7ff42ffb9c6f 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:786:13
    #31 0x7ff43009eb33 in js::DirectEvalStringFromIon(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Handle<JS::Value>, JS::Handle<JSString*>, unsigned char*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:422:10
    #32 0x308de91b36ac  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1680:29 in IsApzAwareListener
Shadow bytes around the buggy address:
  0x0c22803bc850: 00 00 00 00 fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c22803bc860: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c22803bc870: 00 00 00 00 00 00 00 00 00 00 00 04 fa fa fa fa
  0x0c22803bc880: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c22803bc890: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c22803bc8a0:[fd]fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c22803bc8b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22803bc8c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22803bc8d0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c22803bc8e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c22803bc8f0: 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
  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
==8535==ABORTING

(In reply to Nils from comment #0)

I’m seeing this crash quite regularly while fuzzing, however the testcases are not reproducible. If we can find a way to reliable trigger a situation where nsDocShell::CreateAboutBlankContentViewer is called in EnsureContentViewer I should be to craft a testcase.

I don’t know much about fuzzing, but if you have chrome JS execution privileges, creating a <browser nodefaultsrc="true"/> (optionally remote) and then calling docShell.getInterface(Ci.nsIDocument) on the browser’s docshell I suspect would do the trick?

Group: core-security → dom-core-security

Olli: does the stack in comment 0 have enough hints to make any progress here without a testcase?

Assignee: nobody → bugs

Flags: needinfo?(bugs)

Though, I don’t yet see where the issue is. Unfortunately the stack trace doesn’t contain the whole stack trace, only the top of it.

Hmm, but but yes… that stack trace is enough

The bug looks to be in ShouldResistFingerprinting setup.

Component: DOM: Events → Security

I’ve tried and failed to write a testcase. Somehow there is a window wrapper to a window which has (still?) docshell but doesn’t have contentviewer.
Will upload a patch which should fix the issue shown in the stack trace. I haven’t figured out what could cause something similar with onstorage.
The patch will take the principal from window object without accessing docshell. And if there is no document to get principal, window will ask its parent (which is effectively what implicit about:blank creation and getting principal does too with iframes).

ShouldResistFingerprinting(foo*) methods in general seem to have the problem that they return false if null is passed. That is not something I’m trying to fix here.

See attachment for crash with onstorage. I am getting a few of these crashes a day so once we have an ASAN build with fix I can tell whether this works

My patch doesn’t affect onstorage case.
onstorage requires fixes in nsGlobalWindowOuter::NotifyContentBlockingEvent.
Nils, want to file a new bug about onstorage?
Probably Core: Privacy: Anti-Tracking

(I still very much wish to have some testcase for any of these crashes. One such would explain other cases too. And thinking also other ways to fix this. In principle we must not create about:blank in totally random places.)

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1591334 for the onstorage case.

I have tried to come up with a testcase manually without success. Could you think of an assertion we could add that catches this more reliable or maybe catches the Somehow there is a window wrapper to a window which has (still?) docshell but doesn't have contentviewer case?

Comment on attachment 9104055 [details]
Bug 1581084, nsDeviceSensors::IsSensorAllowedByPref should use the principal of the window, not docshell, to check whether to resist fingerprinting, r=bzbarsky

Security Approval Request

How easily could an exploit be constructed based on the patch?: Not very easily, since we don’t even have a way to reproduce the asan failure.

When evaluating when to land the patch, see also bug 1591334.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
Which older supported branches are affected by this flaw?: Maybe 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?: (the patch seems to apply cleanly to esr68)
How likely is this patch to cause regressions; how much testing does it need?: Shouldn’t cause regressions too likely

Comment on attachment 9104055 [details]
Bug 1581084, nsDeviceSensors::IsSensorAllowedByPref should use the principal of the window, not docshell, to check whether to resist fingerprinting, r=bzbarsky

Low risk of exploitation, let’s land the two now and see if it fixes Nils’ stuff. Send in an uplift request when you think prudent.

Group: dom-core-security → core-security-release

Status: NEW → RESOLVED

Closed: 3 years ago

Resolution: — → FIXED

Target Milestone: — → mozilla72

Please nominate this for Beta and ESR68 approval when you get a chance. It grafts cleanly to both as-landed.

Comment on attachment 9104055 [details]
Bug 1581084, nsDeviceSensors::IsSensorAllowedByPref should use the principal of the window, not docshell, to check whether to resist fingerprinting, r=bzbarsky

Beta/Release Uplift Approval Request

User impact if declined: Crashes
Is this code covered by automated tests?: No
Has the fix been verified in Nightly?: No
Needs manual test from QE?: No
If yes, steps to reproduce: (problem here is that we don’t have a testcase. The patch is based on the stack trace)
List of other uplifts needed: None
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): Affects how resist-fingerprinting check is done when one uses device sensors. The pref for resist-fingerprinting is false by default in Firefox.
String changes made/needed: NA

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Crashes
User impact if declined: Crashes
Fix Landed on Version: 72
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): Affects how resist-fingerprinting check is done when one uses device sensors. The pref for resist-fingerprinting is false by default in Firefox.
String or UUID changes made by this patch: NA

Comment on attachment 9104055 [details]
Bug 1581084, nsDeviceSensors::IsSensorAllowedByPref should use the principal of the window, not docshell, to check whether to resist fingerprinting, r=bzbarsky

sec-moderate, on nightly, low risk, uplift approved for 71 beta 8, thanks.

Comment on attachment 9104055 [details]
Bug 1581084, nsDeviceSensors::IsSensorAllowedByPref should use the principal of the window, not docshell, to check whether to resist fingerprinting, r=bzbarsky

Fixes a sec issue. Approved for 68.3esr.

Flags: sec-bounty? → sec-bounty+

Whiteboard: [adv-main71+]

Whiteboard: [adv-main71+] → [adv-main71+][adv-esr68.3+]

Group: core-security-release