Closed Bug 1334876 (CVE-2017-5402) Opened 8 years ago Closed 8 years ago

heap-use-after-free in mozilla::dom::FontFaceSet::DispatchLoadingFinishedEvent

Categories

(Core :: DOM: CSS Object Model, defect)

Product:
Core
Component:
DOM: CSS Object Model
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox-esr45 52+ fixed
firefox51 --- wontfix
firefox52 + fixed
firefox-esr52 --- fixed
firefox53 + fixed
firefox54 + verified

People

(Reporter: nils, Assigned: heycam)

References

Details

(4 keywords, Whiteboard: [post-critsmash-triage][adv-main52+][adv-esr45.8+])

Attachments

(1 file, 4 obsolete files)

patch v1.2 sec-approval=abillings r=bz
6.66 KB, patch
jcristau
: approval-mozilla-aurora+
jcristau
: approval-mozilla-beta+
jcristau
: approval-mozilla-esr45+
Details | Diff | Splinter Review
The following testcase crashes the latest ASAN build of Firefox. <script> function start() { o5=document.createElement('canvas').getContext('2d'); o78=document.createElement('style'); o79=document.createTextNode("@font-face{ font-family: font0; src: url('a') format('eot'"); o78.appendChild(o79); document.documentElement.appendChild(o78); o118=document.fonts; o132=document.createElement('style'); document.documentElement.appendChild(o132); o118.onloadingdone=fun1; o5.font=' smaller font0'; o5.fillText("a",10,10); o221=document.createElement('iframe'); document.documentElement.appendChild(o221); o264=document.createRange(); o264.selectNode(o132); } function fun1() { try{o264.surroundContents(o79);}catch(E){} document.documentElement.offsetHeight; fuzzPriv.CC(); } </script> <body onload="start()"></body> ================================================================= ==25681==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d00004a2a8 at pc 0x7f6ced5e7ee1 bp 0x7ffdb0fbae00 sp 0x7ffdb0fbadf8 READ of size 8 at 0x60d00004a2a8 thread T0 (Web Content) #0 0x7f6ced5e7ee0 in GetThread /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:58:36 #1 0x7f6ced5e7ee0 in AddRef /home/worker/workspace/build/src/layout/style/FontFace.cpp:98 #2 0x7f6ced5e7ee0 in AddRef /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:37 #3 0x7f6ced5e7ee0 in AddRef /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:391 #4 0x7f6ced5e7ee0 in RefPtr<mozilla::dom::FontFace>::assign_with_AddRef(mozilla::dom::FontFace*) /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:54 #5 0x7f6ced5e323a in operator= /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:191:5 #6 0x7f6ced5e323a in init<mozilla::dom::FontFace *&> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/OwningNonNull.h:147 #7 0x7f6ced5e323a in operator= /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/OwningNonNull.h:69 #8 0x7f6ced5e323a in mozilla::dom::FontFaceSet::DispatchLoadingFinishedEvent(nsAString_internal const&, nsTArray<mozilla::dom::FontFace*> const&) /home/worker/workspace/build/src/layout/style/FontFaceSet.cpp:1657 #9 0x7f6ced5d802a in mozilla::dom::FontFaceSet::CheckLoadingFinished() /home/worker/workspace/build/src/layout/style/FontFaceSet.cpp:1641:5 #10 0x7f6ced906279 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1874:11 #11 0x7f6ced913d75 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:295:7 #12 0x7f6ced913a44 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:317:5 #13 0x7f6ced915a84 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /home/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:506:9 #14 0x7f6cee189084 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /home/worker/workspace/build/src/layout/ipc/VsyncChild.cpp:64:5 #15 0x7f6ce82aa407 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:160:20 #16 0x7f6ce7f1ec37 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1425:16 #17 0x7f6ce7e656d0 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1781:14 #18 0x7f6ce7e61c0c in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1716:17 #19 0x7f6ce7e64244 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1589:5 #20 0x7f6ce7e6488e in mozilla::ipc::MessageChannel::MessageTask::Run() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1622:5 #21 0x7f6ce706695b in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1261:7 #22 0x7f6ce70632b0 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:394:10 #23 0x7f6ce7e6d5df in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21 #24 0x7f6ce7ddf868 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3 #25 0x7f6ce7ddf868 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231 #26 0x7f6ce7ddf868 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211 #27 0x7f6ced23481f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3 #28 0x7f6cef42e4d7 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:927:12 #29 0x7f6ce7ddf868 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3 #30 0x7f6ce7ddf868 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231 #31 0x7f6ce7ddf868 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211 #32 0x7f6cef42dcfd in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:759:7 #33 0x4df933 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:115:19 #34 0x4df933 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:284 #35 0x7f6d0243482f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291 #36 0x41ba58 in _start (/home/nils/fuzzer3/firefox/firefox+0x41ba58) 0x60d00004a2a8 is located 40 bytes inside of 144-byte region [0x60d00004a280,0x60d00004a310) freed by thread T0 (Web Content) here: #0 0x4b21ab in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3 #1 0x7f6ce6f0e744 in SnowWhiteKiller::~SnowWhiteKiller() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2664:9 #2 0x7f6ce6f0e336 in nsCycleCollector::FreeSnowWhite(bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2839:3 #3 0x7f6ce6f15506 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3825:3 #4 0x7f6ce6f14cdc in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3650:9 #5 0x7f6ce6f17d56 in nsCycleCollector_collect(nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4143:3 #6 0x7f6ce9b32229 in nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1438:3 #7 0x7f6ce968882d in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsDOMWindowUtils.cpp:1339:3 #8 0x7f6ce7080be1 in NS_InvokeByIndex /home/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:115 #9 0x7f6ce8849b3e in Invoke /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:2010:12 #10 0x7f6ce8849b3e in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1329 #11 0x7f6ce8849b3e in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1296 #12 0x7f6ce885166b in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983:12 #13 0x7f6cf0eb731c in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:239:15 #14 0x7f6cf0eb731c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:460 #15 0x7f6cf0e9d0ae in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:511:12 #16 0x7f6cf0e9d0ae in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2957 #17 0x7f6cf0e80fa1 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:406:12 #18 0x7f6cf0eb759c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:478:15 #19 0x7f6cf0eb7c52 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:524:10 #20 0x7f6cf16ef4f2 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:2788:12 #21 0x7f6ce878521f in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:319:18 #22 0x7f6cf0eb731c in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:239:15 #23 0x7f6cf0eb731c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:460 #24 0x7f6cf0e9d0ae in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:511:12 #25 0x7f6cf0e9d0ae in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2957 #26 0x7f6cf0e80fa1 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:406:12 #27 0x7f6cf0eb759c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:478:15 #28 0x7f6cf0eb7c52 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:524:10 #29 0x7f6cf16f175d 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:2847:12 #30 0x7f6ceafc986f 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 0x7f6ceb8eecf7 in Call<nsISupports *> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:361:12 #32 0x7f6ceb8eecf7 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /home/worker/workspace/build/src/dom/events/JSEventHandler.cpp:214 #33 0x7f6ceb8b8e6d in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1136:16 #34 0x7f6ceb8ba9ec in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1318:20 #35 0x7f6ceb8a5433 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:465:5 #36 0x7f6ceb8a8d24 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 previously allocated by thread T0 (Web Content) here: #0 0x4b24cb in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3 #1 0x4e09ad in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17 #2 0x7f6ced5c9046 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12 #3 0x7f6ced5c9046 in mozilla::dom::FontFace::CreateForRule(nsISupports*, mozilla::dom::FontFaceSet*, nsCSSFontFaceRule*) /home/worker/workspace/build/src/layout/style/FontFace.cpp:150 #4 0x7f6ced5db9b4 in mozilla::dom::FontFaceSet::UpdateRules(nsTArray<nsFontFaceRuleContainer> const&) /home/worker/workspace/build/src/layout/style/FontFaceSet.cpp:718:11 #5 0x7f6ce9a72d47 in nsIDocument::FlushUserFontSet() /home/worker/workspace/build/src/dom/base/nsDocument.cpp:12853:19 #6 0x7f6ce9a732cf in GetUserFontSet /home/worker/workspace/build/src/dom/base/nsDocument.cpp:12811:5 #7 0x7f6ce9a732cf in nsIDocument::Fonts() /home/worker/workspace/build/src/dom/base/nsDocument.cpp:12909 #8 0x7f6ceaf4e288 in mozilla::dom::DocumentBinding::get_fonts(JSContext*, JS::Handle<JSObject*>, nsIDocument*, JSJitGetterCallArgs) /home/worker/workspace/build/src/obj-firefox/dom/bindings/DocumentBinding.cpp:5336:57 #9 0x7f6ceb46a72d in mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2849:13 #10 0x7f6cf0eb731c in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:239:15 #11 0x7f6cf0eb731c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:460 #12 0x7f6cf0eb88fe in InternalCall /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:505:12 #13 0x7f6cf0eb88fe in Call /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:524 #14 0x7f6cf0eb88fe in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:638 #15 0x7f6cf1c5d907 in CallGetter /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1812:16 #16 0x7f6cf1c5d907 in GetExistingProperty<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1860 #17 0x7f6cf1c5d907 in NativeGetPropertyInline<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2089 #18 0x7f6cf1c5d907 in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2123 #19 0x7f6cf16eb71c in GetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1497:12 #20 0x7f6cf16eb71c in JS_ForwardGetPropertyTo(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2534 #21 0x7f6ceb46244f in mozilla::dom::GetPropertyOnPrototype(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, bool*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2036:10 #22 0x7f6ceb178cdc in mozilla::dom::HTMLDocumentBinding::DOMProxyHandler::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) const /home/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:2111:8 #23 0x7f6cf199460e in js::Proxy::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:311:12 #24 0x7f6cf0ec00ca in GetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1496:16 #25 0x7f6cf0ec00ca in GetProperty /home/worker/workspace/build/src/js/src/jsobj.h:844 #26 0x7f6cf0ec00ca 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:4311 #27 0x7f6cf0ea05d4 in GetPropertyOperation /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:192:12 #28 0x7f6cf0ea05d4 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2674 #29 0x7f6cf0e80fa1 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:406:12 #30 0x7f6cf0eb759c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:478:15 #31 0x7f6cf0eb7c52 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:524:10 #32 0x7f6cf16f175d 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:2847:12 #33 0x7f6ceafc986f 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 #34 0x7f6ceb8eecf7 in Call<nsISupports *> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:361:12 #35 0x7f6ceb8eecf7 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /home/worker/workspace/build/src/dom/events/JSEventHandler.cpp:214 #36 0x7f6ceb8b8e6d in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1136:16 #37 0x7f6ceb8ba9ec in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1318:20 #38 0x7f6ceb8a5433 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:465:5 #39 0x7f6ceb8a8d24 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 #40 0x7f6ceda6ace9 in nsDocumentViewer::LoadComplete(nsresult) /home/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:1044:7 #41 0x7f6cee91c41e in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7628:5 #42 0x7f6cee918354 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /home/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7432:7 SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:58:36 in GetThread Shadow bytes around the buggy address: 0x0c1a80001400: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c1a80001410: fd fd fd fd fa fa fa fa fa fa fa fa fd fd fd fd 0x0c1a80001420: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa 0x0c1a80001430: fa fa fa fa fa fa 00 00 00 00 00 00 00 00 00 00 0x0c1a80001440: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa =>0x0c1a80001450: fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd 0x0c1a80001460: fd fd fa fa fa fa fa fa fa fa 00 00 00 00 00 00 0x0c1a80001470: 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa 0x0c1a80001480: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd 0x0c1a80001490: fd fd fd fd fd fd fa fa fa fa fa fa fa fa 00 00 0x0c1a800014a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==25681==ABORTING
Jonathan, could you look at this? There's a lot of font stuff in the stack. Thanks.
Flags: needinfo?(jfkthame)
Keywords: csectype-uaf, sec-critical
Probably the same underlying issue as bug 1334823; ni?heycam.
Flags: needinfo?(jfkthame) → needinfo?(cam)
Group: core-security → dom-core-security
FontFaceSet::DispatchLoadingFinishedEvent potentially dispatches two events: loadingdone and loadingerror. The function holds raw pointers to FontFace objects in local variables. We dispatch the loadingdone event with (new AsyncEventDispatcher(...))->RunDOMEventWhenSafe(), which decides to run it immediately. The comment 0 test causes the FontFace object to be destroyed during the event handler, and then we proceed to use those stale raw pointers when dispatching the loadingerror event.
Assignee: nobody → cam
Flags: needinfo?(cam)
This goes back to the initial implementation of the Font Loading API in bug 1028497, which was enabled by default in release builds in Firefox 40 in bug 1088437.
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox-esr45: --- → affected
status-firefox-esr52: --- → affected
Attached patch patch (obsolete) — — Splinter Review
This is one way to solve it, by always going through the event loop to dispatch the event. Another would be to make the loaded/failed array local variables be nsTArray<RefPtr<FontFace>>.
Attachment #8832762 - Flags: review?(bzbarsky)
Comment on attachment 8832762 [details] [diff] [review] patch I would actually really prefer we do _both_. Or at the very least just the strong refs. Relying on the event dispatch behavior is kinda fragile.... r=me either way, but please do make those be arrays of RefPtr. (Or I guess arrays of OwningNonNull, in which case you may be able to just SwapElements them into init.mFontFaces, if you also pass as non-const ref.)
Attachment #8832762 - Flags: review?(bzbarsky) → review+
Attached patch patch v2 r=bz (obsolete) — — Splinter Review
Attachment #8832762 - Attachment is obsolete: true
Comment on attachment 8832800 [details] [diff] [review] patch v2 r=bz [Security approval request comment] How easily could an exploit be constructed based on the patch? From the patch, it's not too hard to see that it is an issue surrounding FontFace objects being destroyed too early. But it would be harder to turn the UAF into something exploitable. 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? All. If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? I don't, and there are some small changes around here on some older branches, so this patch won't apply cleanly. But it should be easy to manually apply the patch. How likely is this patch to cause regressions; how much testing does it need? Pretty low risk of regressions. The existing test_font_loading_api.html tests should exercise this code sufficiently to make sure we didn't break it.
Attachment #8832800 - Flags: sec-approval?
> From the patch, it's not too hard to see that it is an issue surrounding FontFace objects being destroyed Note that if we want to hide that we could check in the initial patch, trunk and branches, in this bug, and have a followup bug with some summary about "reducing copying" or whatnot for the other part, landed trunk-only.
Comment on attachment 8832800 [details] [diff] [review] patch v2 r=bz sec-approval+ for trunk. We'll want patches for Aurora, Beta, and ESR45 as well.
Attachment #8832800 - Flags: sec-approval? → sec-approval+
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #11) > Note that if we want to hide that we could check in the initial patch, trunk > and branches, in this bug, and have a followup bug with some summary about > "reducing copying" or whatnot for the other part, landed trunk-only. Sounds good to me.
Here's the first patch with reduced context, which makes it apply to all branches.
Attachment #8832800 - Attachment is obsolete: true
Comment on attachment 8833223 [details] [diff] [review] patch v1 r=bz sec-approval=abillings Approval Request Comment [Feature/Bug causing the regression]: bug 1028497 [User impact if declined]: script can cause a UAF [Is this code covered by automated tests?]: the font face event dispatching code in general is, the patch doesn't have an automated test for the issue though [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: limited change to cause an event to be dispatched slightly later [String changes made/needed]: none I tested locally that this patch works on ESR45/beta/aurora/central.
Attachment #8833223 - Flags: approval-mozilla-beta?
Attachment #8833223 - Flags: approval-mozilla-aurora?
Comment on attachment 8833223 [details] [diff] [review] patch v1 r=bz sec-approval=abillings [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: script can cause a UAF Fix Landed on Version: not landed yet... Risk to taking this patch (and alternatives if risky): risk is low, the patch is quite localized, and we have automated tests for font load events String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8833223 - Flags: approval-mozilla-esr45?
Ah, running test_font_loading_api.html locally results in a failure with this patch. If some test tweaks are needed it shouldn't affect the approval requests though.
Carrying over the sec-approval, but want a quick r? on the test change, which makes us wait for the events properly.
Attachment #8833223 - Attachment is obsolete: true
Attachment #8833223 - Flags: approval-mozilla-esr45?
Attachment #8833223 - Flags: approval-mozilla-beta?
Attachment #8833223 - Flags: approval-mozilla-aurora?
Attachment #8833234 - Flags: approval-mozilla-esr45?
Attachment #8833234 - Flags: approval-mozilla-beta?
Attachment #8833234 - Flags: approval-mozilla-aurora?
Attachment #8833234 - Flags: review?(bzbarsky)
OK, so the test change shows that this changes observable behavior, so I went to see what the spec says. I _think_ the relevant spec is at https://drafts.csswg.org/css-font-loading/#switch-the-fontfaceset-to-loaded and it says to do the following in order: 1) Fulfill the [[ReadyPromise]] 2) Queue a task to fire the two events. So per spec, the promise's handlers should in fact run before the event listeners do, and thus our old behavior was buggy and the new one is closer to the spec. Our code doesn't match the spec in one respect, though. It does https://drafts.csswg.org/css-font-loading/#switch-the-fontfaceset-to-loaded step 5 substeps 1-3 synchronously, not off a task. I believe this _is_ observable. For example, say one of the promise handlers calls delete() or clear() on the font face set. Per spec, that should affect the lists of fonts the events have hanging off them, but in our implementation I'm pretty sure it does not. I filed https://github.com/w3c/csswg-drafts/issues/1003 because I think the spec is wrong here. But we should get that sorted out and write more tests (ideally of the web platform variety).
Comment on attachment 8833234 [details] [diff] [review] patch v1.1 sec-approval=abillings r=me
Attachment #8833234 - Flags: review?(bzbarsky) → review+
Comment on attachment 8833234 [details] [diff] [review] patch v1.1 sec-approval=abillings Fix for sec-critical issue, closer to spec, let's uplift it. This should make it into beta 5 later this week.
Attachment #8833234 - Flags: approval-mozilla-esr45?
Attachment #8833234 - Flags: approval-mozilla-esr45+
Attachment #8833234 - Flags: approval-mozilla-beta?
Attachment #8833234 - Flags: approval-mozilla-beta+
Attachment #8833234 - Flags: approval-mozilla-aurora?
Attachment #8833234 - Flags: approval-mozilla-aurora+
The v1.1 patch passed locally for me but not on the build machines it seems. I tweaked the sub-tests that check for events to make them wait long enough for any events that may be dispatched from previous events before proceeding. Carrying over r+ and sec-approval+, thought it looks like I can't copy over the branch approval flags myself. Looks OK on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=df7770426027112cfb38e558a97e770531d7d1f5&group_state=expanded
Attachment #8833234 - Attachment is obsolete: true
Attachment #8834289 - Flags: approval-mozilla-esr45?
Attachment #8834289 - Flags: approval-mozilla-beta?
Attachment #8834289 - Flags: approval-mozilla-aurora?
(In reply to Cameron McCormack (:heycam) from comment #24) > The v1.1 patch passed locally for me but not on the build machines it seems. > I tweaked the sub-tests that check for events to make them wait long enough > for any events that may be dispatched from previous events before > proceeding. from previous sub-tests
Comment on attachment 8834289 [details] [diff] [review] patch v1.2 sec-approval=abillings r=bz carrying over branch approvals from previous patch revision sec-crit fix for aurora53, beta52, esr45
Attachment #8834289 - Flags: approval-mozilla-esr45?
Attachment #8834289 - Flags: approval-mozilla-esr45+
Attachment #8834289 - Flags: approval-mozilla-beta?
Attachment #8834289 - Flags: approval-mozilla-beta+
Attachment #8834289 - Flags: approval-mozilla-aurora?
Attachment #8834289 - Flags: approval-mozilla-aurora+
(In reply to Ryan VanderMeulen [:RyanVM] from comment #28) > This got backed out from inbound for test failures. > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > 4dc3eb7f7b1284459eafc16d1fa0aecc9b2ff385 Note this is the backout of the v1.1 patch (mentioned in comment 23). The v1.2 patch (comment 27) stuck.
Group: dom-core-security → core-security-release
Flags: qe-verify+
Flags: sec-bounty?
Blocks: 1028497
Flags: sec-bounty? → sec-bounty+
Keywords: regression
(In reply to Ryan VanderMeulen [:RyanVM] from comment #31) > https://hg.mozilla.org/releases/mozilla-esr52/rev/ef1f752b6277 Why did we explicitly take this on ESR52 when we haven't made a final normal 52 release yet? Have we already forked for ESR52 and we're hand porting changes? That seems wrong and odd?
Flags: needinfo?(ryanvm)
The ESR52 branch was created by gps roughly two weeks ago. At this point, it's completely identical to Beta and I've been keeping it in sync. Note the identical commit hashes.
Flags: needinfo?(ryanvm)
I was unable to reproduce this issue on affected builds. I've used the testcase from Comment 0 on Ubuntu 16.04 x64, with the following builds from Jan 29: - ASAN 54.0a1, 20170129185945 - ASAN 54.0a1, 20170129170237 Nils, am I missing something here?
Flags: needinfo?(nils)
Andrei, are you using the fuzzPriv extension? ( https://github.com/MozillaSecurity/funfuzz/tree/master/dom/extension )
Flags: needinfo?(nils)
(In reply to Nils from comment #35) > Andrei, are you using the fuzzPriv extension? ( > https://github.com/MozillaSecurity/funfuzz/tree/master/dom/extension ) Agh, of course. I was not using fuzzPriv, thank you Nils. Reproduced the issue with an affected build (ASAN 54.0a1, 20170129185945) and fuzzPriv installed. This is verified fixed on a recent ASAN build (54.0a1, 20170222230118) using Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
Flags: qe-verify+
tracking-firefox-esr52: ? → ---
Whiteboard: [post-critsmash-triage][adv-main52+][adv-esr45.8+]
Alias: CVE-2017-5402
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: