Closed Bug 1356558 (CVE-2017-7750) Opened 7 years ago Closed 7 years ago

heap-use-after-free in nsViewManager::IsPainting

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Version:
50 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr45 --- unaffected
firefox52 --- wontfix
firefox-esr52 54+ fixed
firefox53 --- wontfix
firefox54 + fixed
firefox55 + fixed

People

(Reporter: nils, Assigned: bevis)

References

Details

(4 keywords, Whiteboard: [adv-main54+][adv-esr52.2+])

Attachments

(1 file, 5 obsolete files)

(v4) Do not run ScriptRunner in the intermediate state before destroying PreShell, PresContext, etc at nsDocumentViewer::Hide() is done. r=bz
1.68 KB, patch
bevis
: review+
gchang
: approval-mozilla-beta+
gchang
: approval-mozilla-esr52+
dveditz
: sec-approval+
Details | Diff | Splinter Review 
The following testcase crashes the latest ASAN build of Firefox:

<script>
function start() {
	o2=document.createElement('iframe');
	o2.src="data:text/html,<div>";
	o2.addEventListener('load', fun0,false);
	document.body.appendChild(o2);
}
function fun0() {
	o456=frames[0];
	o457=o456.document;
	o457.designMode='on';
	o544=o457.createElement('audio');
	o545=document.createElement('track');
	o544.appendChild(o545);
	o457.write('<div>');
	o544.controls^=1;
	o457.documentElement.appendChild(o544);
	o457.defaultView.onerror=fun3;
	setTimeout(fun2, 4);
}
function fun2() {
	document.documentElement.hidden^=true;
}
function fun3() {
	document.documentElement.style.display='grid';
	document.documentElement.getBoundingClientRect();
}
</script>
<body onload="start()"></body>

ASAN output:
=================================================================
==10317==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600031d7b0 at pc 0x7f30c7d993cb bp 0x7fff70111a40 sp 0x7fff70111a38
READ of size 8 at 0x60600031d7b0 thread T0 (Web Content)
    #0 0x7f30c7d993ca in RootViewManager /home/worker/workspace/build/src/view/nsViewManager.h:373:51
    #1 0x7f30c7d993ca in IsPainting /home/worker/workspace/build/src/view/nsViewManager.h:364
    #2 0x7f30c7d993ca in nsViewManager::IsPainting(bool&) /home/worker/workspace/build/src/view/nsViewManager.cpp:1083
    #3 0x7f30c851eb35 in IsSafeToFlush /home/worker/workspace/build/src/layout/base/PresShell.cpp:4069:20
    #4 0x7f30c851eb35 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /home/worker/workspace/build/src/layout/base/PresShell.cpp:4139
    #5 0x7f30c8491162 in FlushPendingNotifications /home/worker/workspace/build/src/layout/base/nsIPresShell.h:608:5
    #6 0x7f30c8491162 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1807
    #7 0x7f30c849fa23 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:299:7
    #8 0x7f30c849f6f4 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:321:5
    #9 0x7f30c84a1d5b in RunRefreshDrivers /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:711:5
    #10 0x7f30c84a1d5b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:624
    #11 0x7f30c84a1a59 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:525:9
    #12 0x7f30c8cebc64 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /home/worker/workspace/build/src/layout/ipc/VsyncChild.cpp:64:16
    #13 0x7f30c2f78647 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:155:20
    #14 0x7f30c2c36d46 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1512:28
    #15 0x7f30c2b96880 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1872:25
    #16 0x7f30c2b930c7 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1807:17
    #17 0x7f30c2b954f4 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1680:5
    #18 0x7f30c2b95af6 in mozilla::ipc::MessageChannel::MessageTask::Run() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1713:15
    #19 0x7f30c1de3b90 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1269:14
    #20 0x7f30c1de05d8 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #21 0x7f30c2b9ddf6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:124:5
    #22 0x7f30c2afffe0 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #23 0x7f30c2afffe0 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #24 0x7f30c2afffe0 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #25 0x7f30c7e125ff in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:27
    #26 0x7f30cb416a17 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:875:22
    #27 0x7f30c2afffe0 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #28 0x7f30c2afffe0 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #29 0x7f30c2afffe0 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #30 0x7f30cb41640f in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:699:34
    #31 0x4eb5c3 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:30
    #32 0x4eb5c3 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:286
    #33 0x7f30dd56a82f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291
    #34 0x41cf18 in _start (/home/nils/fuzzer3/firefox/firefox+0x41cf18)

0x60600031d7b0 is located 48 bytes inside of 64-byte region [0x60600031d780,0x60600031d7c0)
freed by thread T0 (Web Content) here:
    #0 0x4bb44b in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3
    #1 0x7f30c85f28a3 in operator delete /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:218:12
    #2 0x7f30c85f28a3 in Release /home/worker/workspace/build/src/view/nsViewManager.h:33
    #3 0x7f30c85f28a3 in Release /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:40
    #4 0x7f30c85f28a3 in Release /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:395
    #5 0x7f30c85f28a3 in assign_assuming_AddRef /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:65
    #6 0x7f30c85f28a3 in operator= /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:166
    #7 0x7f30c85f28a3 in nsDocumentViewer::Hide() /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:2188
    #8 0x7f30ca941be9 in SetVisibility /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6513:9
    #9 0x7f30ca941be9 in non-virtual thunk to nsDocShell::SetVisibility(bool) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6503
    #10 0x7f30c47729fa in nsFrameLoader::Hide() /home/worker/workspace/build/src/dom/base/nsFrameLoader.cpp:1312:12
    #11 0x7f30c89c8b61 in nsHideViewer::Run() /home/worker/workspace/build/src/layout/generic/nsSubDocumentFrame.cpp:922:21
    #12 0x7f30c431d81f in nsContentUtils::RemoveScriptBlocker() /home/worker/workspace/build/src/dom/base/nsContentUtils.cpp:5362:15
    #13 0x7f30c851f023 in ~nsAutoScriptBlocker /home/worker/workspace/build/src/obj-firefox/dist/include/nsContentUtils.h:2986:5
    #14 0x7f30c851f023 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /home/worker/workspace/build/src/layout/base/PresShell.cpp:4210
    #15 0x7f30c8491954 in FlushPendingNotifications /home/worker/workspace/build/src/layout/base/nsIPresShell.h:608:5
    #16 0x7f30c8491954 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1772
    #17 0x7f30c849fa23 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:299:7
    #18 0x7f30c849f6f4 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:321:5
    #19 0x7f30c84a1d5b in RunRefreshDrivers /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:711:5
    #20 0x7f30c84a1d5b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:624
    #21 0x7f30c84a1a59 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:525:9
    #22 0x7f30c8cebc64 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /home/worker/workspace/build/src/layout/ipc/VsyncChild.cpp:64:16
    #23 0x7f30c2f78647 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:155:20
    #24 0x7f30c2c36d46 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1512:28
    #25 0x7f30c2b96880 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1872:25
    #26 0x7f30c2b930c7 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1807:17
    #27 0x7f30c2b954f4 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1680:5
    #28 0x7f30c2b95af6 in mozilla::ipc::MessageChannel::MessageTask::Run() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1713:15
    #29 0x7f30c1de3b90 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1269:14
    #30 0x7f30c1de05d8 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #31 0x7f30c2b9ddf6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:124:5
    #32 0x7f30c2afffe0 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #33 0x7f30c2afffe0 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #34 0x7f30c2afffe0 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #35 0x7f30c7e125ff in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:27
    #36 0x7f30cb416a17 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:875:22
    #37 0x7f30c2afffe0 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #38 0x7f30c2afffe0 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #39 0x7f30c2afffe0 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #40 0x7f30cb41640f in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:699:34
    #41 0x4eb5c3 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:30
    #42 0x4eb5c3 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:286
    #43 0x7f30dd56a82f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291

