1347168 - (CVE-2017-5433) heap-use-after-free in nsSMILCompositor::GetFirstFuncToAffectSandwich

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Nils]

The attached testcase (crash.html) crashes the latest ASAN build of Firefox (BuildID=20170313233726) as follows. The testcase requires the attached mini.svg.

=================================================================
==28440==ERROR: AddressSanitizer: heap-use-after-free on address 0x616000199dc8 at pc 0x7f8edf28b179 bp 0x7ffe8c450540 sp 0x7ffe8c450538
READ of size 8 at 0x616000199dc8 thread T0 (Web Content)
    #0 0x7f8edf28b178 in Equals /home/worker/workspace/build/src/dom/smil/nsSMILTargetIdentifier.h:75:38
    #1 0x7f8edf28b178 in UpdateCachedTarget /home/worker/workspace/build/src/dom/smil/nsSMILAnimationFunction.cpp:336
    #2 0x7f8edf28b178 in nsSMILCompositor::GetFirstFuncToAffectSandwich() /home/worker/workspace/build/src/dom/smil/nsSMILCompositor.cpp:168
    #3 0x7f8edf27f4cd in nsSMILCompositor::ComposeAttribute(bool&) /home/worker/workspace/build/src/dom/smil/nsSMILCompositor.cpp:80:33
    #4 0x7f8edf27d57d in nsSMILAnimationController::DoSample(bool) /home/worker/workspace/build/src/dom/smil/nsSMILAnimationController.cpp:453:5
    #5 0x7f8edec24a21 in mozilla::dom::SVGAnimationElement::ActivateByHyperlink() /home/worker/workspace/build/src/dom/svg/SVGAnimationElement.cpp:404:7
    #6 0x7f8edfefd6a9 in mozilla::PresShell::GoToAnchor(nsAString_internal const&, bool, unsigned int) /home/worker/workspace/build/src/layout/base/PresShell.cpp:3185:7
    #7 0x7f8ee24d1ac6 in nsDocShell::ScrollToAnchor(bool, bool, nsACString_internal&, unsigned int) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:11533:12
    #8 0x7f8ee246bc3c in nsDocShell::InternalLoad(nsIURI*, nsIURI*, bool, nsIURI*, unsigned int, nsIPrincipal*, nsIPrincipal*, unsigned int, nsAString_internal const&, char const*, nsAString_internal const&, nsIInputStream*, nsIInputStream*, unsigned int, nsISHEntry*, bool, nsAString_internal const&, nsIDocShell*, nsIURI*, bool, nsIDocShell**, nsIRequest**) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:10495:12
    #9 0x7f8ee24642fe in nsDocShell::LoadURI(nsIURI*, nsIDocShellLoadInfo*, unsigned int, bool) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:1573:10
    #10 0x7f8edbe6caf5 in mozilla::dom::Location::SetURI(nsIURI*, bool) /home/worker/workspace/build/src/dom/base/Location.cpp:288:12
    #11 0x7f8edbe6d655 in mozilla::dom::Location::SetHash(nsAString_internal const&) /home/worker/workspace/build/src/dom/base/Location.cpp:341:10
    #12 0x7f8edc5fdaf5 in SetHash /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/Location.h:209:14
    #13 0x7f8edc5fdaf5 in mozilla::dom::LocationBinding::set_hash(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Location*, JSJitSetterCallArgs) /home/worker/workspace/build/src/obj-firefox/dom/bindings/LocationBinding.cpp:703
    #14 0x7f8edd9923e2 in mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2921:8
    #15 0x7f8ee345d67f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #16 0x7f8ee345d67f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #17 0x7f8ee345f3e4 in InternalCall /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:493:12
    #18 0x7f8ee345f3e4 in Call /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512
    #19 0x7f8ee345f3e4 in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:641
    #20 0x7f8ee408aaea in js::SetPropertyIgnoringNamedGetter(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyDescriptor>, JS::ObjectOpResult&) /home/worker/workspace/build/src/js/src/proxy/BaseProxyHandler.cpp:245:10
    #21 0x7f8edd99dd79 in mozilla::dom::DOMProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /home/worker/workspace/build/src/dom/bindings/DOMJSProxyHandler.cpp:258:10
    #22 0x7f8ee40bb1f4 in js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:369:12
    #23 0x7f8ee3fa53fa in JSObject::nonNativeSetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /home/worker/workspace/build/src/js/src/jsobj.cpp:1049:12
    #24 0x7f8ee40dd25c in SetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1458:16
    #25 0x7f8ee40dd25c in js::Wrapper::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /home/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:152
    #26 0x7f8ee40922be in js::CrossCompartmentWrapper::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:238:5
    #27 0x7f8ee40bb1f4 in js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:369:12
    #28 0x7f8ee3fa53fa in JSObject::nonNativeSetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /home/worker/workspace/build/src/js/src/jsobj.cpp:1049:12
    #29 0x7f8ee343cdcf in SetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1458:16
    #30 0x7f8ee343cdcf in SetPropertyOperation /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:243
    #31 0x7f8ee343cdcf in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2747
    #32 0x7f8ee34297ab in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #33 0x7f8ee345d996 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #34 0x7f8ee345e072 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:512:10
    #35 0x7f8ee3e34d8c 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:2887:12
    #36 0x7f8edd4117e2 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8
    #37 0x7f8eddd96a8e in HandleEvent<mozilla::dom::EventTarget *> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12
    #38 0x7f8eddd96a8e in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1119
    #39 0x7f8eddd98a58 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1297:20
    #40 0x7f8eddd83133 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:465:5
    #41 0x7f8eddd869f7 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:822:9
    #42 0x7f8edbcc09ce in nsGlobalWindow::PostHandleEvent(mozilla::EventChainPostVisitor&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:3812:7
    #43 0x7f8eddd83245 in PostHandleEvent /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:416:3
    #44 0x7f8eddd83245 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:468
    #45 0x7f8eddd83854 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:518:5
    #46 0x7f8eddd869f7 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:822:9
    #47 0x7f8edffe32a6 in nsDocumentViewer::LoadComplete(nsresult) /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:1043:7
    #48 0x7f8ee24bd63a in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7642:5
    #49 0x7f8ee24b94e4 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7436:7
    #50 0x7f8ee24c0cff in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7333:13
    #51 0x7f8edafbcbc0 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /home/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:1258:3
    #52 0x7f8edafbbb58 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /home/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:842:5
    #53 0x7f8edafb88b6 in nsDocLoader::DocLoaderIsEmpty(bool) /home/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:732:9
    #54 0x7f8edafba9b4 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /home/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:614:5
    #55 0x7f8edafbb56c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /home/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:470:14
    #56 0x7f8ed96dc25b in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /home/worker/workspace/build/src/netwerk/base/nsLoadGroup.cpp:634:18
    #57 0x7f8edc0216ab in nsDocument::DoUnblockOnload() /home/worker/workspace/build/src/dom/base/nsDocument.cpp:8863:7
    #58 0x7f8edc02124b in nsDocument::UnblockOnload(bool) /home/worker/workspace/build/src/dom/base/nsDocument.cpp:8789:9
    #59 0x7f8edbff727c in nsDocument::DispatchContentLoadedEvents() /home/worker/workspace/build/src/dom/base/nsDocument.cpp:5294:3
    #60 0x7f8edc0c7192 in applyImpl<nsDocument, void (nsDocument::*)()> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:855:12
    #61 0x7f8edc0c7192 in apply<nsDocument, void (nsDocument::*)()> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:861
    #62 0x7f8edc0c7192 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, false>::Run() /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:890
    #63 0x7f8ed94e7ab2 in mozilla::ValidatingDispatcher::Runnable::Run() /home/worker/workspace/build/src/xpcom/threads/Dispatcher.cpp:257:21
    #64 0x7f8ed951c632 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1264:7
    #65 0x7f8ed9518ee0 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #66 0x7f8eda32ebff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #67 0x7f8eda2a02a8 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #68 0x7f8eda2a02a8 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #69 0x7f8eda2a02a8 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #70 0x7f8edf7aabaf in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3
    #71 0x7f8ee2fea477 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:871:12
    #72 0x7f8eda2a02a8 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #73 0x7f8eda2a02a8 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #74 0x7f8eda2a02a8 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #75 0x7f8ee2fe9e79 in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:695:7
    #76 0x4e01b6 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:19
    #77 0x4e01b6 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:286
    #78 0x7f8ef4a1082f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291
    #79 0x41c3d8 in _start (/home/nils/fuzzer3/firefox/firefox+0x41c3d8)

0x616000199dc8 is located 584 bytes inside of 616-byte region [0x616000199b80,0x616000199de8)
freed by thread T0 (Web Content) here:
    #0 0x4b2b2b in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3
    #1 0x7f8ed93ba7c4 in SnowWhiteKiller::~SnowWhiteKiller() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2664:9
    #2 0x7f8ed93ba3b6 in nsCycleCollector::FreeSnowWhite(bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2839:3
    #3 0x7f8ed93c1775 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3839:3
    #4 0x7f8ed93c0f30 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3661:9
    #5 0x7f8ed93c3e14 in nsCycleCollector_collect(nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4157:3
    #6 0x7f8edc105b30 in nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1452:3
    #7 0x7f8edbc5e1fd in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsDOMWindowUtils.cpp:1339:3
    #8 0x7f8ed9537d81 in NS_InvokeByIndex /home/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:115
    #9 0x7f8edadd2a87 in Invoke /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:2010:12
    #10 0x7f8edadd2a87 in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1329
    #11 0x7f8edadd2a87 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1296
    #12 0x7f8edadda3fb in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983:12
    #13 0x7f8ee345d67f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #14 0x7f8ee345d67f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #15 0x7f8ee3444172 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #16 0x7f8ee3444172 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2954
    #17 0x7f8ee34297ab in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #18 0x7f8ee345d996 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #19 0x7f8ee345e072 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:512:10
    #20 0x7f8ee3e32fd3 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:2828:12
    #21 0x7f8edad11519 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:319:18
    #22 0x7f8ee345d67f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #23 0x7f8ee345d67f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #24 0x7f8ee3444172 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #25 0x7f8ee3444172 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2954
    #26 0x7f8ee34297ab in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #27 0x7f8ee345d996 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #28 0x7f8ee345e072 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:512:10
    #29 0x7f8ee40dd89c in js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:165:12
    #30 0x7f8ee409452e in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:353:14
    #31 0x7f8ee40bd309 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:464:12
    #32 0x7f8ee40bfc34 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:716:12
    #33 0x7f8ee345d727 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #34 0x7f8ee345d727 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:436
    #35 0x7f8ee345e072 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:512:10
    #36 0x7f8ee3e34d8c 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:2887:12

previously allocated by thread T0 (Web Content) here:
    #0 0x4b2e4b in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3
    #1 0x4e11bd in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7f8edec04339 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12
    #3 0x7f8edec04339 in NS_NewSVGAnimateElement(nsIContent**, already_AddRefed<mozilla::dom::NodeInfo>&&) /home/worker/workspace/build/src/dom/svg/SVGAnimateElement.cpp:10
    #4 0x7f8edec32156 in NS_NewSVGElement(mozilla::dom::Element**, already_AddRefed<mozilla::dom::NodeInfo>&&, mozilla::dom::FromParser) /home/worker/workspace/build/src/dom/svg/SVGElementFactory.cpp:130:19
    #5 0x7f8edc12a6ee in NS_NewElement(mozilla::dom::Element**, already_AddRefed<mozilla::dom::NodeInfo>&&, mozilla::dom::FromParser, nsAString_internal const*) /home/worker/workspace/build/src/dom/base/nsNameSpaceManager.cpp:231:14
    #6 0x7f8edf381810 in nsXMLContentSink::CreateElement(char16_t const**, unsigned int, mozilla::dom::NodeInfo*, unsigned int, nsIContent**, bool*, mozilla::dom::FromParser) /home/worker/workspace/build/src/dom/xml/nsXMLContentSink.cpp:472:8
    #7 0x7f8edf3856f7 in nsXMLContentSink::HandleStartElement(char16_t const*, char16_t const**, unsigned int, unsigned int, bool) /home/worker/workspace/build/src/dom/xml/nsXMLContentSink.cpp:966:12
    #8 0x7f8edb08f7de in nsExpatDriver::HandleStartElement(char16_t const*, char16_t const**) /home/worker/workspace/build/src/parser/htmlparser/nsExpatDriver.cpp:380:19
    #9 0x7f8ee0f96fd1 in doContent /home/worker/workspace/build/src/parser/expat/lib/xmlparse.c:2468:11
    #10 0x7f8ee0f87fcd in contentProcessor /home/worker/workspace/build/src/parser/expat/lib/xmlparse.c:2098:27
    #11 0x7f8ee0f87fcd in doProlog /home/worker/workspace/build/src/parser/expat/lib/xmlparse.c:4078
    #12 0x7f8ee0f7de91 in prologProcessor /home/worker/workspace/build/src/parser/expat/lib/xmlparse.c:3812:10
    #13 0x7f8ee0f7de91 in prologInitProcessor /home/worker/workspace/build/src/parser/expat/lib/xmlparse.c:3629
    #14 0x7f8ee0f7c062 in MOZ_XML_Parse /home/worker/workspace/build/src/parser/expat/lib/xmlparse.c:1530:17
    #15 0x7f8edb0952be in nsExpatDriver::ParseBuffer(char16_t const*, unsigned int, bool, unsigned int*) /home/worker/workspace/build/src/parser/htmlparser/nsExpatDriver.cpp:1014:16
    #16 0x7f8edb09639a in nsExpatDriver::ConsumeToken(nsScanner&, bool&) /home/worker/workspace/build/src/parser/htmlparser/nsExpatDriver.cpp:1112:5
    #17 0x7f8edb0a366b in nsParser::Tokenize(bool) /home/worker/workspace/build/src/parser/htmlparser/nsParser.cpp:1532:16
    #18 0x7f8edb09ef28 in nsParser::ResumeParse(bool, bool, bool) /home/worker/workspace/build/src/parser/htmlparser/nsParser.cpp:1053:41
    #19 0x7f8edb0a47af in nsParser::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) /home/worker/workspace/build/src/parser/htmlparser/nsParser.cpp:1430:12
    #20 0x7f8ed9688b72 in nsBaseChannel::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) /home/worker/workspace/build/src/netwerk/base/nsBaseChannel.cpp:856:17
    #21 0x7f8ed96d4613 in nsInputStreamPump::OnStateTransfer() /home/worker/workspace/build/src/netwerk/base/nsInputStreamPump.cpp:601:22
    #22 0x7f8ed96d300a in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) /home/worker/workspace/build/src/netwerk/base/nsInputStreamPump.cpp:429:25
    #23 0x7f8ed94b9afd in nsInputStreamReadyEvent::Run() /home/worker/workspace/build/src/xpcom/io/nsStreamUtils.cpp:96:9
    #24 0x7f8ed951c632 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1264:7
    #25 0x7f8ed9518ee0 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #26 0x7f8eda32ebff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #27 0x7f8eda2a02a8 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #28 0x7f8eda2a02a8 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #29 0x7f8eda2a02a8 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #30 0x7f8edf7aabaf in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3
    #31 0x7f8ee2fea477 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:871:12
    #32 0x7f8eda2a02a8 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #33 0x7f8eda2a02a8 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #34 0x7f8eda2a02a8 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #35 0x7f8ee2fe9e79 in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:695:7
    #36 0x4e01b6 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:19
    #37 0x4e01b6 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:286

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/dom/smil/nsSMILTargetIdentifier.h:75:38 in Equals
Shadow bytes around the buggy address:
  0x0c2c8002b360: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2c8002b370: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8002b380: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8002b390: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c8002b3a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c2c8002b3b0: fd fd fd fd fd fd fd fd fd[fd]fd fd fd fa fa fa
  0x0c2c8002b3c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2c8002b3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2c8002b3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2c8002b3f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2c8002b400: 00 00 00 00 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
==28440==ABORTING

Comment 1

[Nils]

Attachment: crash.html

Comment 2

[Nils]

Attachment: mini.svg

Comment 3

[Daniel Holbert [:dholbert]]

Looks like there's some syncbase timing in the PoC here. birtles, mind taking a look?

Comment 4

[Brian Birtles (:birtles)]

Yes, I'm looking now (but I'm on a new machine so it's going to take the best part of a day to get an ASAN build going so I can reduce it).

Comment 5

[Brian Birtles (:birtles)]

Looks like I can reproduce on Windows so I don't need an ASAN build after all.

It looks like mAnimationFunctions points to an object that has been freed. mAnimationFunctions is an array of raw pointers to nsSMILAnimationFunction objects which are members of SVGAnimationElement objects. As a result, the SVGAnimationElement object itself must have been freed which is what comment 0 suggests.

Current suspects are nsSMILAnimationController::mAnimationElementTable containing stale pointers to elements, or mLastCompositorTable containing compositors with pointers to their functions.

Comment 6

[Brian Birtles (:birtles)]

Not likely to be mLastCompositorTable. We don't read back function pointers from there (and the elements to references it contains are owning references and they are to the *target* elements, not the animation elements--it's the animation element that is being freed here).

Curiously, mAnimationElementTable appears to be empty at the point when the UAF occurs, whilst currentCompositorTable is not, suggesting the animation element got freed in between populating it and iterating it. That doesn't match the stack in comment 0 however.

Comment 7

[Brian Birtles (:birtles)]

Oh, FlushPendingNotifications runs script! I didn't realize that. In this case its the onresize handler in the test which is triggering garbage collection, which is causing the pointers we collected prior to calling FlushPendingNotifcations to dangle.

Comment 8

[Brian Birtles (:birtles)]

Attachment: reduced-1.svg

Comment 9

[Brian Birtles (:birtles)]

Attachment: reduced-1.html

Comment 10

[Brian Birtles (:birtles)]

Attachment: reduced-1.html

Comment 11

[Brian Birtles (:birtles)]

Attachment 8847468 [details] contains a heavily-annotated reduced test case describing the circumstances that bring about the failure. The referenced SVG file, however, could be reduced further.

The fundamental cause is still simply that we execute script within FlushPendingNotifications. This used to be ok, but bug 814921 moved the point where this is called so that it now is sandwiched between where we store raw pointers in the compositors and where we de-reference them. Not marking this as blocking 814921 yet since I suppose that could alert someone that there is something to be exploited in those patches.

In this particular test case, we actually end up doing nested calls to DoSample on different animation controllers--indeed we actually move the animation elements from one animation controller to another during FlushPendingNotifications, then drop one of the animation elements from the DOM, trigger a restyle causing the other (not-currently-being-iterated) animation controller to drop the last reference to animation element, force cycle collection so it gets deleted, then pop the stack and iterate the raw pointers we gathered from the animation elements in the outer call to DoSample.

Comment 12

[Brian Birtles (:birtles)]

So, as it turns out this is not related to syncbase timing or Element.animate and the reduced test case uses neither of those features.

I haven't started working on a fix yet, and won't get a chance until tomorrow. Perhaps we could do two passes over the mAnimationElementTable. e.g.

  1) Pass #1: Sampling the elements and detecting if we have any compositors at all.
  2) If we don't have any compositors at all, return early.
  3) Flush pending notifications
  4) Pass #2: Iterate over animation elements again and build up compositor table.

That might work. The animation controller itself should be kept alive by the strong ref to the document on the stack. Anyway, I'll come back to this tomorrow.

Comment 13

[Brian Birtles (:birtles)]

Attachment: Crashtest (to be landed after bug has be made public)

Comment 14

[Brian Birtles (:birtles)]

Attachment: Patch approach A

This is a first attempt at fixing this. This simply makes us sample timed elements first, then flush pending restyles if needed, then pick up the compositors.

I've checked that it passes all SMIL crashtests, mochitests, reftests, and layout/style/test/test_restyles_in_smil_animation.html.

That said, I think it might end up being a *lot* simpler to just hold a strong reference to the animation elements while flushing restyles. Perhaps this could be a part of the compositor table, or even just an extra array we put on the stack.

Comment 15

[Brian Birtles (:birtles)]

Attachment: Patch approach B

Daniel, what do you think of this approach?

I've kept the comments and commit message a bit vague so as not to describe the exploit in too much detail but its still pretty obvious.

The raw pointers to functions in the compositors will stay alive beyond the scope of the added array due to their presence in mLastCompositorTable. However, we never dereference animation function pointers in mLastCompositorTable so that should be ok.

Another approach would be to make the compositors keep an owning reference to the animation element whose function they point to and then, at the point when we assign to mLastCompositorTable, iterate over the compositors and call some method that both (a) clears the raw pointer to the animation functions, and (b) drops the owning reference to its animation element. That would be the most robust thing to do so I might try that tomorrow.

For now, this is the simplest possible fix I can think of and probably lowest risk one for uplifting so I thought I'd see what you thought.

Comment 16

[Daniel Holbert [:dholbert]]

Comment on attachment 8847988 [details] [diff] [review]
Patch approach B

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

Thanks for taking this! RE the code-comments -- even in their somewhat-vague formulation here, I'm still a little uneasy with them.  Could you just split the comments into a separate patch, to be landed (maybe with the testcase) later on, once this bug's ready to be opened up?  From a not-giving-away-too-much perspective, I think we should just land the code changes without the comment tweaks.

(The commit message seems appropriately vague/directly-describing-the-code-change, so I think it's fine as-is.)

r=me with that, & one nit noted below:

::: dom/smil/nsSMILAnimationController.cpp
@@ +387,5 @@
>      currentCompositorTable(new nsSMILCompositorTable(0));
> +  // Hold a reference to the animation elements whose animation functions may
> +  // be pointed to by |currentCompositorTable| so that they are guaranteed
> +  // to stay alive even after we call FlushPendingNotifications below.
> +  nsTArray<RefPtr<SVGAnimationElement>> animElems;

You should pre-initialize this array with its capacity that it's going to need (which is mAnimationElementTable.Count()).

I think that'd be:
  nsTArray<RefPtr<SVGAnimationElement>> animElems(mAnimationElementTable.Count());

Comment 17

[Brian Birtles (:birtles)]

Attachment: Fix (applies cleanly to trunk/aurora/beta/release/esr52); r=dholbert

Addressed review comments.

Thanks Daniel!

Comment 18

[Brian Birtles (:birtles)]

Attachment: Fix, backport for ESR45; r=dholbert

Comment 19

[Daniel Holbert [:dholbert]]

Thanks! Updated patch looks good. Remember to request sec-approval before landing.

Comment 20

[Brian Birtles (:birtles)]

Yes, thanks! I'm just waiting to check if the ESR45 backport builds correctly so I can fill out all the fields in the sec-approval form.

Comment 21

[Brian Birtles (:birtles)]

Comment on attachment 8848318 [details] [diff] [review]
Fix (applies cleanly to trunk/aurora/beta/release/esr52); r=dholbert

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
With some effort. It's pretty obvious that certain objects are being destroyed too soon which suggests a use-after-free bug, and it's pretty obvious that those objects get destroyed during a call to FlushPendingNotifications. Producing the right criteria so that the object actually gets destroyed during that call is not trivial but could probably be done fairly quickly with some focused fuzzing.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Not a bulls-eye, but, as described above, it's pretty obvious what is going on.

> Which older supported branches are affected by this flaw?
All branches (introduced by bug 814921 in Firefox 20).

> If not all supported branches, which bug introduced the flaw?
n/a

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I have checked that this patch applies cleanly to aurora, beta, release, and esr52. Attachment 8848327 [details] [diff] is a backport of this patch for ESR45, the code for which is identical--only the context differs.

> How likely is this patch to cause regressions; how much testing does it need?
Highly unlikely. It simply extends the lifetime of certain objects. I have run the relevant SMIL tests locally and they appear to be unaffected.

Comment 22

[Brian Birtles (:birtles)]

Attachment: Comment fixes (to be landed after bug has be made public)

Comment 23

[Al Billings [:abillings - ex-MoCo]]

sec-approval+ for trunk.
We'll want patches nominated for all affected branches including both ESR branches.

Comment 24

[Brian Birtles (:birtles)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/705a77b42fc16df0baf5a770560fb980354a9487

Comment 25

[Brian Birtles (:birtles)]

Comment on attachment 8848318 [details] [diff] [review]
Fix (applies cleanly to trunk/aurora/beta/release/esr52); r=dholbert

Approval Request Comment
[Feature/Bug causing the regression]: Bug 814921
[User impact if declined]: Unpatched use-after-free
[Is this code covered by automated tests?]: The SMIL feature: yes, this specific vulnerability: no. A crashtest is written (attachment 8847970 [details] [diff] [review]) but will not be landed until after this bug is ready to be opened up.
[Has the fix been verified in Nightly?]: No, it has just landed on inbound now (but by the time this request is triaged though it might have hit Nightly).
[Needs manual test from QE? If yes, steps to reproduce]:  Not sure. To verify, simply open attachment 8847970 [details] [diff] [review] in a debug build with the FuzzPriv extension loaded.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It simply extends the lifetime of certain objects.
[String changes made/needed]: None.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: n/a
User impact if declined: Unpatched use-after-free
Fix Landed on Version: Landed on inbound just now
Risk to taking this patch (and alternatives if risky): Minimal -- merely extends the lifetime of certain objects.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 26

[Brian Birtles (:birtles)]

Comment on attachment 8848327 [details] [diff] [review]
Fix, backport for ESR45; r=dholbert

See comment 25.

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/705a77b42fc1

Comment 28

[Gerry Chang [:gchang]]

Comment on attachment 8848318 [details] [diff] [review]
Fix (applies cleanly to trunk/aurora/beta/release/esr52); r=dholbert

Fix a sec-critical. Aurora54+ & Beta53+.

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/476badc4efc5

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c5cc62e792d9

Comment 31

[Julien Cristau [:jcristau]]

Comment on attachment 8848318 [details] [diff] [review]
Fix (applies cleanly to trunk/aurora/beta/release/esr52); r=dholbert

sec-critical uaf fix for esr52

Comment 32

[Julien Cristau [:jcristau]]

Comment on attachment 8848327 [details] [diff] [review]
Fix, backport for ESR45; r=dholbert

sec-critical fix for esr45

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/b4fc7a4cb5e0
https://hg.mozilla.org/releases/mozilla-esr45/rev/5a6390274b64

Comment 34

[Andrei Vaida [:avaida]]

Flagging this for manual testing, per Comment 25.

Comment 35

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8848318 [details] [diff] [review]
Fix (applies cleanly to trunk/aurora/beta/release/esr52); r=dholbert

This is already on 53 (beta and release branches).

Comment 36

[Emil Ghitta, QA [:emilghitta]]

I’ve reproduced the issue mentioned in comment 0 with Firefox 52.0.2 (Build Id:20170323174935, asan-debug) and I have verified that this issue is not reproducible on Firefox 53.0 (Build Id:20170412124643,asan-debug),  Firefox 54.0a2 (Build Id:20170412094440,asan-debug), Firefox 55.a1 (Build Id:20170412031733,asan-debug)and Firefox 52.1.0esr (Build Id:20170412124823,asan-debug) using Ubuntu 16.04 64bit.

Brian, It seems that the FuzzPriv extension is not compatible with Firefox 45.9.0(Build Id:20170411115307,debug),can you please provide me a workaround for this encountered problem?

Thanks!

Comment 37

[Brian Birtles (:birtles)]

You can just run attachment 8847970 [details] [diff] [review] as a crashtest.

Comment 38

[Emil Ghitta, QA [:emilghitta]]

Hi Brian

I tried to retest this issue with an affected Firefox 52.0.2 (Build Id:20170323174935, asan-debug) build, but the issue is not reproducible without the fuzzPriv extension.

Since the issue can only be reproduced with the fuzzPriv extension installed, I can't really tell if this is fixed or not on 45.9esr, as I can't install fuzzPriv there.

Comment 39

[Brian Birtles (:birtles)]

If you run the crashtest as a crashtest, you don't need the fuzzPriv extension. You'll need to apply attachment 8847970 [details] [diff] [review] to the build you're trying to test, then run './mach crashtest dom/smil/crashtests/1347168-1.html'. If you want to find a fuzzPriv extension to install you'll need to ask someone else. I'm afraid I don't know anything about it.