Closed Bug 1610880 (CVE-2020-6805) Opened 2 years ago Closed 2 years ago

heap-use-after-free (READ of size 8) in mozilla::dom::quota::QuotaObject::LockedMaybeUpdateSize

Categories

(Core :: Storage: Quota Manager, defect, P1)

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla75
Tracking Status
firefox-esr68 74+ verified
firefox73 --- wontfix
firefox74 + verified
firefox75 + verified

People

(Reporter: geeknik, Assigned: tt)

References

Details

(5 keywords, Whiteboard: [adv-main74+][adv-esr68.6+])

Attachments

(4 files, 1 obsolete file)

Whilst browsing the web using Firefox Nightly (ASAN built from https://hg.mozilla.org/mozilla-central/rev/be3a05f615a557fd4c5171f789cc460688d9c3b8), the browser crashed. I had 38 tabs open, it's hard to say which one triggered this.

==323834==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f0007f8568 at pc 0x7f846b0a505b bp 0x7f84412f28b0 sp 0x7f84412f28a8
READ of size 8 at 0x60f0007f8568 thread T40 (IPDL Background)
    #0 0x7f846b0a505a in mozilla::dom::quota::QuotaObject::LockedMaybeUpdateSize(long, bool) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3285:39
    #1 0x7f846b0a3084 in mozilla::dom::quota::QuotaObject::MaybeUpdateSize(long, bool) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3226:10
    #2 0x7f846bf31100 in UpdateUsage /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:5503:24
    #3 0x7f846bf31100 in NoteInactiveDatabase /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:4941:28
    #4 0x7f846bf31100 in UnregisterSnapshot /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:5680:15
    #5 0x7f846bf31100 in mozilla::dom::(anonymous namespace)::Snapshot::Finish() /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:5965:14
    #6 0x7f846bf2ede9 in mozilla::dom::(anonymous namespace)::Snapshot::RecvFinish() /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:6122:3
    #7 0x7f8465419a32 in mozilla::dom::PBackgroundLSSnapshotParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundLSSnapshotParent.cpp:215:28
    #8 0x7f8465435cbd in mozilla::ipc::PBackgroundParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundParent.cpp:3522:32
    #9 0x7f8464afc206 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2214:25
    #10 0x7f8464af8502 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2136:9
    #11 0x7f8464afa0f9 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1975:3
    #12 0x7f8464afa727 in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2006:13
    #13 0x7f8463999d59 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1220:14
    #14 0x7f84639a3281 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #15 0x7f8464b049a3 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:332:5
    #16 0x7f8464a283e2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #17 0x7f8464a283e2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #18 0x7f8464a283e2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #19 0x7f8463993736 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:464:10
    #20 0x7f847a1db288 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #21 0x7f847d4464e1 in start_thread (/lib64/libpthread.so.0+0x94e1)
    #22 0x7f847d007692 in clone (/lib64/libc.so.6+0x101692)

0x60f0007f8568 is located 120 bytes inside of 168-byte region [0x60f0007f84f0,0x60f0007f8598)
freed by thread T46 (QuotaManager IO) here:
    #0 0x561eab6d592d in free /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:123:3
    #1 0x7f846b1283d1 in operator delete /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/cxxalloc.h:51:10
    #2 0x7f846b1283d1 in Release /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:875:3
    #3 0x7f846b1283d1 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:50:40
    #4 0x7f846b1283d1 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:379:36
    #5 0x7f846b1283d1 in ~RefPtr /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:81:7
    #6 0x7f846b1283d1 in Destruct /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:547:45
    #7 0x7f846b1283d1 in DestructRange /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2240:7
    #8 0x7f846b1283d1 in RemoveElementsAtUnsafe /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2299:3
    #9 0x7f846b1283d1 in nsTArray_Impl<RefPtr<mozilla::dom::quota::OriginInfo>, nsTArrayInfallibleAllocator>::RemoveElementsAt(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2293:3
    #10 0x7f846b0a6deb in mozilla::dom::quota::QuotaManager::LockedRemoveQuotaForOrigin(mozilla::dom::quota::PersistenceType, nsTSubstring<char> const&, nsTSubstring<char> const&) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:7472:16
    #11 0x7f846b10f234 in RemoveQuotaForOrigin /builds/worker/workspace/build/src/dom/quota/QuotaManager.h:227:5
    #12 0x7f846b10f234 in mozilla::dom::quota::(anonymous namespace)::ClearRequestBase::DeleteFiles(mozilla::dom::quota::QuotaManager*, mozilla::dom::quota::PersistenceType) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:9705:24
    #13 0x7f846b10cdf1 in mozilla::dom::quota::(anonymous namespace)::ClearRequestBase::DoDirectoryWork(mozilla::dom::quota::QuotaManager*) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp
    #14 0x7f846b0f5c3a in DirectoryWork /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:8266:8
    #15 0x7f846b0f5c3a in mozilla::dom::quota::(anonymous namespace)::OriginOperationBase::Run() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:8164:12
    #16 0x7f8463999d59 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1220:14
    #17 0x7f84639a3281 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #18 0x7f8464b0485a in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:302:20
    #19 0x7f8464a283e2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #20 0x7f8464a283e2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #21 0x7f8464a283e2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #22 0x7f8463993736 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:464:10
    #23 0x7f847a1db288 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #24 0x7f847d4464e1 in start_thread (/lib64/libpthread.so.0+0x94e1)

previously allocated by thread T46 (QuotaManager IO) here:
    #0 0x561eab6d5bad in malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:145:3
    #1 0x561eab70aefd in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f846b0ae3d1 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7f846b0ae3d1 in mozilla::dom::quota::QuotaManager::EnsureQuotaForOrigin(mozilla::dom::quota::PersistenceType, nsTSubstring<char> const&, nsTSubstring<char> const&) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:4038:18
    #4 0x7f846bf3793a in DatabaseWork /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:7312:19
    #5 0x7f846bf3793a in mozilla::dom::(anonymous namespace)::PrepareDatastoreOp::NestedRun() /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp:7805:12
    #6 0x7f846bf343d3 in mozilla::dom::(anonymous namespace)::LSRequestBase::Run() /builds/worker/workspace/build/src/dom/localstorage/ActorsParent.cpp
    #7 0x7f8463999d59 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1220:14
    #8 0x7f84639a3281 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #9 0x7f8464b049a3 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:332:5
    #10 0x7f8464a283e2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #11 0x7f8464a283e2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #12 0x7f8464a283e2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #13 0x7f8463993736 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:464:10
    #14 0x7f847a1db288 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #15 0x7f847d4464e1 in start_thread (/lib64/libpthread.so.0+0x94e1)

Thread T40 (IPDL Background) created by T0 here:
    #0 0x561eab6c033a in pthread_create /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:209:3
    #1 0x7f847a1c9663 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:458:14
    #2 0x7f847a1b368e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:533:12
    #3 0x7f8463995d33 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:670:8
    #4 0x7f84639a2461 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:621:12
    #5 0x7f84639a5f43 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:139:57
    #6 0x7f8464abed79 in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:69:10
    #7 0x7f8464abed79 in (anonymous namespace)::ParentImpl::CreateBackgroundThread() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:943:7
    #8 0x7f8464a89125 in CreateActorForSameProcess /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:853:32
    #9 0x7f8464a89125 in GetOrCreateForCurrentThread /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1455:9
    #10 0x7f8464a89125 in mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread(nsIEventTarget*) /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:657:10
    #11 0x7f8469e3c3d7 in mozilla::dom::ClientManager::ClientManager() /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:50:7
    #12 0x7f8469e3f180 in mozilla::dom::ClientManager::GetOrCreateForCurrentThread() /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:208:14
    #13 0x7f8469e30cea in mozilla::dom::ClientManager::CreateSource(mozilla::dom::ClientType, nsISerialEventTarget*, nsIPrincipal*) /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:264:31
    #14 0x7f846f3bb43a in nsDocShell::MaybeCreateInitialClientSource(nsIPrincipal*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:2504:26
    #15 0x7f846f3ef16f in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIPrincipal*, nsIContentSecurityPolicy*, nsIURI*, bool, bool, mozilla::dom::WindowGlobalChild*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6597:5
    #16 0x7f846f3f033c in CreateAboutBlankContentViewer /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:6655:10
    #17 0x7f846f3f033c in non-virtual thunk to nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIPrincipal*, nsIContentSecurityPolicy*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp
    #18 0x7f846f4b753d in mozilla::AppWindow::Initialize(nsIAppWindow*, nsIAppWindow*, nsIURI*, int, int, bool, nsIRemoteTab*, mozIDOMWindowProxy*, nsWidgetInitData&) /builds/worker/workspace/build/src/xpfe/appshell/AppWindow.cpp:297:21
    #19 0x7f846f4dcec3 in nsAppShellService::JustCreateTopWindow(nsIAppWindow*, nsIURI*, unsigned int, int, int, bool, nsIRemoteTab*, mozIDOMWindowProxy*, mozilla::AppWindow**) /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp:668:25
    #20 0x7f846f4de36d in nsAppShellService::CreateTopLevelWindow(nsIAppWindow*, nsIURI*, unsigned int, int, int, nsIRemoteTab*, mozIDOMWindowProxy*, nsIAppWindow**) /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp:172:8
    #21 0x7f846fdc7073 in nsAppStartup::CreateChromeWindow(nsIWebBrowserChrome*, unsigned int, nsIRemoteTab*, mozIDOMWindowProxy*, unsigned long, bool*, nsIWebBrowserChrome**) /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:629:15
    #22 0x7f846ff9629d in CreateChromeWindow /builds/worker/workspace/build/src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:419:33
    #23 0x7f846ff9629d in nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy*, char const*, char const*, char const*, bool, bool, bool, nsIArray*, bool, bool, bool, nsDocShellLoadState*, mozilla::dom::BrowsingContext**) /builds/worker/workspace/build/src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:904:12
    #24 0x7f846ff93822 in nsWindowWatcher::OpenWindow(mozIDOMWindowProxy*, char const*, char const*, char const*, nsISupports*, mozIDOMWindowProxy**) /builds/worker/workspace/build/src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:292:3
    #25 0x7f84639d6351 in NS_InvokeByIndex /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
    #26 0x7f846572be24 in Invoke /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1643:10
    #27 0x7f846572be24 in Call /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1184:19
    #28 0x7f846572be24 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1150:23
    #29 0x7f8465731ec4 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:947:10
    #30 0x7f84702b8c9e in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:452:13
    #31 0x7f84702b8c9e in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:544:12
    #32 0x7f84702a1e1d in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:612:10
    #33 0x7f84702a1e1d in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3021:16
    #34 0x7f8470284cb5 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:424:10
    #35 0x7f84702b95d8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:580:13
    #36 0x7f84702bb5a9 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:625:8
    #37 0x7f847045637b 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:2734:10
    #38 0x7f846571c568 in nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:956:17
    #39 0x7f84639d79e1 in PrepareAndDispatch /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:125:37
    #40 0x7f84639d68ea in SharedStub (/home/geeknik/firefox/libxul.so+0x22fd8ea)
    #41 0x7f846fa7d5e6 in nsCommandLine::EnumerateHandlers(nsresult (*)(nsICommandLineHandler*, nsICommandLine*, void*), void*) /builds/worker/workspace/build/src/toolkit/components/commandlines/nsCommandLine.cpp:448:10
    #42 0x7f846fa7eb2a in nsCommandLine::Run() /builds/worker/workspace/build/src/toolkit/components/commandlines/nsCommandLine.cpp:503:8
    #43 0x7f847003ef37 in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4537:19
    #44 0x7f84700415ee in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4740:8
    #45 0x7f8470042a10 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4821:21
    #46 0x561eab708511 in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:217:22
    #47 0x561eab708511 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:339:16
    #48 0x7f847cf2d1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

Thread T46 (QuotaManager IO) created by T40 (IPDL Background) here:
    #0 0x561eab6c033a in pthread_create /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:209:3
    #1 0x7f847a1c9663 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:458:14
    #2 0x7f847a1b368e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:533:12
    #3 0x7f8463995d33 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:670:8
    #4 0x7f84639a2461 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:621:12
    #5 0x7f84639a5f43 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:139:57
    #6 0x7f846b0a97e3 in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:69:10
    #7 0x7f846b0a97e3 in mozilla::dom::quota::QuotaManager::Init(nsTSubstring<char16_t> const&) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3917:8
    #8 0x7f846b0a8a2f in mozilla::dom::quota::QuotaManager::GetOrCreate(nsIRunnable*, nsIEventTarget*) /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3534:28
    #9 0x7f846b4d5b8a in FinishOpen /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:20517:3
    #10 0x7f846b4d5b8a in mozilla::dom::indexedDB::(anonymous namespace)::FactoryOp::Run() /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:20649:12
    #11 0x7f8463999d59 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1220:14
    #12 0x7f84639a3281 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #13 0x7f8464b0485a in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:302:20
    #14 0x7f8464a283e2 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #15 0x7f8464a283e2 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #16 0x7f8464a283e2 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #17 0x7f8463993736 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:464:10
    #18 0x7f847a1db288 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #19 0x7f847d4464e1 in start_thread (/lib64/libpthread.so.0+0x94e1)

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3285:39 in mozilla::dom::quota::QuotaObject::LockedMaybeUpdateSize(long, bool)
Shadow bytes around the buggy address:
  0x0c1e800f7050: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x0c1e800f7060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e800f7070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e800f7080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e800f7090: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
=>0x0c1e800f70a0: fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd
  0x0c1e800f70b0: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e800f70c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e800f70d0: fa fa fa fa fa fa fa fa fa fa fd fd fd fd fd fd
  0x0c1e800f70e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1e800f70f0: 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
==323834==ABORTING
Severity: normal → critical
Group: core-security → dom-core-security
Duplicate of this bug: 1610924
See Also: → 1610884
Duplicate of this bug: 1610884

I believe https://addons.mozilla.org/en-US/firefox/addon/site-bleacher/ is to blame for this particular crash, but only when installed with uBlock Origin and/or uMatrix on a dirty profile. It should crash within 5 minutes of browsing around. Site Bleacher does its work after a tab is closed, so make sure to open and close some random tabs. Clean profiles don't seem to be affected much at all. Setting dom.storage.next_gen to false completely fixes the problem.

Priority: -- → P1

Simplified STR:

Create new profile
Install Site Bleacher
Open a half dozen random tabs
close all but 1
Tap the Site Bleacher icon and select clean.

It seems we want to access an OriginInfo instance through a raw pointer mOriginInfo which lives in a QuotaObject instance that outlives the ClearRequest that has cleared the last counting reference on this OriginInfo.

Flags: needinfo?(jvarga)

:ttung, can you take this?

Flags: needinfo?(jvarga) → needinfo?(ttung)

Sure

Assignee: nobody → ttung
Status: NEW → ASSIGNED
Flags: needinfo?(ttung)

Note to me:

  1. We probably want this back to catch similar issues in the future earlier. (Not in this bug of course)
  2. Check DirectoryLock because I would expect ClearRequestOp should wait for a directory lock for an origin if there is a quota client holding a directory lock for that origin. (Or break/abort other matching directory locks held by quota clients)

I couldn't reproduce the issue by following the comment #4, but I found something in our code:

When AbortOperations is called, I expect all existing accesses (Database, Snapshot) to DataStore (since it holds QuotaObject) should be aborted.

I still need to look into the code around more since lifecycles for Snapshot and Database are not cleared enough to me, but there seems to have an issue around AbortOperation and Snapshot.

(In reply to Tom Tung [:tt, :ttung] from comment #9)

I still need to look into the code around more since lifecycles for Snapshot and Database are not cleared enough to me, but there seems to have an issue around AbortOperation and Snapshot.

Actually, it's probably not related AbortOperation because it's used to ensure QM internal operations get the directory lock earlier.

Finally, I could reproduce the issue via opt-debug-asan build and it crashed at what I suspected in comment 8 (I suggested to change that debug assertion to release assertion for Nighly and early Beta).

Below is my stack:
Assertion failure: !mQuotaObjects.Count(), at /Users/tomtung/Work/mozilla-central/dom/quota/ActorsParent.cpp:918
#01: mozilla::dom::quota::OriginInfo::Release()[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbb588b3]
#02: nsTArray_Impl<RefPtr<mozilla::dom::quota::OriginInfo>, nsTArrayInfallibleAllocator>::RemoveElementsAt(unsigned long, unsigned long)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbb630fa]
#03: mozilla::dom::quota::QuotaManager::LockedRemoveQuotaForOrigin(mozilla::dom::quota::PersistenceType, nsTSubstring<char> const&, nsTSubstring<char> const&)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba84e52]
#04: mozilla::dom::quota::(anonymous namespace)::ClearRequestBase::DeleteFiles(mozilla::dom::quota::QuotaManager*, mozilla::dom::quota::PersistenceType)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbb3578c]
#05: mozilla::dom::quota::(anonymous namespace)::ClearRequestBase::DoDirectoryWork(mozilla::dom::quota::QuotaManager*)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbb32023]
#06: mozilla::dom::quota::(anonymous namespace)::OriginOperationBase::Run()[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0xbb09a23]
#07: nsThread::ProcessNextEvent(bool, bool*)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0x47157a]
#08: NS_ProcessNextEvent(nsIThread*, bool)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4863c4]
#09: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1d4c344]
#10: MessageLoop::RunInternal()[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1bda2b1]
#11: MessageLoop::Run()[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1bd9eee]
#12: nsThread::ThreadFunc(void*)[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/XUL +0x465f0d]
#13: _pt_root[/Users/tomtung/Work/mozilla-central/objdir/dist/NightlyDebug.app/Contents/MacOS/libnss3.dylib +0x43d995]
#14: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x32eb]
#15: _pthread_start[/usr/lib/system/libsystem_pthread.dylib +0x6249]
AddressSanitizer:DEADLYSIGNAL

I'm going to reproduce that with printing some log to ensure I understand the order.

Attached patch test.patchSplinter Review

This test reproduces the crash I hit yesterday.

Oh, yeah. ClearRequestBase::DeleteFiles can't remove entire origin if the directory lock for it only covers specific client directory.

I meant bug 1529122.

(In reply to Jan Varga [:janv] from comment #13)

Oh, yeah. ClearRequestBase::DeleteFiles can't remove entire origin if the directory lock for it only covers specific client directory.

Yeah, I'm working on a patch to create an internal operation in QuotaManager. It will only remove the origin directory when it has a directory lock for the whole origin and the origin directory is still empty when it goes to the IO thread. Note that it will be canceled at the early stage if some other client holding a directory lock for that origin.

Well, maybe we should consider if it's worth doing at all, seems like a non trivial patch. Leaving an empty origin directory in the repository doesn't look like a big issue, especially at this point when we plan to completely change the layout on disk. Users won't be able to tell if a directory belongs to specific origin.

(In reply to Jan Varga [:janv] from comment #16)

Well, maybe we should consider if it's worth doing at all, seems like a non trivial patch. Leaving an empty origin directory in the repository doesn't look like a big issue, especially at this point when we plan to completely change the layout on disk. Users won't be able to tell if a directory belongs to specific origin.

That's a good point! I was worried about having too many empty origin directories on the disk, but we can consider doing that in other bugs, especially it's a sec-high bug (we shouldn't make a nontrivial patch on sec bug).

I will prepare a small patch.

Comment on attachment 9125817 [details]
Bug 1610880 - Do not remove the empty origin directory when clearing a client;

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: From the patch, attackers might suspect the issue is related to race between quota clients. However, it's still not easy to figure out how to do that.
  • 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?: In theory 68, but invloved functions in STR is disabled by default in 73 or earler.
  • If not all supported branches, which bug introduced the flaw?: 1529122 (Main), 128679 (functions being called in the reported STR))
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: (Plan to uplift to esr 68, beta 74 after there is no any related issue reported for a week)
  • How likely is this patch to cause regressions; how much testing does it need?: It should be small since I have pushed to try and the try looks green. Also, I made a test to verify the fix works.
Attachment #9125817 - Flags: sec-approval?

Comment on attachment 9125817 [details]
Bug 1610880 - Do not remove the empty origin directory when clearing a client;

Approved to land and request uplift when ready

Attachment #9125817 - Flags: sec-approval? → sec-approval+
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75

Confirming that my STR no longer produces a crash.

Please nominate this for Beta approval when you get a chance. It grafts cleanly as-landed. If you want to land this on ESR68 too, it'll need a rebased patch. Given that the functionality in question is disabled by default there, I'm not convinced it's worth the effort, though.

Flags: needinfo?(ttung)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #23)

Please nominate this for Beta approval when you get a chance. It grafts cleanly as-landed. If you want to land this on ESR68 too, it'll need a rebased patch. Given that the functionality in question is disabled by default there, I'm not convinced it's worth the effort, though.

Thanks for the notice! I plan to push to Beta and ESR next week (after ensuring there is no related crash).

Flags: needinfo?(ttung)

(In reply to Tom Tung [:tt, :ttung] from comment #24)

Thanks for the notice! I plan to push to Beta and ESR next week (after ensuring there is no related crash).

(I will file another bug for the test case, but I guess it will be a month later)

Flags: in-testsuite?

Ping for a Beta uplift request :)

Flags: needinfo?(ttung)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #26)

Ping for a Beta uplift request :)

(Will do it today and then clear the ni)

Comment on attachment 9125817 [details]
Bug 1610880 - Do not remove the empty origin directory when clearing a client;

Beta/Release Uplift Approval Request

  • User impact if declined: Might UAF if they follow steps in comment#4 or simliar scenario.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Refer to comment#4 or run the attached test in the bug. (It will be landed in another bug a month later)
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Because the fix is verified on Nightly (and a unit test) and there is no related issue which has been reported recently.
  • String changes made/needed:
Flags: needinfo?(ttung)
Attachment #9125817 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Attached patch patchForEsr68.patch (obsolete) — Splinter Review

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Might UAF if they follow steps in comment#4 or similar scenario.
Fix Landed on Version: 75
Needs manual test from QE?: Yes
If yes, steps to reproduce: Refer to comment#4 or run the attached test in the bug. (Note that you need to turn on the pref dom.storage.next_gen)
Risk to taking this patch (and alternatives if risky): Low
Why is the change risky/not risky? (and alternatives if risky): Because the fix is verified on Nightly (and a unit test) and there is no related issue which has been reported recently.

Attachment #9127815 - Flags: approval-mozilla-esr68?
QA Whiteboard: [qa-triaged]

Comment on attachment 9125817 [details]
Bug 1610880 - Do not remove the empty origin directory when clearing a client;

Uplift approved for 74.0b6, thanks.

Attachment #9125817 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 9127815 [details] [diff] [review]
patchForEsr68.patch

I'm still not sure how serious this is on ESR68 if the functionality is disabled by default on <73, but OK. Approved for 68.6esr.
Attachment #9127815 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Comment on attachment 9127815 [details] [diff] [review]
patchForEsr68.patch

Actually, this patch doesn't apply cleanly to ESR68 tip. Please attach a revised patch.
Flags: needinfo?(ttung)
Attachment #9127815 - Flags: approval-mozilla-esr68+

(In reply to Ryan VanderMeulen [:RyanVM] from comment #33)

Comment on attachment 9127815 [details] [diff] [review]
patchForEsr68.patch

Actually, this patch doesn't apply cleanly to ESR68 tip. Please attach a
revised patch.

Ah, sorry, my bad. I downloaded the patch from Phaboritor but I forgot to rebased that....

Flags: needinfo?(ttung)
Attachment #9127815 - Attachment is obsolete: true

Comment on attachment 9128075 [details] [diff] [review]
patchForEsr68.patch

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Although STR needs LSNG (which is disable by default on 68) to be involved, it can happen on other storage clients (IDB, DOM Cache) in theory. The possibility should be much lower since they don't cache data in memory for a while and then flush into disk later.
  • Fix Landed on Version: 75, 74
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): Risky should be low, but many codes in QuotaManager has been changed between 68 and 74 so I marked it as medium.

I ran all tests under QuotaManager and the attached test and all of them passed.

  • String or UUID changes made by this patch:
Attachment #9128075 - Flags: approval-mozilla-esr68?
Attachment #9125817 - Flags: approval-mozilla-esr68?

I reproduced the initial crash by following the STR from Comment 4 - on Ubuntu 18.04 using the provided Firefox Nightly (ASAN build) from the Description.
I can confirm that the crash is no longer reproducible on Firefox Nightly 75.0a1 (20200221095110) and Firefox 74 beta 7.

Flags: sec-bounty? → sec-bounty+
Attachment #9125817 - Flags: approval-mozilla-esr68?
Comment on attachment 9128075 [details] [diff] [review]
patchForEsr68.patch

Approved for 68.6esr.
Attachment #9128075 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Blocks: 1618483

I can confirm that there is no crash when following the steps from Comment 4 - verified on Ubuntu 18.04 x64 on the asan Firefox 68.6.0 esr build.
(https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr68&selectedJob=290826015)

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Whiteboard: [adv-main74+][adv-esr68.6+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.