previously allocated by thread T0 (Web Content) here:
    #0 0x4bb79c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
    #1 0x4ec75d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7f30c85f1e3b in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12
    #3 0x7f30c85f1e3b in nsDocumentViewer::MakeWindow(nsSize const&, nsView*) /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:2446
    #4 0x7f30c85fe6f0 in nsDocumentViewer::Show() /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:2104:10
    #5 0x7f30ca941be9 in SetVisibility /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6513:9
    #6 0x7f30ca941be9 in non-virtual thunk to nsDocShell::SetVisibility(bool) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6503
    #7 0x7f30c4771941 in nsFrameLoader::Show(int, int, int, int, nsSubDocumentFrame*) /home/worker/workspace/build/src/dom/base/nsFrameLoader.cpp:1140:15
    #8 0x7f30c894f641 in nsSubDocumentFrame::ShowViewer() /home/worker/workspace/build/src/layout/generic/nsSubDocumentFrame.cpp:185:22
    #9 0x7f30c89c85e4 in AsyncFrameInit::Run() /home/worker/workspace/build/src/layout/generic/nsSubDocumentFrame.cpp:92:60
    #10 0x7f30c431d81f in nsContentUtils::RemoveScriptBlocker() /home/worker/workspace/build/src/dom/base/nsContentUtils.cpp:5362:15
    #11 0x7f30c851f023 in ~nsAutoScriptBlocker /home/worker/workspace/build/src/obj-firefox/dist/include/nsContentUtils.h:2986:5
    #12 0x7f30c851f023 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /home/worker/workspace/build/src/layout/base/PresShell.cpp:4210
    #13 0x7f30c46fd041 in FlushPendingNotifications /home/worker/workspace/build/src/obj-firefox/dist/include/nsIPresShell.h:599:5
    #14 0x7f30c46fd041 in nsDocument::FlushPendingNotifications(mozilla::FlushType) /home/worker/workspace/build/src/dom/base/nsDocument.cpp:8056
    #15 0x7f30c44e7118 in GetPrimaryFrame /home/worker/workspace/build/src/dom/base/Element.cpp:2176:10
    #16 0x7f30c44e7118 in mozilla::dom::Element::GetBoundingClientRect() /home/worker/workspace/build/src/dom/base/Element.cpp:967
    #17 0x7f30c5bbe7bc in mozilla::dom::ElementBinding::getBoundingClientRect(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:2086:59
    #18 0x7f30c609344e in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2953:13
    #19 0x7f30cb8ccc63 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #20 0x7f30cb8ccc63 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:455
    #21 0x7f30cb8b568f in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:506:12
    #22 0x7f30cb8b568f in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2997
    #23 0x7f30cb89bb28 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:395:12
    #24 0x7f30cb8ccde8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473:15
    #25 0x7f30cb8cd612 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:519:10
    #26 0x7f30cc4e08ee in js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:165:12
    #27 0x7f30cc497f94 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:353:23
    #28 0x7f30cc4c1153 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:464:21
    #29 0x7f30cc4c3ab7 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:716:12
    #30 0x7f30cb8ccfb3 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #31 0x7f30cb8ccfb3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:437
    #32 0x7f30cb8cd612 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:519:10
    #33 0x7f30cc24c17b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2885:12
    #34 0x7f30c5b071c4 in mozilla::dom::OnErrorEventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::EventOrString const&, mozilla::dom::Optional<nsAString> const&, mozilla::dom::Optional<unsigned int> const&, mozilla::dom::Optional<unsigned int> const&, mozilla::dom::Optional<JS::Handle<JS::Value> > const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:398:37
    #35 0x7f30c649ae83 in Call<nsISupports *> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:532:12
    #36 0x7f30c649ae83 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /home/worker/workspace/build/src/dom/events/JSEventHandler.cpp:166
    #37 0x7f30c64670e2 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1123:51
    #38 0x7f30c6468ecc in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1297:20

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/view/nsViewManager.h:373:51 in RootViewManager
Shadow bytes around the buggy address:
  0x0c0c8005baa0: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c0c8005bab0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c0c8005bac0: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x0c0c8005bad0: fd fd fd fd fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c8005bae0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
=>0x0c0c8005baf0: fd fd fd fd fd fd[fd]fd fa fa fa fa fd fd fd fd
  0x0c0c8005bb00: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c0c8005bb10: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c0c8005bb20: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x0c0c8005bb30: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c0c8005bb40: fa fa fa fa fd fd fd fd fd fd fd fd 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
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==10317==ABORTING
INFO: Last good revision: d60c87d89619d97f20a4443c3357f1baffba3cea
INFO: First bad revision: 98f2ac456f11f55823e7687cf7d97da9dc0f991a
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d60c87d89619d97f20a4443c3357f1baffba3cea&tochange=98f2ac456f11f55823e7687cf7d97da9dc0f991a

Bug 871747 looks like the likely culprit there.
Has Regression Range: --- → yes
status-firefox52: --- → wontfix
status-firefox53: --- → wontfix
status-firefox54: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → affected
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
tracking-firefox-esr52: --- → ?
Flags: needinfo?(bechen)
Flags: in-testsuite?
Version: Trunk → 50 Branch
Hmm, I don't have idea about this bug.

Bug 871747 uses the nsContentUtils::RunInStableState to run a task which should do nothing.
And the free stack track at comment 0 shows
#12 0x7f30c431d81f in nsContentUtils::RemoveScriptBlocker() /home/worker/workspace/build/src/dom/base/nsContentUtils.cpp:5362:15
#13 0x7f30c851f023 in ~nsAutoScriptBlocker /home/worker/workspace/build/src/obj-firefox/dist/include/nsContentUtils.h:2986:5

Maybe there are some conditions for calling nsContentUtils::RunInStableState?
Seems that we need to ni someone who is familiar with nsViewManager.h.
Flags: needinfo?(bechen)
Have we verified, via local backout or bisect, that bug 871747 is the problem?

If so, the problem may not be the RunInStableState call.  It might be whatever other work is done.  But first, we should check whether bug 871747 is the issue.  Local backout should be pretty trivial.

In the crash stack, what event is firing that triggers the stack leading to the crash?  On what target?
Flags: needinfo?(bechen)
(In reply to [Out of Office Until 24-April] Ryan VanderMeulen [:RyanVM] from comment #4)
> Here's a crash report from a current nightly build:
> https://crash-stats.mozilla.com/report/index/1e2823ef-b727-40ad-ba3c-
> 8b4610170421
> 
> A Try push with bug 871747 backed out doesn't crash for me.
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=2ef45d22acc1cbf1b5374982d85d96122e0d582b

Now I can reproduce it at my local asan build.
And I also do some basic debug:
When the TrackElement binded to the AudioElement, the HTMLTrackElement::BindToTree call HTMLTrackElement::CreateTextTrack and 
HTMLTrackElement::DispatchLoadResource. I just remove the DispatchLoadResource function, and still crash. So we can exclude the sContentUtils::RunInStableState.
The CreateTextTrack will hold the reference about the nsPIDOMWindowInner, maybe something relative to the func2 and func3 in testcase.
Flags: needinfo?(bechen)
Update status:

I suspect there is something wrong in HTMLTrackElement::BindToTree and HTMLTrackElement::CreateTextTrack.

The testcase create a TrackElement and an AudioElement, then
1. bind TrackElement to AudioElement.
  - call createTectTrack
2. bind AudioElement to domtree. 
Crash.

If we change the binding order in testcase:
1. bind audioElement to domtree
2. bind TrackElement to AudioElement
  - call createTectTrack
No crash.
OK.  So one obvious difference between those two cases is that the first case will call HTMLTrackElement::BindToTree twice: the first time when it's added as a child of the AudioElement and the second time when the whole thing is placed in the DOM.

But I would expect the second call to do nothing, since at that point mMediaParent is already set, right?

The other important thing that happens in this testcase is the 

  o457.write('<div>');

that happens between the o544.appendChild(o545); and o457.documentElement.appendChild(o544); call.  That will blow away the existing DOM descendants of frames[0].document, create a new Window for it, etc.  Note that the TextTrack that the HTMLTrackElement is holding is parented to the old Window.  Which isn't necessarily a problem per se... but I bet if you remove that line the crash goes away, right?  Is that also true if the order of that line and the "o544.appendChild(o545)" line is switched?

It's a bit hard to make sense of the comment 0 stacks, because the line numbers don't line up with current source code and I'm not sure which revision that comment is based on.  But it _looks_ like  nsRefreshDriver::Tick at nsRefreshDriver.cpp:1772 makes a FlushPendingNotifications call on a presshell.  This ends up removing a scriptblocker, which triggers nsHideViewer::Run, which destroys a viewmanager.  Note that the presshell holds a weak ref to the viewmanager... but should be nulling it out in PresShell::Destroy.    Which should also be called by nsDocumentViewer::Hide.

Anyway, then nsRefreshDriver::Tick at nsRefreshDriver.cpp:1807 does another FlushPendingNotifications call.  Presumablt this is in the layout-flush branch, whereas the other was in the style-flush branch.  Is it on the same presshell whose vewmanager died during that previous call, or a different one?  If it's the same one, why does it not have a null mViewManager?
Flags: needinfo?(bechen)
Thanks for the information, very useful. Me and bevistseng will keep debugging next Monday. 

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #7)
> OK.  So one obvious difference between those two cases is that the first
> case will call HTMLTrackElement::BindToTree twice: the first time when it's
> added as a child of the AudioElement and the second time when the whole
> thing is placed in the DOM.
> 
> But I would expect the second call to do nothing, since at that point
> mMediaParent is already set, right?
Yes, because the mMediaParent and mTextTrack are not null.

> 
> The other important thing that happens in this testcase is the 
> 
>   o457.write('<div>');
> 
> that happens between the o544.appendChild(o545); and
> o457.documentElement.appendChild(o544); call.  That will blow away the
> existing DOM descendants of frames[0].document, create a new Window for it,
> etc.  Note that the TextTrack that the HTMLTrackElement is holding is
> parented to the old Window.  Which isn't necessarily a problem per se... but
> I bet if you remove that line the crash goes away, right?  Is that also true
> if the order of that line and the "o544.appendChild(o545)" line is switched?
Here looks like it is a problem that "TextTrack is holding the old Window".
I should handle it at HTMLTrackElement::BindtoTree.

This explains that why the two BindtoTree calls have the same OnwerDoc() but different innerWindow(globalObject).

> 
> It's a bit hard to make sense of the comment 0 stacks, because the line
> numbers don't line up with current source code and I'm not sure which
> revision that comment is based on.  But it _looks_ like 
> nsRefreshDriver::Tick at nsRefreshDriver.cpp:1772 makes a
> FlushPendingNotifications call on a presshell.  This ends up removing a
> scriptblocker, which triggers nsHideViewer::Run, which destroys a
> viewmanager.  Note that the presshell holds a weak ref to the viewmanager...
> but should be nulling it out in PresShell::Destroy.    Which should also be
> called by nsDocumentViewer::Hide.
> 
> Anyway, then nsRefreshDriver::Tick at nsRefreshDriver.cpp:1807 does another
> FlushPendingNotifications call.  Presumablt this is in the layout-flush
> branch, whereas the other was in the style-flush branch.  Is it on the same
> presshell whose vewmanager died during that previous call, or a different
> one?  If it's the same one, why does it not have a null mViewManager?

So based on the testcase
1. TextTrack hold a old Window because the "o457.write('<div>');"
2. The relation ship between nsDocumentViewer mViewManager mPresShell is not robust under the testcase.

keep debugging.
> Here looks like it is a problem that "TextTrack is holding the old Window".

It's not really a problem you can do much about, or should.  That's just how things work; objects created before the document.open will have JS wrappers from the old window.
I am not really familiar with nsDocumentViewer but, when the symptom happens I always git an assertion at nsDocumentViewer::Show() when ::Hide() is called:
http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/layout/base/nsDocumentViewer.cpp#2106

> NS_ASSERTION(!mPresContext, "Shouldn't have a prescontext if we have no shell!");

Hence, it seems NG to re-enter to ::Show() before Hide() is done caused by the nsAutoScriptBlocker in nsDocumentViewer::DestroyPresShell().
I didn't see this UAF problem anymore with this WIP patch by have nsAutoAutoScriptBlocker in Hide() to ensure that all the ScriptRunner will be called after PresShell, PresContext, ViewManager, etc are destroyed.

However, I am not sure if this will break other things.

Here is the call stack when reentering ::Show() was hit for reference:
#0  nsDocumentViewer::Show (this=0x6120002899c0)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsDocumentViewer.cpp:2050
#1  0x00007f1fef720a66 in nsDocShell::SetVisibility (this=<optimized out>,
    aVisibility=<error reading variable: access outside bounds of object referenced via synthetic pointer>) at /home/bevis/Projects/Build/gecko/src/docshell/base/nsDocShell.cpp:6511
#2  0x00007f1fef720ae0 in non-virtual thunk to nsDocShell::SetVisibility(bool) ()
    at /home/bevis/Projects/Build/gecko/src/docshell/base/nsDocShell.cpp:6501
#3  0x00007f1fe962a070 in nsFrameLoader::Show (this=<optimized out>, marginWidth=1634768,
    marginHeight=149281760, scrollbarPrefX=149281792, scrollbarPrefY=-1591952512, frame=0x60c0008bd2c0)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsFrameLoader.cpp:1140
#4  0x00007f1fed7b271b in nsSubDocumentFrame::ShowViewer (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/layout/generic/nsSubDocumentFrame.cpp:185
#5  0x00007f1fed81dd1d in AsyncFrameInit::Run (this=0x6040005fad90)
    at /home/bevis/Projects/Build/gecko/src/layout/generic/nsSubDocumentFrame.cpp:92
#6  0x00007f1fe920e432 in nsContentUtils::RemoveScriptBlocker ()
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5390
#7  0x00007f1fed37742b in nsAutoScriptBlocker::~nsAutoScriptBlocker (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/nsContentUtils.h:3036
#8  mozilla::PresShell::DoFlushPendingNotifications (this=0x61800019c480, aFlush=...)
    at /home/bevis/Projects/Build/gecko/src/layout/base/PresShell.cpp:4206
#9  0x00007f1fe95cab37 in nsIPresShell::FlushPendingNotifications (this=0x61800019c480,
    aType=mozilla::FlushType::Layout)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/nsIPresShell.h:595
#10 nsDocument::FlushPendingNotifications (this=<optimized out>, aType=mozilla::FlushType::Layout)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsDocument.cpp:8096
#11 0x00007f1fe93c6f99 in mozilla::dom::Element::GetPrimaryFrame (this=0x60f00018e920,
    aType=mozilla::FlushType::Layout) at /home/bevis/Projects/Build/gecko/src/dom/base/Element.cpp:2176
#12 0x00007f1fe93c9ba4 in mozilla::dom::Element::GetBoundingClientRect (this=0x60f00018e920)
    at /home/bevis/Projects/Build/gecko/src/dom/base/Element.cpp:967
#13 0x00007f1feac1604e in mozilla::dom::ElementBinding::getBoundingClientRect (cx=<optimized out>,
    obj=..., self=0x60f00018e920, args=...)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dom/bindings/ElementBinding.cpp:2086
#14 0x00007f1feb14367c in mozilla::dom::GenericBindingMethod (cx=0x61f000004680, argc=<optimized out>,
    vp=<optimized out>) at /home/bevis/Projects/Build/gecko/src/dom/bindings/BindingUtils.cpp:2954
#15 0x00007f1ff0691b6d in js::CallJSNative (cx=<optimized out>,
    native=0x7f1feb143334 <mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*)>,
    args=...) at /home/bevis/Projects/Build/gecko/src/js/src/jscntxtinlines.h:291
#16 0x00007f1ff066ead7 in js::InternalCallOrConstruct (cx=<optimized out>, args=...,
    construct=js::NO_CONSTRUCT) at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:470
#17 0x00007f1ff064d3c8 in js::CallFromStack (cx=0x61f000004680, args=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:521
#18 Interpret (cx=0x61f000004680, state=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:3025
#19 0x00007f1ff063941b in js::RunScript (cx=<optimized out>, state=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:410
#20 0x00007f1ff066eafa in js::InternalCallOrConstruct (cx=<optimized out>, args=...,
    construct=js::NO_CONSTRUCT) at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:488
#21 0x00007f1ff066fd58 in js::Call (cx=0x6120002899c0, fval=..., thisv=..., args=..., rval=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:534
#22 0x00007f1ff18304d1 in js::Wrapper::call (this=<optimized out>, cx=<optimized out>, proxy=...,
    args=...) at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Wrapper.cpp:165
#23 0x00007f1ff17bdf90 in js::CrossCompartmentWrapper::call (
    this=0x7f1ff66ca8a0 <js::CrossCompartmentWrapper::singleton>, cx=0x61f000004680, wrapper=...,
    args=...) at /home/bevis/Projects/Build/gecko/src/js/src/proxy/CrossCompartmentWrapper.cpp:353
#24 0x00007f1ff18060f5 in js::Proxy::call (cx=<optimized out>, proxy=..., args=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Proxy.cpp:479
#25 0x00007f1ff180c8fd in js::proxy_Call (cx=0x61f000004680, argc=<optimized out>, vp=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Proxy.cpp:731
---Type <return> to continue, or q <return> to quit---
#26 0x00007f1ff0691b6d in js::CallJSNative (cx=<optimized out>,
    native=0x7f1ff180c6b0 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/jscntxtinlines.h:291
#27 0x00007f1ff066eb4e in js::InternalCallOrConstruct (cx=<optimized out>, args=...,
    construct=js::NO_CONSTRUCT) at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:452
#28 0x00007f1ff066fd58 in js::Call (cx=0x6120002899c0, fval=..., thisv=..., args=..., rval=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:534
#29 0x00007f1ff141e1b1 in JS::Call (cx=0x61f000004680, thisv=..., fval=..., args=..., rval=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/jsapi.cpp:2885
#30 0x00007f1feab85189 in mozilla::dom::OnErrorEventHandlerNonNull::Call (this=<optimized out>,
    cx=0x61f000004680, aThisVal=..., event=..., source=..., lineno=..., column=..., error=...,
    aRetVal=..., aRv=...)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dom/bindings/EventHandlerBinding.cpp:398
#31 0x00007f1feb50b1a3 in mozilla::dom::OnErrorEventHandlerNonNull::Call<nsISupports*> (
    this=0x6060006b4320, thisVal=@0x7ffd08e621d0: 0xfffe7f1f966702a0, event=..., source=...,
    lineno=..., column=..., error=..., aRetVal=..., aRv=..., aExecutionReason=<optimized out>,
    aExceptionHandling=<optimized out>, aCompartment=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/mozilla/dom/EventHandlerBinding.h:532
#32 0x00007f1feb4f889e in mozilla::JSEventHandler::HandleEvent (this=<optimized out>,
    aEvent=0x7ffd08e62500) at /home/bevis/Projects/Build/gecko/src/dom/events/JSEventHandler.cpp:166
#33 0x00007f1feb4cbd53 in mozilla::EventListenerManager::HandleEventSubType (this=0x60d0003f02d0,
    aListener=<optimized out>, aDOMEvent=0x60d0003ffc40, aCurrentTarget=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/events/EventListenerManager.cpp:1137
#34 0x00007f1feb4cd402 in mozilla::EventListenerManager::HandleEventInternal (this=<optimized out>,
    aPresContext=0x61a000164a80, aEvent=<optimized out>, aDOMEvent=0x7ffd08e636f0,
    aCurrentTarget=0x619001c38180, aEventStatus=0x7ffd08e636f8)
    at /home/bevis/Projects/Build/gecko/src/dom/events/EventListenerManager.cpp:1311
#35 0x00007f1feb506771 in mozilla::EventTargetChainItem::HandleEvent (this=<optimized out>,
    aVisitor=..., aCd=...) at /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:316
#36 0x00007f1feb4bc68e in mozilla::EventTargetChainItem::HandleEventTargetChain (
    aChain=nsTArray<mozilla::EventTargetChainItem> & = {...}, aVisitor=..., aCallback=<optimized out>,
    aCd=...) at /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:465
#37 0x00007f1feb4bec8b in mozilla::EventDispatcher::Dispatch (aTarget=<optimized out>,
    aPresContext=<optimized out>, aEvent=<optimized out>, aDOMEvent=0x60d0003ffc40,
    aEventStatus=0x7ffd08e63a20, aCallback=<optimized out>, aTargets=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:826
#38 0x00007f1feb4c029d in mozilla::EventDispatcher::DispatchDOMEvent (aTarget=0x619001c381a0,
    aEvent=<optimized out>, aDOMEvent=0x60d0003ffc40, aPresContext=0x61a000164a80,
    aEventStatus=0x7ffd08e63a20)
    at /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:892
#39 0x00007f1fe96d43c0 in ScriptErrorEvent::Run (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsJSEnvironment.cpp:480
#40 0x00007f1fe920ed43 in nsContentUtils::AddScriptRunner (aRunnable=...)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5461
#41 0x00007f1fe920d1bb in nsContentUtils::AddScriptRunner (aRunnable=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5468
#42 0x00007f1fe9482e5e in mozilla::dom::AutoJSAPI::ReportException (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/ScriptSettings.cpp:602
#43 0x00007f1fe9482649 in mozilla::dom::AutoJSAPI::~AutoJSAPI (this=0x7ffd08e64360)
    at /home/bevis/Projects/Build/gecko/src/dom/base/ScriptSettings.cpp:320
#44 0x00007f1fec89128d in nsXBLProtoImplAnonymousMethod::Execute (this=<optimized out>,
    aBoundElement=0x7ffd08e64600, aAddonId=0x7ffd08e64530)
    at /home/bevis/Projects/Build/gecko/src/dom/xbl/nsXBLProtoImplMethod.cpp:326
#45 0x00007f1fec860919 in nsBindingManager::RemovedFromDocumentInternal (this=0x60800013fea0,
    aContent=0x60d0003deb50, aOldDocument=<optimized out>, aDestructorHandling=(unknown: 886384))
    at /home/bevis/Projects/Build/gecko/src/dom/xbl/nsBindingManager.cpp:214
#46 0x00007f1fe93d0bbc in mozilla::dom::RemoveFromBindingManagerRunnable::Run (this=0x60600023d020)
---Type <return> to continue, or q <return> to quit---
    at /home/bevis/Projects/Build/gecko/src/dom/base/Element.cpp:1744
#47 0x00007f1fe920ed43 in nsContentUtils::AddScriptRunner (aRunnable=...)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5461
#48 0x00007f1fe920d1bb in nsContentUtils::AddScriptRunner (aRunnable=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5468
#49 0x00007f1fe93d1384 in mozilla::dom::Element::UnbindFromTree (this=<optimized out>,
    aDeep=<optimized out>, aNullParent=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/Element.cpp:1849
#50 0x00007f1fec9ee762 in nsXULElement::UnbindFromTree (this=<optimized out>,
    aDeep=<error reading variable: access outside bounds of object referenced via synthetic pointer>,
    aNullParent=true) at /home/bevis/Projects/Build/gecko/src/dom/xul/nsXULElement.cpp:888
#51 0x00007f1fe9237a22 in AnonymousContentDestroyer::Run (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5154
#52 0x00007f1fe920e432 in nsContentUtils::RemoveScriptBlocker ()
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5390
#53 0x00007f1fed442379 in nsAutoScriptBlocker::~nsAutoScriptBlocker (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/nsContentUtils.h:3036
#54 nsDocumentViewer::DestroyPresShell (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsDocumentViewer.cpp:4612
#55 0x00007f1fed43b62c in nsDocumentViewer::Hide (this=0x6120002899c0)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsDocumentViewer.cpp:2204
#56 0x00007f1fef720a66 in nsDocShell::SetVisibility (this=<optimized out>,
    aVisibility=<error reading variable: access outside bounds of object referenced via synthetic pointer>) at /home/bevis/Projects/Build/gecko/src/docshell/base/nsDocShell.cpp:6511
#57 0x00007f1fef720ae0 in non-virtual thunk to nsDocShell::SetVisibility(bool) ()
    at /home/bevis/Projects/Build/gecko/src/docshell/base/nsDocShell.cpp:6501
#58 0x00007f1fe962aee5 in nsFrameLoader::Hide (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsFrameLoader.cpp:1312
#59 0x00007f1fed81e04b in nsHideViewer::Run (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/layout/generic/nsSubDocumentFrame.cpp:922
#60 0x00007f1fe920e432 in nsContentUtils::RemoveScriptBlocker ()
    at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5390
#61 0x00007f1fed37742b in nsAutoScriptBlocker::~nsAutoScriptBlocker (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/nsContentUtils.h:3036
#62 mozilla::PresShell::DoFlushPendingNotifications (this=0x61800019c480, aFlush=...)
    at /home/bevis/Projects/Build/gecko/src/layout/base/PresShell.cpp:4206
#63 0x00007f1fed2e75cc in nsIPresShell::FlushPendingNotifications (this=0x61800019c480,
    aType=<error reading variable: access outside bounds of object referenced via synthetic pointer>)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsIPresShell.h:604
#64 nsRefreshDriver::Tick (this=<optimized out>, aNowEpoch=<optimized out>, aNowTime=...)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsRefreshDriver.cpp:1823
#65 0x00007f1fed2f2bda in mozilla::RefreshDriverTimer::TickRefreshDrivers (this=0x6080001538b0,
    aJsNow=1493028724186519, aNow=..., aDrivers=...)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsRefreshDriver.cpp:300
#66 0x00007f1fed2f27f7 in mozilla::RefreshDriverTimer::Tick (this=0x6080001538a0,
    jsnow=1493028724186519, now=...)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsRefreshDriver.cpp:321
#67 0x00007f1fed2f5a29 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers (this=0x6080001538a0,
    aTimeStamp=...) at /home/bevis/Projects/Build/gecko/src/layout/base/nsRefreshDriver.cpp:752
#68 0x00007f1fed2f4e58 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver (this=<optimized out>, aVsyncTimestamp=...)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsRefreshDriver.cpp:665
#69 0x00007f1fed2f0090 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run (this=<optimized out>)
    at /home/bevis/Projects/Build/gecko/src/layout/base/nsRefreshDriver.cpp:512
#70 0x00007f1fe6a90bb4 in nsThread::ProcessNextEvent (this=<optimized out>, aMayWait=<optimized out>,
    aResult=0x7ffd08e66b40) at /home/bevis/Projects/Build/gecko/src/xpcom/threads/nsThread.cpp:1270
#71 0x00007f1fe6a8d7ad in NS_ProcessNextEvent (aThread=0x615000074e80,
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #10)
> Hence, it seems NG to re-enter to ::Show() before Hide() is done caused by
> the nsAutoScriptBlocker in nsDocumentViewer::DestroyPresShell().
> I didn't see this UAF problem anymore with this WIP patch by have
> nsAutoAutoScriptBlocker in Hide() to ensure that all the ScriptRunner will
> be called after PresShell, PresContext, ViewManager, etc are destroyed.
> 
> However, I am not sure if this will break other things.

So far, treeherder result looks fine:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6896f9dcf7818247ab63169ffff0d5705da7b019
Oh, we're reentering Show() from Hide() via DestroyPresShell()?

That's definitely not ok or expected.  Putting a scriptblocker on the stack around DestroPresShell/DestroyPresContext makes sense to me; the comment should probably say something about how we don't want scriptrunners running in our intermediate state between DestroyPresShell starting and us finishing up Hide.

Now if only I understood what the track element had to do with triggering this stuff... ;)
Update comments for why we need nsAutoScriptBlocker in nsDocumentViewer::Hide()
Attachment #8860904 - Attachment is obsolete: true
Update again to not remove the trailing spaces.
Attachment #8861270 - Attachment is obsolete: true
Update our finding about why this crash happened:
1. Show() was re-entered from Hide() via DestroyPresShell().
2. new mPresShell was created in Show() via InitPresentationStuff() with old mViewManager to be reset in Hide().
3. Once a FlushPendingNotifications() request comes again in next tick, this new mPresShell will hit use-after-free on its mViewManager which was actually freed in Hide().

However, we still haven't figure out why "o457.write('<div>');" cause this difference to hit this re-entry problem.
Not sure if comment 7 from :bz explains this but how holding a old parent window causes this re-entry is still a mystery to me. :-\
Flags: sec-bounty?
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #10)
> Created attachment 8860904 [details] [diff] [review]
> (WIP) Supress nsAutoScriptBlocker when destroying PreShell, PresContext, etc
> at Hide()
> 
> I am not really familiar with nsDocumentViewer but, when the symptom happens
> I always git an assertion at nsDocumentViewer::Show() when ::Hide() is
> called:
> http://searchfox.org/mozilla-central/rev/
> 313e5199bf58200f158c6fcbe193e41b88ed58a6/layout/base/nsDocumentViewer.
> cpp#2106
> 
> > NS_ASSERTION(!mPresContext, "Shouldn't have a prescontext if we have no shell!");
> 
> Hence, it seems NG to re-enter to ::Show() before Hide() is done caused by
> the nsAutoScriptBlocker in nsDocumentViewer::DestroyPresShell().
> I didn't see this UAF problem anymore with this WIP patch by have
> nsAutoAutoScriptBlocker in Hide() to ensure that all the ScriptRunner will
> be called after PresShell, PresContext, ViewManager, etc are destroyed.
> 
> However, I am not sure if this will break other things.
> 
> Here is the call stack when reentering ::Show() was hit for reference:
> #0  nsDocumentViewer::Show (this=0x6120002899c0)
>     at
> /home/bevis/Projects/Build/gecko/src/layout/base/nsDocumentViewer.cpp:2050
> #1  0x00007f1fef720a66 in nsDocShell::SetVisibility (this=<optimized out>,
>     aVisibility=<error reading variable: access outside bounds of object
> referenced via synthetic pointer>) at
> /home/bevis/Projects/Build/gecko/src/docshell/base/nsDocShell.cpp:6511
> #2  0x00007f1fef720ae0 in non-virtual thunk to
> nsDocShell::SetVisibility(bool) ()
>     at /home/bevis/Projects/Build/gecko/src/docshell/base/nsDocShell.cpp:6501
> #3  0x00007f1fe962a070 in nsFrameLoader::Show (this=<optimized out>,
> marginWidth=1634768,
>     marginHeight=149281760, scrollbarPrefX=149281792,
> scrollbarPrefY=-1591952512, frame=0x60c0008bd2c0)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/nsFrameLoader.cpp:1140
> #4  0x00007f1fed7b271b in nsSubDocumentFrame::ShowViewer (this=<optimized
> out>)
>     at
> /home/bevis/Projects/Build/gecko/src/layout/generic/nsSubDocumentFrame.cpp:
> 185
> #5  0x00007f1fed81dd1d in AsyncFrameInit::Run (this=0x6040005fad90)
>     at
> /home/bevis/Projects/Build/gecko/src/layout/generic/nsSubDocumentFrame.cpp:92
> #6  0x00007f1fe920e432 in nsContentUtils::RemoveScriptBlocker ()
>     at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5390
> #7  0x00007f1fed37742b in nsAutoScriptBlocker::~nsAutoScriptBlocker
> (this=<optimized out>)
>     at
> /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/
> nsContentUtils.h:3036
> #8  mozilla::PresShell::DoFlushPendingNotifications (this=0x61800019c480,
> aFlush=...)
>     at /home/bevis/Projects/Build/gecko/src/layout/base/PresShell.cpp:4206
> #9  0x00007f1fe95cab37 in nsIPresShell::FlushPendingNotifications
> (this=0x61800019c480,
>     aType=mozilla::FlushType::Layout)
>     at
> /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/
> nsIPresShell.h:595
> #10 nsDocument::FlushPendingNotifications (this=<optimized out>,
> aType=mozilla::FlushType::Layout)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/nsDocument.cpp:8096
> #11 0x00007f1fe93c6f99 in mozilla::dom::Element::GetPrimaryFrame
> (this=0x60f00018e920,
>     aType=mozilla::FlushType::Layout) at
> /home/bevis/Projects/Build/gecko/src/dom/base/Element.cpp:2176
> #12 0x00007f1fe93c9ba4 in mozilla::dom::Element::GetBoundingClientRect
> (this=0x60f00018e920)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/Element.cpp:967
> #13 0x00007f1feac1604e in
> mozilla::dom::ElementBinding::getBoundingClientRect (cx=<optimized out>,
>     obj=..., self=0x60f00018e920, args=...)
>     at
> /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dom/bindings/
> ElementBinding.cpp:2086
> #14 0x00007f1feb14367c in mozilla::dom::GenericBindingMethod
> (cx=0x61f000004680, argc=<optimized out>,
>     vp=<optimized out>) at
> /home/bevis/Projects/Build/gecko/src/dom/bindings/BindingUtils.cpp:2954
> #15 0x00007f1ff0691b6d in js::CallJSNative (cx=<optimized out>,
>     native=0x7f1feb143334 <mozilla::dom::GenericBindingMethod(JSContext*,
> unsigned int, JS::Value*)>,
>     args=...) at
> /home/bevis/Projects/Build/gecko/src/js/src/jscntxtinlines.h:291
> #16 0x00007f1ff066ead7 in js::InternalCallOrConstruct (cx=<optimized out>,
> args=...,
>     construct=js::NO_CONSTRUCT) at
> /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:470
> #17 0x00007f1ff064d3c8 in js::CallFromStack (cx=0x61f000004680, args=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:521
> #18 Interpret (cx=0x61f000004680, state=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:3025
> #19 0x00007f1ff063941b in js::RunScript (cx=<optimized out>, state=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:410
> #20 0x00007f1ff066eafa in js::InternalCallOrConstruct (cx=<optimized out>,
> args=...,
>     construct=js::NO_CONSTRUCT) at
> /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:488
> #21 0x00007f1ff066fd58 in js::Call (cx=0x6120002899c0, fval=..., thisv=...,
> args=..., rval=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:534
> #22 0x00007f1ff18304d1 in js::Wrapper::call (this=<optimized out>,
> cx=<optimized out>, proxy=...,
>     args=...) at
> /home/bevis/Projects/Build/gecko/src/js/src/proxy/Wrapper.cpp:165
> #23 0x00007f1ff17bdf90 in js::CrossCompartmentWrapper::call (
>     this=0x7f1ff66ca8a0 <js::CrossCompartmentWrapper::singleton>,
> cx=0x61f000004680, wrapper=...,
>     args=...) at
> /home/bevis/Projects/Build/gecko/src/js/src/proxy/CrossCompartmentWrapper.
> cpp:353
> #24 0x00007f1ff18060f5 in js::Proxy::call (cx=<optimized out>, proxy=...,
> args=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Proxy.cpp:479
> #25 0x00007f1ff180c8fd in js::proxy_Call (cx=0x61f000004680, argc=<optimized
> out>, vp=<optimized out>)
>     at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Proxy.cpp:731
> ---Type <return> to continue, or q <return> to quit---
> #26 0x00007f1ff0691b6d in js::CallJSNative (cx=<optimized out>,
>     native=0x7f1ff180c6b0 <js::proxy_Call(JSContext*, unsigned int,
> JS::Value*)>, args=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/jscntxtinlines.h:291
> #27 0x00007f1ff066eb4e in js::InternalCallOrConstruct (cx=<optimized out>,
> args=...,
>     construct=js::NO_CONSTRUCT) at
> /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:452
> #28 0x00007f1ff066fd58 in js::Call (cx=0x6120002899c0, fval=..., thisv=...,
> args=..., rval=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/vm/Interpreter.cpp:534
> #29 0x00007f1ff141e1b1 in JS::Call (cx=0x61f000004680, thisv=..., fval=...,
> args=..., rval=...)
>     at /home/bevis/Projects/Build/gecko/src/js/src/jsapi.cpp:2885
> #30 0x00007f1feab85189 in mozilla::dom::OnErrorEventHandlerNonNull::Call
> (this=<optimized out>,
>     cx=0x61f000004680, aThisVal=..., event=..., source=..., lineno=...,
> column=..., error=...,
>     aRetVal=..., aRv=...)
>     at
> /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dom/bindings/
> EventHandlerBinding.cpp:398
> #31 0x00007f1feb50b1a3 in
> mozilla::dom::OnErrorEventHandlerNonNull::Call<nsISupports*> (
>     this=0x6060006b4320, thisVal=@0x7ffd08e621d0: 0xfffe7f1f966702a0,
> event=..., source=...,
>     lineno=..., column=..., error=..., aRetVal=..., aRv=...,
> aExecutionReason=<optimized out>,
>     aExceptionHandling=<optimized out>, aCompartment=<optimized out>)
>     at
> /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/include/mozilla/dom/
> EventHandlerBinding.h:532
> #32 0x00007f1feb4f889e in mozilla::JSEventHandler::HandleEvent
> (this=<optimized out>,
>     aEvent=0x7ffd08e62500) at
> /home/bevis/Projects/Build/gecko/src/dom/events/JSEventHandler.cpp:166
> #33 0x00007f1feb4cbd53 in mozilla::EventListenerManager::HandleEventSubType
> (this=0x60d0003f02d0,
>     aListener=<optimized out>, aDOMEvent=0x60d0003ffc40,
> aCurrentTarget=<optimized out>)
>     at
> /home/bevis/Projects/Build/gecko/src/dom/events/EventListenerManager.cpp:1137
> #34 0x00007f1feb4cd402 in mozilla::EventListenerManager::HandleEventInternal
> (this=<optimized out>,
>     aPresContext=0x61a000164a80, aEvent=<optimized out>,
> aDOMEvent=0x7ffd08e636f0,
>     aCurrentTarget=0x619001c38180, aEventStatus=0x7ffd08e636f8)
>     at
> /home/bevis/Projects/Build/gecko/src/dom/events/EventListenerManager.cpp:1311
> #35 0x00007f1feb506771 in mozilla::EventTargetChainItem::HandleEvent
> (this=<optimized out>,
>     aVisitor=..., aCd=...) at
> /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:316
> #36 0x00007f1feb4bc68e in
> mozilla::EventTargetChainItem::HandleEventTargetChain (
>     aChain=nsTArray<mozilla::EventTargetChainItem> & = {...}, aVisitor=...,
> aCallback=<optimized out>,
>     aCd=...) at
> /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:465
> #37 0x00007f1feb4bec8b in mozilla::EventDispatcher::Dispatch
> (aTarget=<optimized out>,
>     aPresContext=<optimized out>, aEvent=<optimized out>,
> aDOMEvent=0x60d0003ffc40,
>     aEventStatus=0x7ffd08e63a20, aCallback=<optimized out>,
> aTargets=<optimized out>)
>     at
> /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:826
> #38 0x00007f1feb4c029d in mozilla::EventDispatcher::DispatchDOMEvent
> (aTarget=0x619001c381a0,
>     aEvent=<optimized out>, aDOMEvent=0x60d0003ffc40,
> aPresContext=0x61a000164a80,
>     aEventStatus=0x7ffd08e63a20)
>     at
> /home/bevis/Projects/Build/gecko/src/dom/events/EventDispatcher.cpp:892
> #39 0x00007f1fe96d43c0 in ScriptErrorEvent::Run (this=<optimized out>)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/nsJSEnvironment.cpp:480
> #40 0x00007f1fe920ed43 in nsContentUtils::AddScriptRunner (aRunnable=...)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5461
> #41 0x00007f1fe920d1bb in nsContentUtils::AddScriptRunner
> (aRunnable=<optimized out>)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/nsContentUtils.cpp:5468
> #42 0x00007f1fe9482e5e in mozilla::dom::AutoJSAPI::ReportException
> (this=<optimized out>)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/ScriptSettings.cpp:602
> #43 0x00007f1fe9482649 in mozilla::dom::AutoJSAPI::~AutoJSAPI
> (this=0x7ffd08e64360)
>     at /home/bevis/Projects/Build/gecko/src/dom/base/ScriptSettings.cpp:320
> #44 0x00007f1fec89128d in nsXBLProtoImplAnonymousMethod::Execute
> (this=<optimized out>,
>     aBoundElement=0x7ffd08e64600, aAddonId=0x7ffd08e64530)
>     at
> /home/bevis/Projects/Build/gecko/src/dom/xbl/nsXBLProtoImplMethod.cpp:326

Call stack 44~13, Is that mapping to the getBoundingClientRect in func3(), "o457.defaultView.onerror=fun3;"?
If so, maybe something happened between o457.defaultView and TrackElement?
Flags: needinfo?(bechen) → needinfo?(btseng)
Remove o457.defaultView.onerror=func3, no crash.
It makes sense to have one more nsAutoScriptBlocker at Hide() to fix the reentry found in comment 15.
Take this bug to clarify how this reentry was triggered before requesting formal review of the patch in comment 14.
Assignee: nobody → btseng
Flags: needinfo?(btseng)
After further testing, I found that the reason why we hit this reentry are that
1. nsDocumentViewer::Hide() was called after "document.documentElement.hidden^=true;" in fun2() was executed.
   However, Error was thrown from videocontrols.xml:
> JavaScript error: chrome://global/content/bindings/videocontrols.xml, line 728: TypeError: can't access dead object 
   Triggered from the frames of the call stack in comment 10:
   #55 0x00007f1fed43b62c in nsDocumentViewer::Hide
   #54 nsDocumentViewer::DestroyPresShell (this=<optimized out>)
   ...
   #39 0x00007f1fe96d43c0 in ScriptErrorEvent::Run
   ...
   #32 0x00007f1feb4f889e in mozilla::JSEventHandler::HandleEvent
2. nsDocumentViewer::Show() was called at the same stack after "document.documentElement.getBoundingClientRect()" in 
   fun3() was executed from the o457.defaultView.onerror callback triggered by step 1 according to the frames in the call 
   stack:
   #32 0x00007f1feb4f889e in mozilla::JSEventHandler::HandleEvent 
   ...
   #13 0x00007f1feac1604e in mozilla::dom::ElementBinding::getBoundingClientRect
   ...
   #0  nsDocumentViewer::Show (this=0x6120002899c0)

:bz does this explain why this reentry was triggered by this track element?

I haven't figured out why the TypeError was reported from videocontrols.xml but it's the reason why nsDocumentViewer::Show() was called before nsDocumentViewer::Hide() was done.

Do you know why this TypeError was reported in step 1?
Flags: needinfo?(bzbarsky)
update callstack from JS and native for reference when TypeError was reported:
(rr) js
0 terminateEventListeners() ["chrome://global/content/bindings/videocontrols.xml":728]
    this = [object Object]
1 videoControls_XBL_Destructor() ["chrome://global/content/bindings/videocontrols.xml":1887]
    this = [object XULElement]

(rr) bt
#0  ReportDead (cx=0x61f000004680)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/DeadObjectProxy.cpp:20
#1  0x00007fd268fef095 in js::DeadObjectProxy<(js::DeadProxyIsCallableIsConstructorOption)0>::getOwnPropertyDescriptor (
    this=0x7fd270d43a00 <js::DeadObjectProxy<(js::DeadProxyIsCallableIsConstructorOption)0>::singleton()::singleton>, cx=0x61f000004680, wrapper=..., id=..., desc=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/DeadObjectProxy.cpp:28
#2  0x00007fd268f4fc46 in js::BaseProxyHandler::get (
    this=0x7fd270d43a00 <js::DeadObjectProxy<(js::DeadProxyIsCallableIsConstructorOption)0>::singleton()::singleton>, cx=0x61f000004680, proxy=..., receiver=..., id=..., vp=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/BaseProxyHandler.cpp:102
#3  0x00007fd268fe8bf2 in js::Proxy::get (cx=0x61f000004680, proxy=..., receiver_=..., id=..., vp=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Proxy.cpp:340
#4  0x00007fd268fb6a49 in js::ProxyGetProperty (cx=0x61f000004680, proxy=..., id=..., vp=...)
    at /home/bevis/Projects/Build/gecko/src/js/src/proxy/Proxy.cpp:347
#5  0x0000254228d4cce6 in ?? ()
#6  0x00007fd23556d520 in ?? ()
#7  0x00007ffe931e5028 in ?? ()
#8  0x00007ffe931e50d8 in ?? ()
#9  0xfff9000000000000 in ?? ()
#10 0x00007fd270f5e040 in CallNativeGetterInfo ()
   from /home/bevis/Projects/Build/gecko/src/objdir-ff-asan/dist/bin/libxul.so
#11 0x00007fd24927d100 in ?? ()
#12 0x00002542292b7ab2 in ?? ()
#13 0x0000000000002022 in ?? ()
#14 0x00007fd23557d580 in ?? ()
#15 0x00007fd2492aab28 in ?? ()
#16 0x00007ffe931e50d8 in ?? ()
#17 0x0000621001768b40 in ?? ()
#18 0x00002542292caacc in ?? ()
#19 0x0000000000006021 in ?? ()
#20 0xfffe7fd23557d580 in ?? ()
#21 0xfff9000000000000 in ?? ()
#22 0xfffe7fd249301530 in ?? ()
#23 0xfffe7fd243817980 in ?? ()
#24 0xfffe7fd243843220 in ?? ()
#25 0x0000000000000000 in ?? ()
OK, now things make sense, more or less.  I wish the stacks in this bug had been attachments, not pasted in, fwiw; it would make it a lot easier to not miss things...

So the sequence of events is:

1)  Create track element.  It creates a track; that track is bound to the window.
2)  document.open creates a new window.  Track is still bound to the old window.
3)  Destroy the presshell, by setting "display:none" on an ancestor for the
    <iframe> element.  This tears down the frametree and destroys the native anonymous
    content bound to those frames. We unhook the XBL bindings from that anonymous
    content and run their destructors.
4)  The <destructor> in videocontrols.xml does this.Utils.terminateEventListeners(), which does:

        for (let element of this.controlListeners) {
          element.item.removeEventListener(element.event, element.func,
            { mozSystemGroup: true });
        }
5)  One of the controlListeners's .item is presumably still bound to the old window,
    but hueyfix has kicked in, and since the old window is not current we replaced it
    with a dead object wrapper.  I can't tell whether this listener is the track
    created in step 1, or the TextTrackList that is returned from
    this.video.textTracks in the init() method in videocontrols.xml.
    I would expect this.video.textTracks to always be in the list and hence
    always cause this exception in this testcase, even if there are no tracks.
    Anyway, I didn't sort through the exact events here.  So it's still not 100% clear
    why the track element is involved, but at least it's plausible that it is now.
    We should probably add try/catch to this code around all its access to content bits;
    I filed bug 1359849 on this.
6)  The XBL <destructor> throws an exception, due to the dead object it encounters.
7)  The exception is reported to the web page via an error event.  This is broken, imo.
    I filed bug 1359859 on this and will post a patch.
8)  As a result, the web page gets to run script as part of presshell destruction,
    and that script toggles things back to not being "display:none" and flushes layout.
    This triggers construction of a new presshell in the subframe.

Since we really don't want to be running script, XBL or otherwise, until we have finished our destruction steps, using a scriptblocker around presshell teardown and the following bits to prevent step 4 from happening while we're in an inconsistent state, is exactly the right thing to do here.  Fixing bug 1359859 will also help, by not running the error handler here.  But it's safer to not run any script there at all, because an XBL destructor could accidentally trigger layout side-effects anyway.
Flags: needinfo?(bzbarsky)
And thank you very much for tracking this down!
Group: core-security → layout-core-security
Revise commit messages and comments.

May I have your review for this change?
(Thanks for further explanation and breaking down of the root cause!)
Attachment #8861342 - Attachment is obsolete: true
Attachment #8862749 - Flags: review?(bzbarsky)
Comment on attachment 8862749 [details] [diff] [review]
(v2) Do not clean up ScriptRunner in the intermediate state when destroying PreShell, PresContext, etc at Hide().

s/clean up/run/, and probably s/Hide()/nsDocumentViewer::Hide()/ in the first line.

For the rest of the commit message, maybe something like this:

  We do not want to allow script execution to run in the middle
  of our cleanup, when we have nulled out some of our pointers
  to PresShell, PresContext, ViewManager, etc, but not all of them.

r=me
Attachment #8862749 - Flags: review?(bzbarsky) → review+
Couldn't we have the same problem in any function in nsDocumentViewer that calls DestroyPresShell? Seems like they would all get confused by re-entrancy and we should put a scriptblocker everywhere DestroyPresShell is called (and audit all those callers too).
And PresShell:Initialize will also exit a script blocker, so this call in nsDocumentViewer::InitPresentationStuff

https://dxr.mozilla.org/mozilla-central/rev/2fe636103d7167f3a5d57f61bd19fddcc878ca3c/layout/base/nsDocumentViewer.cpp#737

could also re-enter into nsDocumentViewer functions.
> Couldn't we have the same problem in any function in nsDocumentViewer that calls DestroyPresShell?

Hmm.  Yes, we could.  We should probably add an assert to DestroyPresShell that it's running under a scriptblocker, and fix all the callsites...  And remove the nsAutoScriptBlocker in DestroyPresShell itself.

And yes, we should do something similar in InitPresentationStuff.  Good catch!
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
This is 100% reproducible with the script in comment 0.

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?
FF52

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Yes, approval‑mozilla‑beta, approval‑mozilla‑esr52 will be requested in this comment below.

How likely is this patch to cause regressions; how much testing does it need?
The risk is low because we try to postpone the execution of the scripts to a stable state which makes more sense to the expected behavior.
Test has been done locally.
And the treeherder result looks fine:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4021fe20e7f26a08e5bf8c465dab9ebf4d9eb17

Approval Request Comment
[Feature/Bug causing the regression]: Bug 871747
[User impact if declined]: Potential security problem.
[Is this code covered by automated tests?]: 
No, but it was tested manually and the treeherder result looks fine without introducing other regression:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4021fe20e7f26a08e5bf8c465dab9ebf4d9eb17
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]:No
[Why is the change risky/not risky?]:
The risk is low because we try to postpone the execution of the scripts to a stable state which makes more sense to the expected behavior.
[String changes made/needed]:No.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: This is a sec-high bug
Fix Landed on Version: FF55
Risk to taking this patch (and alternatives if risky): The risk is low because we try to postpone the execution of the scripts to a stable state which makes more sense to the expected behavior.
String or UUID changes made by this patch: No.
Attachment #8862749 - Attachment is obsolete: true
Attachment #8863995 - Flags: sec-approval?
Attachment #8863995 - Flags: review+
Attachment #8863995 - Flags: approval-mozilla-esr52?
Attachment #8863995 - Flags: approval-mozilla-beta?
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #28)
> Created attachment 8863995 [details] [diff] [review]
> (v3) Do not run ScriptRunner in the intermediate state when destroying
> PreShell, PresContext, etc at nsDocumentViewer::Hide(). r=bz
> 
> [Security approval request comment]
> How easily could an exploit be constructed based on the patch?
> This is 100% reproducible with the script in comment 0.

The question here isn't how reproducible the bug is. The question is "how easily could an exploit be constructed based on the PATCH?"

The concern here that we're trying to understand is "Will this patch 0day us as soon as you check it in because the security problem it fixes is completely obvious and easy to exploit once someone reads the patch?" We gate when we do checkins to reduce the risk issue and we partially determine that based on how this question is answered. 

So, does the patch make this obvious? Will someone seeing our quite public checkin of the fix have enough information to make an exploit and go pwn people until Firefox 54 is released?

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

This comment in the patch seems a little dangerous looking:

    // Flushing could be triggered by ScriptRunners during DestroyPresShell()
    // which causes further reentry to Show() that we don't expect in this
    // intermediate state between DestroyPresShell() starting and finishing up
(In reply to Al Billings [:abillings] from comment #29)
> So, does the patch make this obvious? Will someone seeing our quite public
> checkin of the fix have enough information to make an exploit and go pwn
> people until Firefox 54 is released?
> 
Thanks for guarding this.

The fix itself doesn't but the "reentry to Show()" in commit message and the comment in nsDocumentViewer.cpp implies that a JS call to getBoundingClientRect() during an uncaught exception from "document.documentElement.hidden^=true;" could be a problem.

> > Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
> 
> This comment in the patch seems a little dangerous looking:
> 
>     // Flushing could be triggered by ScriptRunners during DestroyPresShell()
>     // which causes further reentry to Show() that we don't expect in this
>     // intermediate state between DestroyPresShell() starting and finishing up

m.. both commit message and the comment in nsDocumentViewer.cpp explain the problem to be fixed.

Hi Boris,

Should I revise the commit message and comment like this instead without telling anything about the reentry to nsDocumentViewer::Show()?

"
Do not run ScriptRunners risen from DestroyPresShell() in the intermediate state 
of destroying PresShell, PresContext, ViewManager, etc.
"
Flags: needinfo?(bzbarsky)
Yeah, we can do that.  So something like:

  Do not run ScriptRunners queued by DestroyPresShell() in the intermediate state
  before we're done destroying PresShell, PresContext, ViewManager, etc.

Also, please note comment 26.  That needs a followup bug, or better yet fixing all of those places at once in a single commit, because once we land this people will start looking at other similar problems.
Flags: needinfo?(bzbarsky)
See Also: → 1362924
Remove the "reentry to nsDocumentViewer::Show()" from the comment and the commit message to lower the security concern about how this UAF could be reproduced.
Attachment #8863995 - Attachment is obsolete: true
Attachment #8863995 - Flags: sec-approval?
Attachment #8863995 - Flags: approval-mozilla-esr52?
Attachment #8863995 - Flags: approval-mozilla-beta?
Attachment #8865332 - Flags: review+
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #31)
> Also, please note comment 26.  That needs a followup bug, or better yet
> fixing all of those places at once in a single commit, because once we land
> this people will start looking at other similar problems.

Bug 1362924 was filed to address comment 26.
If the patch in comment 32 is still too sensitive, we'll have to land the patch of bug 1362924 and this one at the same time.
Comment on attachment 8865332 [details] [diff] [review]
(v4) Do not run ScriptRunner in the intermediate state before destroying PreShell, PresContext, etc at nsDocumentViewer::Hide() is done. r=bz

[Security approval request comment]
Q: How easily could an exploit be constructed based on the patch?
A: It's not easy to exploit from the fix.
   The only concern is in the comment and the the commit message of the 
   previous patch which has been revised to not tell anything about the 
   reentry problem from ::Hide() to ::Show().

Q: Do comments in the patch, the check-in comment, or tests included in
   the patch paint a bulls-eye on the security problem?
A: The reentry is the root cause of this problem but is not mentioned in 
   the commit message and comments in the source file.

Q: Which older supported branches are affected by this flaw?
A: FF52

Q: If not all supported branches, which bug introduced the flaw?
A: bug 871747

Q: Do you have backports for the affected branches? 
   If not, how different, hard to create, and risky will they be?
A: Yes, approval‑mozilla‑beta, approval‑mozilla‑esr52 will be requested
   later once this sec-approval is granted.

Q: How likely is this patch to cause regressions; how much testing does it need?
A: The risk to cause regressions is low because we try to postpone the 
   execution of the scripts to a stable state which makes more sense to 
   the expected behavior. So far, the test in comment 0 is the only 
   known use case that can cause this problem and has been tested locally.
   Besides, the treeherder result looks fine:
   https://treeherder.mozilla.org/#/jobs?repo=try&revision=8541dac514bbf88d7d8750ba6ae58b686254733d
Attachment #8865332 - Flags: sec-approval?
Blocks: 871747
Keywords: regression, testcase
Comment on attachment 8865332 [details] [diff] [review]
(v4) Do not run ScriptRunner in the intermediate state before destroying PreShell, PresContext, etc at nsDocumentViewer::Hide() is done. r=bz

sec-approval=dveditz
Attachment #8865332 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/mozilla-central/rev/5743fb35b0e0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Beta and ESR52 approval on this when you get a chance.
Flags: needinfo?(btseng)
Comment on attachment 8865332 [details] [diff] [review]
(v4) Do not run ScriptRunner in the intermediate state before destroying PreShell, PresContext, etc at nsDocumentViewer::Hide() is done. r=bz

[Beta Approval Request Comment]
[Feature/Bug causing the regression]: 
  Bug 871747
[User impact if declined]: 
  Potential security problem.
[Is this code covered by automated tests?]: 
  No, but it was tested manually and the treeherder result looks fine without
  introducing other regression:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4021fe20e7f26a08e5bf8c465dab9ebf4d9eb17
[Has the fix been verified in Nightly?]: 
  Yes.
[Needs manual test from QE? If yes, steps to reproduce]: 
  No.
[List of other uplifts needed for the feature/fix]:
  No.
[Is the change risky?]:
  No.
[Why is the change risky/not risky?]:
  The risk is low because we try to postpone the execution of the scripts to a 
  stable state which makes more sense to the expected behavior.
[String changes made/needed]:
  No.

[ESR52 Approval Request Comment]
Q: If this is not a sec:{high,crit} bug, please state case for ESR consideration: 
A: It's a sec-high bug.
Q: User impact if declined:
A: Potential security problem.
Q: Fix Landed on Version: 
A: FF55
Q: Risk to taking this patch (and alternatives if risky): 
A: The risk is low because we try to postpone the execution of the scripts to a 
   stable state which makes more sense to the expected behavior.
Q: String or UUID changes made by this patch:
A: No.
Flags: needinfo?(btseng)
Attachment #8865332 - Flags: approval-mozilla-esr52?
Attachment #8865332 - Flags: approval-mozilla-beta?
Group: layout-core-security → core-security-release
Comment on attachment 8865332 [details] [diff] [review]
(v4) Do not run ScriptRunner in the intermediate state before destroying PreShell, PresContext, etc at nsDocumentViewer::Hide() is done. r=bz

Fix a sec-high. Beta54+ & ESR52+. Should be in 54 beta 7.
Attachment #8865332 - Flags: approval-mozilla-esr52?
Attachment #8865332 - Flags: approval-mozilla-esr52+
Attachment #8865332 - Flags: approval-mozilla-beta?
Attachment #8865332 - Flags: approval-mozilla-beta+
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #38)
> [Is this code covered by automated tests?]: 
>   No, but it was tested manually and the treeherder result looks fine without
>   introducing other regression:
>  
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=f4021fe20e7f26a08e5bf8c465dab9ebf4d9eb17
> [Has the fix been verified in Nightly?]: 
>   Yes.
> [Needs manual test from QE? If yes, steps to reproduce]: 
>   No.

Setting qe-verify- based on Bevis' assessment on manual testing needs.
Flags: qe-verify-
Flags: sec-bounty? → sec-bounty+
Whiteboard: [adv-main54+][adv-esr52.2+]
Alias: CVE-2017-7750
Group: core-security-release
Reading the bug the videocontrols no longer throw when toggling hidden, so the crashtest never arrives to fun3...

It's a little wild that window.onerror would catch that exception isn't it?

In my local testing I didn't manage to make it catch exceptions thrown _from_ XBL though. Maybe it needs to be some particular uncatchable exception?

Anyway, that doesn't seem expected. Boris, does that seem plausible? Is it worth a new bug?

In any case, seems hard to turn this into a valuable crashtest unfortunately, it definitely doesn't trigger the same code paths anymore :(
Flags: needinfo?(emilio)
Err, Boris, see the question in comment 44. Not sure if it's expected, nor I have managed to repro something similar, but the original exception was "can't access dead object", so not sure if those are magic in some way.
Flags: needinfo?(bzbarsky)
The "can't access dead object" exception depends on whether hueyfix runs and whether we hueyfix the particular edge involved.

> It's a little wild that window.onerror would catch that exception isn't it?

I'm a bit surprised that would happen too.  I'd expect the global in this case to be the XBL global and hence that we'd never reach DispatchScriptErrorEvent (because we have no "inner" in AutoJSAPI::ReportException).

I have no idea why it would have worked any other way, offhand....  We did use to have the addon scope special-case that we removed in bug 1445551, but that would not have been used for XBL scopes.

I guess we could try bisecting when the test started not firing the window onerror if we really want to.
Flags: needinfo?(bzbarsky)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: