Closed Bug 1283625 Opened 4 years ago Closed 4 years ago

heap-use-after-free in mozilla::dom::KeyframeEffect::NotifySpecifiedTimingUpdated

Categories

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

50 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox47 --- unaffected
firefox48 + verified
firefox49 + verified
firefox-esr45 --- unaffected
firefox50 + verified

People

(Reporter: nils, Assigned: boris)

References

Details

(Keywords: csectype-uaf, regression, sec-critical)

Attachments

(3 files, 2 obsolete files)

The testcase crashes the latest Firefox ASAN build (BuildID=20160629071428) as follows. The testcase requires the fuzzPriv extension.

<script>
function start() {
        o0=window.document;
        o19=document.createElement('span');
        window.fuzzPriv.callDrawWindow(0);
        window.top.requestAnimationFrame(f1);
}
function f1() {
        o378=o19.animate([{left: 'auto',strokeWidth: '1014%',widows: '64',MozAppearance: 'tab',MozUserModify: 'write-only'},{left: 'auto',strokeWidth: '2048vmax',widows: '64',MozAppearance: 'treeitem',MozUserModify: 'write-only'}],1);
        window.top.setTimeout(f2, 24);
}
function f2() {
        o756=o378.effect;
        o879=o756['timing'];
        delete o378;
        delete o756;
        window.top.setTimeout(f3, 24);
}
function f3() {
        window.fuzzPriv.CC();
        window.fuzzPriv.forceGC();
        window.fuzzPriv.CC();
        o879['delay']=4294967295;
}
</script>
<body onload="start()"></body>

Asan output:

=================================================================
==10933==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d00003d868 at pc 0x7f265aa27400 bp 0x7ffeaaa6e550 sp 0x7ffeaaa6e548
READ of size 1 at 0x60d00003d868 thread T0 (Web Content)
    #0 0x7f265aa273ff in isSome /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/Maybe.h:155:32
    #1 0x7f265aa273ff in operator bool /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/Maybe.h:154
    #2 0x7f265aa273ff in mozilla::dom::KeyframeEffect::NotifySpecifiedTimingUpdated() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/animation/KeyframeEffect.cpp:1499
    #3 0x7f265b106c92 in mozilla::dom::AnimationEffectTimingBinding::set_delay(JSContext*, JS::Handle<JSObject*>, mozilla::dom::AnimationEffectTiming*, JSJitSetterCallArgs) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/AnimationEffectTimingBinding.cpp:44:3
    #4 0x7f265cc9915e in mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/bindings/BindingUtils.cpp:2752:8
    #5 0x7f2662b8ae2b in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:232:15
    #6 0x7f2662b8ae2b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:441
    #7 0x7f2662b8cc6c in Call /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:517:10
    #8 0x7f2662b8cc6c in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:644
    #9 0x7f2662bf2694 in SetExistingProperty /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/NativeObject.cpp:2364:10
    #10 0x7f2662bf2694 in js::NativeSetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::QualifiedBool, JS::ObjectOpResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/NativeObject.cpp:2399
    #11 0x7f2662b654c6 in SetProperty /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/NativeObject.h:1498:12
    #12 0x7f2662b654c6 in SetPropertyOperation /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:256
    #13 0x7f2662b654c6 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:2666
    #14 0x7f2662b526fb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:399:12
    #15 0x7f2662b8b4d0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:471:15
    #16 0x7f2662b8bb41 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:517:10
    #17 0x7f26626cdbc8 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:2840:12
    #18 0x7f265c8f086d in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/FunctionBinding.cpp:36:8
    #19 0x7f265ab67512 in Call<nsCOMPtr<nsISupports> > /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:70:12
    #20 0x7f265ab67512 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsGlobalWindow.cpp:12267
    #21 0x7f265ab453b4 in nsGlobalWindow::RunTimeout(nsTimeout*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsGlobalWindow.cpp:12510:32
    #22 0x7f265aaddcb1 in nsGlobalWindow::TimerCallback(nsITimer*, void*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsGlobalWindow.cpp:12756:3
    #23 0x7f265816dfa7 in nsTimerImpl::Fire() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsTimerImpl.cpp:524:7
    #24 0x7f265814515c in nsTimerEvent::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/TimerThread.cpp:286:3
    #25 0x7f2658151fa6 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1073:7
    #26 0x7f26581d088c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #27 0x7f2658f2f344 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:132:5
    #28 0x7f2658ea3db8 in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235:3
    #29 0x7f2658ea3db8 in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #30 0x7f2658ea3db8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #31 0x7f265e9015ff in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156:3
    #32 0x7f266099b207 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:832:12
    #33 0x7f2658ea3db8 in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235:3
    #34 0x7f2658ea3db8 in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #35 0x7f2658ea3db8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #36 0x7f266099a898 in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:662:7
    #37 0x4e2bc5 in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:224:19
    #38 0x7f265521b82f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291
    #39 0x41e778 in _start (/home/nils/fuzzer3/firefox/plugin-container+0x41e778)

0x60d00003d868 is located 56 bytes inside of 144-byte region [0x60d00003d830,0x60d00003d8c0)
freed by thread T0 (Web Content) here:
    #0 0x4b4ecb in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3
    #1 0x7f265802f6f4 in SnowWhiteKiller::~SnowWhiteKiller() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2685:9
    #2 0x7f265802f2e6 in nsCycleCollector::FreeSnowWhite(bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2859:3
    #3 0x7f26580359a5 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:3841:3
    #4 0x7f265803515c in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:3666:9
    #5 0x7f2658038466 in nsCycleCollector_collect(nsICycleCollectorListener*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:4160:3
    #6 0x7f265af1fa39 in nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsJSEnvironment.cpp:1435:3
    #7 0x7f265aa9cd2d in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*, int) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsDOMWindowUtils.cpp:1291:3
    #8 0x7f2658177b26 in NS_InvokeByIndex /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:180:23
    #9 0x7f2659b31bb2 in Invoke /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCWrappedNative.cpp:2075:12
    #10 0x7f2659b31bb2 in Call /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1394
    #11 0x7f2659b31bb2 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1361
    #12 0x7f2659b38d9c in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1128:12
    #13 0x7f2662b8ae2b in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:232:15
    #14 0x7f2662b8ae2b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:441
    #15 0x7f2662b6c079 in CallFromStack /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:504:12
    #16 0x7f2662b6c079 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:2873
    #17 0x7f2662b526fb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:399:12
    #18 0x7f2662b8b4d0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:471:15
    #19 0x7f2662b8bb41 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:517:10
    #20 0x7f26626cb678 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:2781:12
    #21 0x7f2659a6aa75 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/ExportHelpers.cpp:353:18
    #22 0x7f2662b8ae2b in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:232:15
    #23 0x7f2662b8ae2b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:441
    #24 0x7f2662b6c079 in CallFromStack /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:504:12
    #25 0x7f2662b6c079 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:2873
    #26 0x7f2662b526fb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:399:12
    #27 0x7f2662b8b4d0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:471:15
    #28 0x7f2662b8bb41 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:517:10
    #29 0x7f26626cdbc8 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:2840:12
    #30 0x7f265c8f086d in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/FunctionBinding.cpp:36:8
    #31 0x7f265ab67512 in Call<nsCOMPtr<nsISupports> > /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:70:12
    #32 0x7f265ab67512 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsGlobalWindow.cpp:12267
    #33 0x7f265ab453b4 in nsGlobalWindow::RunTimeout(nsTimeout*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsGlobalWindow.cpp:12510:32
    #34 0x7f265aaddcb1 in nsGlobalWindow::TimerCallback(nsITimer*, void*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsGlobalWindow.cpp:12756:3
    #35 0x7f265816dfa7 in nsTimerImpl::Fire() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsTimerImpl.cpp:524:7
    #36 0x7f265814515c in nsTimerEvent::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/TimerThread.cpp:286:3

previously allocated by thread T0 (Web Content) here:
    #0 0x4b51eb in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3
    #1 0x4e2ebd in moz_xmalloc /builds/slave/m-cen-l64-asan-000000000000000/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7f265aa26966 in operator new /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:193:12
    #3 0x7f265aa26966 in already_AddRefed<mozilla::dom::KeyframeEffect> mozilla::dom::KeyframeEffectReadOnly::ConstructKeyframeEffect<mozilla::dom::KeyframeEffect, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions>(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/animation/KeyframeEffect.cpp:816
    #4 0x7f265aa2610d in mozilla::dom::KeyframeEffect::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/animation/KeyframeEffect.cpp:1490:10
    #5 0x7f265ac39b76 in mozilla::dom::Element::Animate(mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JSContext*, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/Element.cpp:3344:5
    #6 0x7f265ac394cb in mozilla::dom::Element::Animate(JSContext*, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/Element.cpp:3301:10
    #7 0x7f265c88069b in mozilla::dom::ElementBinding::animate(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:3418:55
    #8 0x7f265cc99977 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/bindings/BindingUtils.cpp:2784:13
    #9 0x7f2662b8ae2b in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:232:15
    #10 0x7f2662b8ae2b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:441
    #11 0x7f2662b6c079 in CallFromStack /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:504:12
    #12 0x7f2662b6c079 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:2873
    #13 0x7f2662b526fb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:399:12
    #14 0x7f2662b8b4d0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:471:15
    #15 0x7f2662b8bb41 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:517:10
    #16 0x7f26626cdbc8 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:2840:12
    #17 0x7f265c3abfeb in mozilla::dom::FrameRequestCallback::Call(JSContext*, JS::Handle<JS::Value>, double, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:663:8
    #18 0x7f265ef92ce2 in Call /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/dom/WindowBinding.h:578:12
    #19 0x7f265ef92ce2 in Call /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/dom/WindowBinding.h:593
    #20 0x7f265ef92ce2 in nsRefreshDriver::RunFrameRequestCallbacks(mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:1615
    #21 0x7f265ef8cd23 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:1725:7
    #22 0x7f265ef9979c in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:251:7
    #23 0x7f265ef99469 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:270:5
    #24 0x7f265ef9afb4 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:430:9
    #25 0x7f265f8d3ca4 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/ipc/VsyncChild.cpp:64:5
    #26 0x7f26594f518a in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:240:20
    #27 0x7f2658fe420d in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:2133:16
    #28 0x7f2658f28077 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1658:14
    #29 0x7f2658f24eb6 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1596:17
    #30 0x7f2658f12c87 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1563:5
    #31 0x7f2658f42b62 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:729:12
    #32 0x7f2658f42b62 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:735
    #33 0x7f2658f42b62 in mozilla::detail::RunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:763
    #34 0x7f2658f4214f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:476:22
    #35 0x7f2658f4214f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:495
    #36 0x7f2658151fa6 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1073:7
    #37 0x7f26581d088c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/Maybe.h:155:32 in isSome
Shadow bytes around the buggy address:
  0x0c1a7ffffab0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1a7ffffac0: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa
  0x0c1a7ffffad0: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1a7ffffae0: fd fd fd fd fa fa fa fa fa fa fa fa fd fd fd fd
  0x0c1a7ffffaf0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa
=>0x0c1a7ffffb00: fa fa fa fa fa fa fd fd fd fd fd fd fd[fd]fd fd
  0x0c1a7ffffb10: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c1a7ffffb20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1a7ffffb30: 00 00 fa fa fa fa fa fa fa fa 00 00 00 00 00 00
  0x0c1a7ffffb40: 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c1a7ffffb50: 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
==10933==ABORTING
Flags: needinfo?(bbirtles)
Hi Boris, would you like to have a look at this one?

At a glance, it looks like we're holding on to the timing object whilst the effect is destroyed. So perhaps we're failing to call AnimationEffectTiming::Unlink() when the KeyframeEffect is destroyed?

Note that this is sec-critical. I'm not sure if you've dealt with many of these before but please don't post patches to try server which reproduce this bug or help others discover it. We'll also need sec-approval before landing a fix.
Flags: needinfo?(bbirtles) → needinfo?(boris.chiou)
(In reply to Brian Birtles (:birtles) from comment #1)
> Hi Boris, would you like to have a look at this one?
> 
> At a glance, it looks like we're holding on to the timing object whilst the
> effect is destroyed. So perhaps we're failing to call
> AnimationEffectTiming::Unlink() when the KeyframeEffect is destroyed?
> 
> Note that this is sec-critical. I'm not sure if you've dealt with many of
> these before but please don't post patches to try server which reproduce
> this bug or help others discover it. We'll also need sec-approval before
> landing a fix.

I can take a look at this. Actually, I haven't dealt with sec-critical bug before, so how do I verify the patches before landing if I cannot push to try? Thanks.
Flags: needinfo?(boris.chiou)
(In reply to Boris Chiou [:boris] from comment #2)
> I can take a look at this. Actually, I haven't dealt with sec-critical bug
> before, so how do I verify the patches before landing if I cannot push to
> try? Thanks.

First we need to work out the cause and which branches are affected. I think release will not be affected (since this API is disabled on release channels) so we won't need to be as careful.

If the fix is simple, you can probably just test it locally. Also, if the fix alone doesn't make it obvious how to exploit the bug, it might be ok to push the fix only (and *not* the tests) to try. That's my understanding, anyway.

https://wiki.mozilla.org/Security/Bug_Approval_Process has some background.
(In reply to Brian Birtles (:birtles) from comment #1)
> Hi Boris, would you like to have a look at this one?
> 
> At a glance, it looks like we're holding on to the timing object whilst the
> effect is destroyed. So perhaps we're failing to call
> AnimationEffectTiming::Unlink() when the KeyframeEffect is destroyed?

You are right. I'm trying to reproduce this and dump some log:

> function start() {
>   o0=window.document;
>   o19=document.createElement('span');
>   window.fuzzPriv.callDrawWindow(0);
>   window.top.requestAnimationFrame(f1);
> }
> function f1() {
>   o378=o19.animate([ {strokeWidth: '1014%'}, {strokeWidth: '2048vmax'} ], 1);
1. Construct AnimationEffectTiming (0x121179200), and AnimationEffectTiming::mEffect is 0x1237f8ee0.
2. Construct KeyframeEffect (0x1237f8ee0), and KeyframeEffect::mTiming is 0x121179200.
>   window.top.setTimeout(f2, 24);
> }
> function f2() {
>   o756=o378.effect;
>   o879=o756['timing'];
>   delete o378;
Delete Animation object
>   delete o756;
Delete KeyframeEffect object
>   window.top.setTimeout(f3, 24);
> }
> function f3() {
>   window.fuzzPriv.CC();
>   window.fuzzPriv.forceGC();
>   window.fuzzPriv.CC();
Here, we destruct the current KeyframeEffect (0x1237f8ee0), but KeyframeEffect::mTiming is nullptr, so we don't unlink it.

I don't know why mTiming becomes nullptr because this AnimationEffectTiming object is still alive. Is it possible cycle-collection module nullifies it even if JS context still holds it?
>   o879['delay']=4294967295;
> }
(In reply to Boris Chiou [:boris] from comment #4)
> > function f3() {
> >   window.fuzzPriv.CC();
> >   window.fuzzPriv.forceGC();
> >   window.fuzzPriv.CC();
> Here, we destruct the current KeyframeEffect (0x1237f8ee0), but
> KeyframeEffect::mTiming is nullptr, so we don't unlink it.
> 
> I don't know why mTiming becomes nullptr because this AnimationEffectTiming
> object is still alive. Is it possible cycle-collection module nullifies it
> even if JS context still holds it?

I thought cycle-collection won't nullify mTiming before calling the destructor if it is still alive. Maybe we can use WeakPtr<KeyframeEffect> in AnimationEffectTiming to avoid this problem.
(In reply to Boris Chiou [:boris] from comment #5)
> I thought cycle-collection won't nullify mTiming before calling the
> destructor if it is still alive. Maybe we can use WeakPtr<KeyframeEffect> in
> AnimationEffectTiming to avoid this problem.

I have a better idea: rewrite the unlink macro
e.g.

Let
> NS_IMPL_CYCLE_COLLECTION_INHERITED(KeyframeEffectReadOnly,
>                                    AnimationEffectReadOnly,
>                                    mTarget,
>                                    mAnimation,
>                                    mTiming)

becomes:

NS_IMPL_CYCLE_COLLECTION_CLASS(KeyframeEffectReadOnly)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(KeyframeEffectReadOnly,
                                                AnimationEffectReadOnly)
  if (tmp->mTiming) {
    tmp->mTiming->Unlink();
  }
NS_IMPL_CYCLE_COLLECTION_UNLINK(mAnimation)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mTarget)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mTiming)
NS_IMPL_CYCLE_COLLECTION_UNLINK_END

NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(KeyframeEffectReadOnly,
                                                  AnimationEffectReadOnly)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnimation)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTarget)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTiming)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END


This solution may be simpler. We call mTiming->Unlink() before making mTiming = nullptr, and then we can remove these code from the destructor. (e.g. use ~KeyframeEffect() override = default). However, I have to add UNLINK macros and TRAVERSE macros for other data members.
Assignee: nobody → boris.chiou
Status: NEW → ASSIGNED
MozReview-Commit-ID: 3eIAL77aDLV
Attachment #8767471 - Attachment is obsolete: true
Comment on attachment 8767473 [details] [diff] [review]
Unlink timing object before nullifying it. (v2)

Review of attachment 8767473 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure if KeyframeEffect could be destroyed by other ways, instead of cycle-collection, so I didn't remove the unlink code in the destructor.
Attachment #8767473 - Flags: review?(bbirtles)
Comment on attachment 8767473 [details] [diff] [review]
Unlink timing object before nullifying it. (v2)

Review of attachment 8767473 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. A few nits:

* We normally indent the unlink / traverse blocks like this[1]
* NS_IMPL_CYCLE_COLLECTION_UNLINK now accepts variadic args so you should
  be able to just write:
    NS_IMPL_CYCLE_COLLECTION_UNLINK(mTarget, mAnimation, mTiming)
* Unless there's a reason not to, we should keep the existing order of
  arguments which matches the order in the header file
* I don't think we need the additional comment

[1] http://searchfox.org/mozilla-central/rev/261fe13dcd88cfd2e99e65755e7ca4b7a2e583df/dom/animation/AnimationTimeline.cpp#16

That would give us something like:

  NS_IMPL_CYCLE_COLLECTION_CLASS(KeyframeEffectReadOnly)

  NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(KeyframeEffectReadOnly,
                                                  AnimationEffectReadOnly)
    if (tmp->mTiming) {
      tmp->mTiming->Unlink();
    }
    NS_IMPL_CYCLE_COLLECTION_UNLINK(mTarget, mAnimation, mTiming)
  NS_IMPL_CYCLE_COLLECTION_UNLINK_END

  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(KeyframeEffectReadOnly,
                                                    AnimationEffectReadOnly)
    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTarget, mAnimation, mTiming)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END

r=birtles with those changes made
Attachment #8767473 - Flags: review?(bbirtles) → review+
(In reply to Boris Chiou [:boris] from comment #9)
> I'm not sure if KeyframeEffect could be destroyed by other ways, instead of
> cycle-collection, so I didn't remove the unlink code in the destructor.

Yes, it certainly can so we need to keep that code.
After updating the patch, please request sec-approval.
MozReview-Commit-ID: 3eIAL77aDLV
Attachment #8767542 - Flags: review+
Attachment #8767473 - Attachment is obsolete: true
Attachment #8767542 - Flags: sec-approval?
Backport to aurora. Carry birtles' r+ from attachment 8767542 [details] [diff] [review].
Attachment #8767824 - Flags: review+
Backport to beta. Carry birtles' r+ from attachment 8767542 [details] [diff] [review].
Attachment #8767825 - Flags: review+
Comment on attachment 8767542 [details] [diff] [review]
Unlink timing object before nullifying it. (v3, carry birtles' r+)

Review of attachment 8767542 [details] [diff] [review]:
-----------------------------------------------------------------

[Security approval request comment]
* How easily could an exploit be constructed based on the patch?
Not easy. The patch fixes the unlink order.

* Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Only mention "unlink", but don't have any hint to reproduce this problem.

Which older supported branches are affected by this flaw?
* 48 (beta) and 49 (aurora)

If not all supported branches, which bug introduced the flaw?
* Bug 1249564.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
* Yes. attachment 8767824 [details] [diff] [review] for aurora and attachment 8767825 [details] [diff] [review] for beta. Only few lines in these patches.

How likely is this patch to cause regressions; how much testing does it need?
* Unlikely, and no test. It simply clears an unused pointer and we already have null check for it.
The regression is from 48, so adding tracking flags for 48+.
sec-approval+ for trunk. We'll want patches nominated for Aurora and Beta as well, to go in following trunk.
Attachment #8767542 - Flags: sec-approval? → sec-approval+
Priority: -- → P1
Comment on attachment 8767824 [details] [diff] [review]
Backport of fix for aurora. r=birtles

Review of attachment 8767824 [details] [diff] [review]:
-----------------------------------------------------------------

Approval Request Comment
[Feature/regressing bug #]: Bug 1249564
[User impact if declined]: Security bug: potentially exploitable use-after-free
[Describe test coverage new/current, TreeHerder]: No specific tests for this bug (it's a security bug where the fix has yet to ship) but a fix for this bug has just landed on inbound[1] so that should let us see the results of existing regression tests.
[Risks and why]: Minimal. A few-line fix that simply nullifies a pointer, so we will skip it in other code to avoid use-after-free.
[String/UUID change made/needed]: None.

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/2df474f955a769969bd8f263abd942f6b2991fa4
Attachment #8767824 - Flags: approval-mozilla-aurora?
Comment on attachment 8767825 [details] [diff] [review]
Backport of fix for beta. r=birtles

Review of attachment 8767825 [details] [diff] [review]:
-----------------------------------------------------------------

Approval Request Comment
[Feature/regressing bug #]: Bug 1249564
[User impact if declined]: Security bug: potentially exploitable use-after-free
[Describe test coverage new/current, TreeHerder]: No specific tests for this bug (it's a security bug where the fix has yet to ship) but a fix for this bug has just landed on inbound[1] so that should let us see the results of existing regression tests.
[Risks and why]: Minimal. A few-line fix that simply nullifies a pointer, so we will skip it in other code to avoid use-after-free.
[String/UUID change made/needed]: None.

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/2df474f955a769969bd8f263abd942f6b2991fa4
Attachment #8767825 - Flags: approval-mozilla-beta?
Comment on attachment 8767824 [details] [diff] [review]
Backport of fix for aurora. r=birtles

Fix for sec issue, please uplift to aurora.
Attachment #8767824 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Just realized this hasn't landed on m-c yet, it should be OK to do so. I'd like to uplift this to beta today so it can go into the beta 6 build.
https://hg.mozilla.org/mozilla-central/rev/2df474f955a7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Attachment #8767825 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Group: core-security → core-security-release
I've managed to reproduce this bug on an older Nightly build from 2016-06-30 with domFuzzLite3 add-on installed, using Windows 10 x64 [1], Ubuntu 14.04 LTS [2] and Mac OS X 10.11 [3].

The is verified fixed on 50.0a1 (2016-07-27), 49.0a2 (2016-07-27) and 48.0-build2, using Windows 10 x64, Ubuntu 14.04 LTS x86 and Mac OS X 10.11.


[1] https://crash-stats.mozilla.com/report/index/0f8760b1-6192-410e-9537-f8b8b2160725
[2] https://crash-stats.mozilla.com/report/index/694461f6-823e-45f8-9bf2-309ad2160725
[3] https://crash-stats.mozilla.com/report/index/f2366982-7611-4dc9-8043-b34d821
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.