Closed Bug 1302973 (CVE-2016-9068) Opened 8 years ago Closed 8 years ago

heap-use-after-free in nsRefreshDriver::Tick

Categories

(Core :: DOM: Animation, defect, P1)

Product:
Core
Component:
DOM: Animation
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox49 --- wontfix
firefox-esr45 --- unaffected
firefox50 + verified
firefox51 + verified
firefox52 + verified

People

(Reporter: nils, Assigned: birtles)

References

Details

(4 keywords, Whiteboard: [adv-main50+])

Attachments

(5 files, 8 obsolete files)

crash.html (testcase, crashes firefox)
671 bytes, text/html
Details
Crash test
1.53 KB, patch
Details | Diff | Splinter Review
Fix for trunk: Store timelines on document; r=smaug
10.30 KB, patch
birtles
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
Fix rebased for aurora/beta
10.30 KB, patch
ritu
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
Fix rebased for release
10.25 KB, patch
lizzard
: approval-mozilla-release-
Details | Diff | Splinter Review 
The following testcase crashes the latest ASAN build of Firefox (BuildID=20160914134208) as follows. The testcase requires the fuzzPriv extension.

crash.html:
<script>
function start() {
        o0=window.document;
        o1128=document.createElement('iframe');
        try{while(document.removeChild(document.firstChild));}catch(e){}
        o1479=document.implementation.createHTMLDocument();
        o1479.body.appendChild(o1128);
        document.appendChild(o1479.documentElement);
        o1638=frames[0];
        o1639=o1638.document;
        o1773=o1639.createElement('meta');
        o1925=o1773.animate([{listStyleType: 'upper-alpha'},{listStyleType: 'decimal'}],136);
        o1639.write('');
        o1925.play();
        o0.write('');
        o1925=null; o1639=null; o1773=null; o1638=null; o1479=null;
        fuzzPriv.forceGC();fuzzPriv.CC();fuzzPriv.forceGC();fuzzPriv.CC();
}
</script>
<body onload="start()"></body>

ASAN output:
=================================================================
==16324==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000053ef8 at pc 0x7fb55609e7ce bp 0x7ffef123ed90 sp 0x7ffef123ed88
READ of size 8 at 0x60e000053ef8 thread T0 (Web Content)
    #0 0x7fb55609e7cd in AddRef /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:37:5
    #1 0x7fb55609e7cd in AddRef /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:384
    #2 0x7fb55609e7cd in RefPtr /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:111
    #3 0x7fb55609e7cd in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1700
    #4 0x7fb5560a820c in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:247:7
    #5 0x7fb5560a7ed9 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:266:5
    #6 0x7fb5560a99c4 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:426:9
    #7 0x7fb556a07684 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /home/worker/workspace/build/src/layout/ipc/VsyncChild.cpp:64:5
    #8 0x7fb5505a3b8a in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:234:20
    #9 0x7fb55008e63f in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1551:16
    #10 0x7fb54ffcded9 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1662:14
    #11 0x7fb54ffcada6 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1600:17
    #12 0x7fb54ffb8d67 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1567:5
    #13 0x7fb54ffe71a2 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:729:12
    #14 0x7fb54ffe71a2 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:735
    #15 0x7fb54ffe71a2 in mozilla::detail::RunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:764
    #16 0x7fb54ffe6a2f in Run /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:545:22
    #17 0x7fb54ffe6a2f in mozilla::ipc::MessageChannel::DequeueTask::Run() /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:564
    #18 0x7fb54f21272d in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1059:7
    #19 0x7fb54f290fac in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #20 0x7fb54ffd511f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #21 0x7fb54ff48398 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #22 0x7fb54ff48398 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:225
    #23 0x7fb54ff48398 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:205
    #24 0x7fb5559f7e0f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3
    #25 0x7fb557b3ae97 in XRE_RunAppShell /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:875:12
    #26 0x7fb54ff48398 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #27 0x7fb54ff48398 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:225
    #28 0x7fb54ff48398 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:205
    #29 0x7fb557b3a3bd in XRE_InitChildProcess /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:705:7
    #30 0x4dfb2b in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:197:19
    #31 0x4dfb2b in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:367
    #32 0x7fb56a76e82f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291
    #33 0x41ba08 in _start (/home/nils/fuzzer3/firefox/firefox+0x41ba08)

0x60e000053ef8 is located 120 bytes inside of 160-byte region [0x60e000053e80,0x60e000053f20)
freed by thread T0 (Web Content) here:
    #0 0x4b215b in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3
    #1 0x7fb54f0ee084 in SnowWhiteKiller::~SnowWhiteKiller() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2681:9
    #2 0x7fb54f0edc76 in nsCycleCollector::FreeSnowWhite(bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2855:3
    #3 0x7fb54f0f449c in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3832:3
    #4 0x7fb54f0f3c7c in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3657:9
    #5 0x7fb54f0f6e56 in nsCycleCollector_collect(nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4148:3
    #6 0x7fb551f96ed9 in nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1440:3
    #7 0x7fb551b1581d in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsDOMWindowUtils.cpp:1338:3
    #8 0x7fb54f238486 in NS_InvokeByIndex /home/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:180:23
    #9 0x7fb550b603ae in Invoke /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:2065:12
    #10 0x7fb550b603ae in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1384
    #11 0x7fb550b603ae in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1351
    #12 0x7fb550b67acb in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1143:12
    #13 0x7fb559d9761c in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:235:15
    #14 0x7fb559d9761c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442
    #15 0x7fb559d773a0 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:505:12
    #16 0x7fb559d773a0 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2918
    #17 0x7fb559d5c93b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:400:12
    #18 0x7fb559d97e34 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:472:15
    #19 0x7fb559d988a1 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:518:10
    #20 0x7fb5598944da in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2776:12
    #21 0x7fb550a9b1af in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:353:18
    #22 0x7fb559d9761c in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:235:15
    #23 0x7fb559d9761c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442
    #24 0x7fb559d773a0 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:505:12
    #25 0x7fb559d773a0 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2918
    #26 0x7fb559d5c93b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:400:12
    #27 0x7fb559d97e34 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:472:15
    #28 0x7fb559d988a1 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:518:10
    #29 0x7fb559896a38 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:2835:12
    #30 0x7fb5537bf878 in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:259:37
    #31 0x7fb5540db041 in Call<nsISupports *> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:361:12
    #32 0x7fb5540db041 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /home/worker/workspace/build/src/dom/events/JSEventHandler.cpp:214
    #33 0x7fb5540a7994 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1133:16
    #34 0x7fb5540a94c1 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1286:17
    #35 0x7fb5540927b2 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:380:5
    #36 0x7fb554096c9d in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:711:9

previously allocated by thread T0 (Web Content) here:
    #0 0x4b247b in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3
    #1 0x4e0bcd in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7fb551e8afc4 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12
    #3 0x7fb551e8afc4 in nsDocument::Timeline() /home/worker/workspace/build/src/dom/base/nsDocument.cpp:3023
    #4 0x7fb556360547 in PresShell::Init(nsIDocument*, nsPresContext*, nsViewManager*, mozilla::StyleSetHandle) /home/worker/workspace/build/src/layout/base/nsPresShell.cpp:972:3
    #5 0x7fb551e9229e in nsDocument::doCreateShell(nsPresContext*, nsViewManager*, mozilla::StyleSetHandle) /home/worker/workspace/build/src/dom/base/nsDocument.cpp:3630:3
    #6 0x7fb5544896e7 in nsHTMLDocument::CreateShell(nsPresContext*, nsViewManager*, mozilla::StyleSetHandle) /home/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:274:10
    #7 0x7fb5562cdff1 in nsDocumentViewer::InitPresentationStuff(bool) /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:634:16
    #8 0x7fb5562cd915 in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool) /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:889:10
    #9 0x7fb5562ccad7 in nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:618:10
    #10 0x7fb55704d1c7 in nsDocShell::SetupNewViewer(nsIContentViewer*) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:9374:7
    #11 0x7fb55704b969 in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7227:17
    #12 0x7fb557059d4d in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIURI*, bool) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:8087:14
    #13 0x7fb556ff9b03 in nsDocShell::EnsureContentViewer() /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7956:17
    #14 0x7fb55702bf87 in GetDocument /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:4504:3
    #15 0x7fb55702bf87 in non-virtual thunk to nsDocShell::GetDocument() /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:4502
    #16 0x7fb551bfbcb7 in MaybeCreateDoc /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:3613:38
    #17 0x7fb551bfbcb7 in GetDoc /home/worker/workspace/build/src/obj-firefox/dist/include/nsPIDOMWindow.h:174
    #18 0x7fb551bfbcb7 in EnsureInnerWindow /home/worker/workspace/build/src/obj-firefox/dist/include/nsPIDOMWindow.h:840
    #19 0x7fb551bfbcb7 in nsGlobalWindow::WrapObject(JSContext*, JS::Handle<JSObject*>) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.h:354
    #20 0x7fb550ab4a3c in XPCConvert::NativeInterface2JSObject(JS::MutableHandle<JS::Value>, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, bool, nsresult*) /home/worker/workspace/build/src/js/xpconnect/src/XPCConvert.cpp:785:16
    #21 0x7fb550ab29e3 in XPCConvert::NativeData2JS(JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, nsresult*) /home/worker/workspace/build/src/js/xpconnect/src/XPCConvert.cpp:344:16
    #22 0x7fb550b61312 in GatherAndConvertResults /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1601:18
    #23 0x7fb550b61312 in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1395
    #24 0x7fb550b61312 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1351
    #25 0x7fb550b6840f in GetAttribute /home/worker/workspace/build/src/js/xpconnect/src/xpcprivate.h:1929:17
    #26 0x7fb550b6840f in XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1179
    #27 0x7fb559d9761c in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:235:15
    #28 0x7fb559d9761c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442
    #29 0x7fb559e337c0 in Call /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:518:10
    #30 0x7fb559e337c0 in CallGetter /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:632
    #31 0x7fb559e337c0 in CallGetter(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<js::Shape*>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1757
    #32 0x7fb559da5e05 in GetExistingProperty<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1805:10
    #33 0x7fb559da5e05 in NativeGetPropertyInline<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2032
    #34 0x7fb559da5e05 in NativeGetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2066
    #35 0x7fb559da5e05 in GetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1491
    #36 0x7fb559da5e05 in GetProperty /home/worker/workspace/build/src/js/src/jsobj.h:846
    #37 0x7fb559da5e05 in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:4247
    #38 0x7fb559d7bb6b in GetPropertyOperation /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:190:12
    #39 0x7fb559d7bb6b in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2635
    #40 0x7fb559d5c93b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:400:12
    #41 0x7fb559d97e34 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:472:15
    #42 0x7fb559d988a1 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:518:10
    #43 0x7fb5598944da in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2776:12
    #44 0x7fb550b447f1 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:1211:23
    #45 0x7fb54f239d06 in PrepareAndDispatch /home/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:122:14
    #46 0x7fb54f238cd6 in SharedStub (/home/nils/fuzzer3/firefox/libxul.so+0x1faacd6)

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:37:5 in AddRef
Shadow bytes around the buggy address:
  0x0c1c80002780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1c80002790: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1c800027a0: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c1c800027b0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1c800027c0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x0c1c800027d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]
  0x0c1c800027e0: fd fd fd fd fa fa fa fa fa fa fa fa fd fd fd fd
  0x0c1c800027f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1c80002800: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1c80002810: 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c1c80002820: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
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
==16324==ABORTING
Group: core-security → layout-core-security
Brian, do you know who might look at this? Animation seems to be involved. Thanks.
Flags: needinfo?(bbirtles)
Keywords: csectype-uaf, sec-critical
This seems very similar to bug 1291665. I wonder why the fix from that bug didn't fix this one. Attachment 8781395 [details] [diff] might help but I need to check that we don't have a similar problem for independently constructed timelines (since attachment 8781395 [details] [diff] [review] won't help with that).

The next step is probably adding some diagnostics like bug 1291665 comment 10 to see if we can work out what is happening here.
After a bit of debugging I am seeing this pattern:

C++: >> DocumentTimeline(0x60e0003c55c0)::NotifyRefreshDriverCreated
        (refresh driver: 0x616000431180)
        (observing: 0)
C++: << DocumentTimeline(0x60e0003c55c0)::NotifyRefreshDriverCreated
        (observing: 0)
C++: >> DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: Adding 0x60e0003c55c0 to refresh driver 0x616000431180
C++: << DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: >> DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: << DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: Created animation 0x6120002ad9c0
C++: >> DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: << DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: >> DocumentTimeline(0x60e0003c55c0)::NotifyRefreshDriverDestroying
        (refresh driver: 0x616000431180)
        (observing: 1)
C++: << DocumentTimeline(0x60e0003c55c0)::NotifyRefreshDriverDestroying
        (observing: 0)
C++: >> DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: Adding 0x60e0003c55c0 to refresh driver 0x616000431180
C++: << DocumentTimeline(0x60e0003c55c0)::NotifyAnimationUpdated 0x6120002ad9c0
C++: >> DocumentTimeline(0x60e0003c55c0)::WillRefresh
        (driver: (nil))
        (observing: 1)

In other words, we notify that the refresh driver is destroying and successfully disassociate from it, but then a subsequent change makes us re-register. What's more, this happens during some interim period where we can still successfully go from document->pres shell->pres context->refresh driver.

If that's the case, I should be able to continue debugging this on windows (and in a debug build assertions in DocumentTimeline::WillRefresh should be failing)
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Flags: needinfo?(bbirtles)
A few more details. There are two places where we disassociate the timeline from its refresh driver:

A) nsDocument::Reset

In this case we do:

* mDocumentTimeline->NotifyRefreshDriverDestroying(rd);
* mDocumentTimeline = nullptr;

B) PresShell::Destroy()

In this case we do:

* mDocument->DeleteShell();
* mDocument->Timeline()->NotifyRefreshDriverDestroying(rd);


Now, in DocumentTimeline, we get the refresh driver by going:

mDocument->GetShell()->GetPresContext()->RefreshDriver() (with suitable null checks along the way)

For (B), we call DeleteShell() before calling NotifyRefreshDriverDestroying, and that breaks the link between the document and the presshell so any further attempts to get the refresh driver will fail (as expected).

It looks like in this bug we hit (A) where we disassociate from the refresh driver but don't immediately break the link(s) from document to refresh driver. As a result, we end up in a state where we can still get to the refresh driver (and any change to the animation will cause us to re-associate with it, despite the fact that it will probably disappear soon).


Stepping back, there seems to be an even bigger problem in that script-created timelines never get the NotifyRefreshDriverDestroying message. So how do they make sure they are disconnected from their refresh driver when it is going away?

Looking into how things normally get destroyed we have:

PresShell::Destroy()
  If mDocument is set:
  - mDocument->DeleteShell();
    Sets mPresShell = nullptr;
  - Calls mDocument->Timeline()->NotifyRefreshDriverDestroying(rd);
  If mPresContext is set:
  - Calls nsPresContext::DetachShell()
    - Calls Disconnect on its refresh driver (after nulling out mShell)

nsPresContext::~nsPresContext()
  - Calls DetachShell()
    - Calls Disconnect it its refresh driver (after nulling out mShell)
  - Calls Destroy()
    - Sets mRefreshDriver = null

For nsDocument::Reset, I think the idea here was basically that we should make sure that the object identity of document.timeline changes when we call reset. As a result, I think it's probably fine if the timeline from the old document can somehow be resurrected. Rather than try to fix that, we need to fix the general case of timelines that are not the document timeline, yet which might be watching the refresh driver associated with a pres shell that has been destroyed.

Basically, I think we need to store a hash of timelines on the pres shell so we can clear them out in PresShell::Destroy (at the point where we currently call mDocument->Timeline()->NotifyRefreshDriverDestroying). Alternatively, I guess we could stick that map on the document and iterate over it in nsDocument::DeleteShell()?
Priority: -- → P1
Attached patch Possible fix for trunk (obsolete) — — Splinter Review 
This patch thoroughly reworks how we track timelines so we can safely unregister them from the refresh driver when it disappears.

It's a fairly large patch so I think we might try something simpler for aurora. Fortunately, the ability to create extra timelines is not enabled on aurora so we could probably just include something like attachment 8781395 [details] [diff] [review]. Or, even simpler still, we could possibly just null out mDocument when the refresh driver dies (and sprinkle a few null checks about).
Attached patch Crash test — — Splinter Review 

    
    

    
  


  
Slight simplification

        Attachment #8796036 -
        Attachment is obsolete: true

    
    

    
  
One part of this patch I'm not sure about is the change where we stop unregistering the default document timeline from the refresh driver when we reset the document.

I made this change as a simplification since we originally only added that code because, once clearing the default document timeline we would no longer be able to reach it to tell it to disassociate itself when the pres shell is being destroyed. This bug reveals that even if we do that, however, the document it able to re-add itself. As a result this patch holds a separate list of timelines that it uses specifically for disassociating them from the refresh driver when it is about to be destroyed. That should make the handling of all timelines consistent and simple.

However, it may be better to more aggressively disassociate timelines when a document is reset. For example, clearing refresh driver observers sooner may improve performance / battery usage---but I think that would only matter when resetting documents and I'm not sure how common that is? Also, if we clear the connection between a timeline and a document we might free it up for GC sooner?

If we decide to go that route I think we could:

* Iterate over all the timelines when we reset the document (just like we do when we destroy the pres shell)
* Null out DocumentTimeline::mDocument (the wrapper cache parent object is AnimationTimeline::mWindow so we don't need to keep this reference) -- this is to ensure we do re-register with the refresh driver
* Add necessary null checks for mDocument within DocumentTimeline

    
  

        Attachment #8796040 -
        Attachment description: Possible fix for trunk v2 → Approach A (part 1): Store timelines on pres context

    
    

    
  


  
This addresses some of the concerns I raised in comment 8.

    
    

    
  


  
This is an alternative approach that I suspect is better (and more consistent with the patch I intend to land on aurora).

In this patch we record all documents created with the document they use to look up the refresh driver. Then, when either the document is reset or the refresh driver is destroyed, we go through the list of documents, disassociate them from the refresh driver, and break the link with the document so they can no longer re-register with the refresh driver.

I'm not entirely clear if it's a good idea to neuter all timelines when we call document.reset() but maybe it's ok?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix for release channels; r=hiro (obsolete)
        — 
          — Splinter Review
      

    


  
As reviewed in bug 1291665 comment 15.

For release channels we don't need to worry about independently-created DocumentTimeline objects since we don't ship the DocumentTimeline constructor on release channels yet.

        Attachment #8797855 -
        Flags: review+

    
    

    
  


  
Rebased

        Attachment #8797854 -
        Attachment is obsolete: true

    
    

    
  
Brian, how far back does this affect Firefox?
Flags: needinfo?(bbirtles)

    
    

    
  
So this test fails when we reset the document, but then trigger a change on an animation connected to that document's timeline. I *think* it's only possible to trigger a change on an animation after the document is reset using the animation API. If that's the case, this goes back to Firefox 48 which is the first release where we shipped Element.animate (bug 1245000).
Flags: needinfo?(bbirtles)

    
    

    
  


  
Approach B was failing in dom/animation/test/document-timeline/test_document-timeline.html because we'd disconnect from the document meaning that document.timeline.currentTime would return null after we destroyed the pres shell. I don't think it should do that.

After looking into it, it looks like I was complicating this bug more that necessary. The fundamental cause is that in bug 1232829 we came across a case where the pres shell was destroyed but we failed to unregister from the refresh driver because the call to nsDocument::Reset broke the connection from document to timeline (so we couldn't reach the timeline to tell it to unregister). To fix that we make the timeline unregister from the refresh driver in nsDocument::Reset.

*However* if after doing that we modify an animation attached to the timeline, it can cause the timeline to re-register with the refresh driver and then we have the same problem, i.e. we can't reach the timeline to tell it to unregister from the refresh driver when the pres shell disappears.

Furthermore, while investigating I notice that we never take care to unregister script-generated timelines when the presshell is destroyed. Fortunately, that feature is only enabled on non-release builds so the damage is limited but we should fix that while we're at it.

This updated patch should fix both problems by:

* Recording all timelines against the document they use to fetch their refresh driver
* *Not* clearing this reference when a document is reset (so we can always reach these timelines, even if they get re-added somehow)
* Notifying all timelines when the presshell is being destroyed

I'm still not sure if we should forcefully disconnect all timelines from their document when it is reset. Originally I tried doing this but was a bit too eager and also did this when the presshell was destroyed which caused dom/animation/test/document-timeline/test_document-timeline.html to fail. Perhaps we can try doing that in a separate bug (but only for resetting the document). For now, this should be enough to fix the crash.

        Attachment #8797889 -
        Attachment is obsolete: true

    
  

        Attachment #8798326 -
        Flags: review?(bugs)

    
    

    
  
Comment on attachment 8798326 [details] [diff] [review]
Approach B: Store timelines on the document

>+  mozilla::LinkedList<mozilla::dom::DocumentTimeline>& Timelines() override
>+    { return mTimelines; }
Nit, I'd write this
mozilla::LinkedList<mozilla::dom::DocumentTimeline>& Timelines() override
{
  return mTimelines;
}

        Attachment #8798326 -
        Flags: review?(bugs) → review+

    
    

    
  


  
Addressed review feedback, updated patch title, added an assert to nsDocument::~nsDocument.

        Attachment #8796040 -
        Attachment is obsolete: true

        Attachment #8797852 -
        Attachment is obsolete: true

        Attachment #8797855 -
        Attachment is obsolete: true

        Attachment #8798326 -
        Attachment is obsolete: true

        Attachment #8798682 -
        Flags: review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix rebased for release (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  
Comment on attachment 8798682 [details] [diff] [review]
Fix for trunk: Store timelines on document; r=smaug

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Somewhat easily. Once you recognize that this is a security-related patch you could infer that it has something to do with the interaction between timelines, the refresh driver and documents being reset. Of course you'd still need to be able to trigger GC to expose the bug.

> 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?
Release, Aurora, Beta (Firefox 48 onwards)

> If not all supported branches, which bug introduced the flaw?
Ultimately it was introduced by bug 1195180 but only shipped to release channels by bug 1245000.

> Do you have backports for the affected branches? If not, how different,
> hard to create, and risky will they be?
Yes (attachment 8798683 [details] [diff] [review] and attachment 8798684 [details] [diff] [review]) although I have yet to test these, only that they apply cleanly.

> How likely is this patch to cause regressions; how much testing does it need?
Not likely. I am currently running it through try (https://treeherder.mozilla.org/#/jobs?repo=try&revision=3afb34a77846) with a patch title that does not clearly suggest a security bug.

        Attachment #8798682 -
        Flags: sec-approval?

    
    

    
  
sec-approval+ for trunk. Please nominate Aurora and Beta patches for approval once trunk goes in.

          status-firefox49:
          affected → wontfix

          tracking-firefox50:
          --- → +

          tracking-firefox51:
          --- → +

          tracking-firefox52:
          --- → +

        Attachment #8798682 -
        Flags: sec-approval? → sec-approval+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/bfe9d976f750
Status: ASSIGNED → RESOLVED
Closed: 8 years ago

          status-firefox52:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52

    
    

    
  
Comment on attachment 8798683 [details] [diff] [review]
Fix rebased for aurora/beta

Approval Request Comment
[Feature/regressing bug #]: Ultimately it was introduced by bug 1195180 but only shipped to release channels by bug 1245000.
[User impact if declined]: Security vulnerability, unpatched use after free.
[Describe test coverage new/current, TreeHerder]: Landed on m-c about 15 hours ago and seems to be ok. I've pushed to try just now for aurora[1]/beta[2] and it seems to be ok.
[Risks and why]: No known obvious risks.
[String/UUID change made/needed]: none

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab6fb813091
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a4e9fa5f6d2

        Attachment #8798683 -
        Flags: approval-mozilla-beta?

        Attachment #8798683 -
        Flags: approval-mozilla-aurora?

    
    

    
  


  
Looks like I missed a few lines when I rebased this last time.

        Attachment #8798684 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 8799608 [details] [diff] [review]
Fix rebased for release

Approval Request Comment
[Feature/regressing bug #]: Ultimately it was introduced by bug 1195180 but only shipped to release channels by bug 1245000
[User impact if declined]: Security vulnerability, unpatched use after free.
[Describe test coverage new/current, TreeHerder]: Landed on m-c about 15 hours ago and seems to be ok. I've just pushed to try now.[1]
[Risks and why]: No known obvious risks.
[String/UUID change made/needed]: None.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=0479fb308eba

        Attachment #8799608 -
        Flags: approval-mozilla-release?

    
    

    
  
Comment on attachment 8798683 [details] [diff] [review]
Fix rebased for aurora/beta

Sec-crit, Aurora51+, Beta50+

        Attachment #8798683 -
        Flags: approval-mozilla-beta?

        Attachment #8798683 -
        Flags: approval-mozilla-beta+

        Attachment #8798683 -
        Flags: approval-mozilla-aurora?

        Attachment #8798683 -
        Flags: approval-mozilla-aurora+

    
    

    
  
Comment on attachment 8799608 [details] [diff] [review]
Fix rebased for release

This got wontfixed for 49 already in security triage.

        Attachment #8799608 -
        Flags: approval-mozilla-release? → approval-mozilla-release-
Group: layout-core-security → core-security-release
Flags: qe-verify+

    
    

    
  
I've managed to reproduce this bug on an affected build (49.0.1-build3, 20160922113459), using the testcase provided in Comment 0 and the fuzzPriv extension installed.

This is verified fixed on:

  - 50.0b8-build1 (20161017130949)
  - 51.0a2 (2016-10-18)
  - 52.0a1 (2016-10-18)

using Windows 10 x64, Ubuntu 14.04 x86 and Mac OS X 10.11.6. A crash is no longer triggered by this scenario.
Status: RESOLVED → VERIFIED

          status-firefox50:
          fixed → verified

          status-firefox51:
          fixed → verified

          status-firefox52:
          fixed → verified
Flags: sec-bounty?
Blocks: 1195180, 1245000
Flags: sec-bounty?
Flags: sec-bounty+
Flags: in-testsuite?
Keywords: sec-critical → regression, 
          
            sec-high
Whiteboard: [adv-main50+]
Alias: CVE-2016-9068
Group: core-security-release

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: