Closed Bug 1527652 Opened 5 years ago Closed 4 years ago

Use after free in function Length(), nsTArray.h, likely due to dom/cache/StreamControl.cpp

Tracking

()

Status:

RESOLVED DUPLICATE of bug 1507180

People

(Reporter: hanno, Assigned: asuth)

Details

(Keywords: csectype-uaf, sec-moderate)

Multiple Authors

Description

•

5 years ago

•

Edited

I observed a use after free bug with Firefox ASAN builds.

Unfortunately I don't have much more than a stack trace. The following 5 webpages were opened when it happened:
onlybooks.org circle.page donsat.com.ua finkenbusch.de nfsplanet.com

But I was unable to reproduce the issue a second time.

Trying to make sense of the stack trace the last part where the trace for the access matches the trace for the free is in:
https://hg.mozilla.org/mozilla-central/file/tip/dom/cache/StreamControl.cpp

and it points to these lines:

while (iter.HasMore()) {
iter.GetNext()->CloseStream();

}

It doesn't look immediately suspicious.

Stack trace:

==23313==ERROR: AddressSanitizer: heap-use-after-free on address 0x6080003a70e0 at pc 0x7fb253bd5a8f bp 0x7fb2396a5bf0 sp 0x7fb2396a5be8
READ of size 8 at 0x6080003a70e0 thread T30 (IPDL Background)
    #0 0x7fb253bd5a8e in Length /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:344:37
    #1 0x7fb253bd5a8e in Length /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:84
    #2 0x7fb253bd5a8e in HasMore /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:318
    #3 0x7fb253bd5a8e in mozilla::dom::cache::StreamControl::CloseAllReadStreams() /builds/worker/workspace/build/src/dom/cache/StreamControl.cpp:61
    #4 0x7fb253b7dd16 in NotifyCloseAll /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:163:3
    #5 0x7fb253b7dd16 in mozilla::dom::cache::CacheStreamControlParent::CloseAll() /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:143
    #6 0x7fb253b85861 in mozilla::dom::cache::Context::CancelAll() /builds/worker/workspace/build/src/dom/cache/Context.cpp:814:23
    #7 0x7fb253bcccc5 in mozilla::dom::cache::Manager::Shutdown() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1906:14
    #8 0x7fb253bc2883 in mozilla::dom::cache::Manager::Factory::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:317:18
    #9 0x7fb253bc23da in mozilla::dom::cache::Manager::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1524:3
    #10 0x7fb2552403e9 in mozilla::dom::quota::QuotaManager::Shutdown() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3265:22
    #11 0x7fb25524004b in mozilla::dom::quota::QuotaManager::ShutdownRunnable::Run() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:2473:19
    #12 0x7fb24dfb7836 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1162:14
    #13 0x7fb24dfbda58 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:474:10
    #14 0x7fb24ef6c19a in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
    #15 0x7fb24eeb2d7f in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #16 0x7fb24eeb2d7f in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #17 0x7fb24eeb2d7f in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #18 0x7fb24dfb19ea in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:449:11
    #19 0x7fb2660cb666 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #20 0x7fb2692be163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)
    #21 0x7fb268ea3dee in clone (/lib/x86_64-linux-gnu/libc.so.6+0x11adee)

0x6080003a70e0 is located 64 bytes inside of 96-byte region [0x6080003a70a0,0x6080003a7100)
freed by thread T30 (IPDL Background) here:
    #0 0x559b8dbba5d2 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7fb24ef0a80b in mozilla::ipc::BackgroundParentImpl::DeallocPCacheStreamControlParent(mozilla::dom::cache::PCacheStreamControlParent*) /builds/worker/workspace/build/src/ipc/glue/BackgroundParentImpl.cpp:904:3
    #2 0x7fb24f78c615 in mozilla::ipc::PBackgroundParent::RemoveManagee(int, mozilla::ipc::IProtocol*) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundParent.cpp
    #3 0x7fb24f886cbf in mozilla::dom::cache::PCacheStreamControlParent::Send__delete__(mozilla::dom::cache::PCacheStreamControlParent*) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PCacheStreamControlParent.cpp:117:12
    #4 0x7fb253bd2564 in NoteClosed /builds/worker/workspace/build/src/dom/cache/StreamControl.cpp:29:3
    #5 0x7fb253bd2564 in NoteClosedOnOwningThread /builds/worker/workspace/build/src/dom/cache/ReadStream.cpp:399
    #6 0x7fb253bd2564 in mozilla::dom::cache::ReadStream::Inner::NoteClosed() /builds/worker/workspace/build/src/dom/cache/ReadStream.cpp:363
    #7 0x7fb253bd5941 in mozilla::dom::cache::StreamControl::CloseAllReadStreams() /builds/worker/workspace/build/src/dom/cache/StreamControl.cpp:62:21
    #8 0x7fb253b7dd16 in NotifyCloseAll /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:163:3
    #9 0x7fb253b7dd16 in mozilla::dom::cache::CacheStreamControlParent::CloseAll() /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:143
    #10 0x7fb253b85861 in mozilla::dom::cache::Context::CancelAll() /builds/worker/workspace/build/src/dom/cache/Context.cpp:814:23
    #11 0x7fb253bcccc5 in mozilla::dom::cache::Manager::Shutdown() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1906:14
    #12 0x7fb253bc2883 in mozilla::dom::cache::Manager::Factory::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:317:18
    #13 0x7fb253bc23da in mozilla::dom::cache::Manager::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1524:3
    #14 0x7fb2552403e9 in mozilla::dom::quota::QuotaManager::Shutdown() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3265:22
    #15 0x7fb25524004b in mozilla::dom::quota::QuotaManager::ShutdownRunnable::Run() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:2473:19
    #16 0x7fb24dfb7836 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1162:14
    #17 0x7fb24dfbda58 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:474:10
    #18 0x7fb24ef6c19a in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
    #19 0x7fb24eeb2d7f in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #20 0x7fb24eeb2d7f in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #21 0x7fb24eeb2d7f in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #22 0x7fb24dfb19ea in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:449:11
    #23 0x7fb2660cb666 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #24 0x7fb2692be163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)

previously allocated by thread T30 (IPDL Background) here:
    #0 0x559b8dbba953 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x559b8dbef17d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:68:15
    #2 0x7fb253b5f031 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
    #3 0x7fb253b5f031 in mozilla::dom::cache::AutoParentOpResult::SerializeReadStream(nsID const&, mozilla::dom::cache::StreamList*, mozilla::dom::cache::CacheReadStream*) /builds/worker/workspace/build/src/dom/cache/AutoUtils.cpp:501
    #4 0x7fb253b5da8c in mozilla::dom::cache::AutoParentOpResult::SerializeResponseBody(mozilla::dom::cache::SavedResponse const&, mozilla::dom::cache::StreamList*, mozilla::dom::cache::CacheResponse*) /builds/worker/workspace/build/src/dom/cache/AutoUtils.cpp:485:3
    #5 0x7fb253b70dc0 in mozilla::dom::cache::CacheOpParent::OnOpComplete(mozilla::ErrorResult&&, mozilla::dom::cache::CacheOpResult const&, long, nsTArray<mozilla::dom::cache::SavedResponse> const&, nsTArray<mozilla::dom::cache::SavedRequest> const&, mozilla::dom::cache::StreamList*) /builds/worker/workspace/build/src/dom/cache/CacheOpParent.cpp:177:12
    #6 0x7fb253bc0f62 in mozilla::dom::cache::Manager::Listener::OnOpComplete(mozilla::ErrorResult&&, mozilla::dom::cache::CacheOpResult const&, mozilla::dom::cache::SavedResponse const&, mozilla::dom::cache::StreamList*) /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1489:3
    #7 0x7fb253be68b3 in mozilla::dom::cache::Manager::CacheMatchAction::Complete(mozilla::dom::cache::Manager::Listener*, mozilla::ErrorResult&&) /builds/worker/workspace/build/src/dom/cache/Manager.cpp:557:18
    #8 0x7fb253be5a58 in mozilla::dom::cache::Manager::BaseAction::CompleteOnInitiatingThread(nsresult) /builds/worker/workspace/build/src/dom/cache/Manager.cpp:436:7
    #9 0x7fb253b8304f in mozilla::dom::cache::Context::ActionRunnable::Run() /builds/worker/workspace/build/src/dom/cache/Context.cpp:645:16
    #10 0x7fb24dfb7836 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1162:14
    #11 0x7fb24dfbda58 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:474:10
    #12 0x7fb2556e7821 in SpinEventLoopUntil<mozilla::ProcessFailureBehavior::ReportToCaller, (lambda at /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:16106:3)> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:348:25
    #13 0x7fb2556e7821 in mozilla::dom::indexedDB::(anonymous namespace)::QuotaClient::ShutdownWorkThreads() /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:16106
    #14 0x7fb2552403e9 in mozilla::dom::quota::QuotaManager::Shutdown() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3265:22
    #15 0x7fb25524004b in mozilla::dom::quota::QuotaManager::ShutdownRunnable::Run() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:2473:19
    #16 0x7fb24dfb7836 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1162:14
    #17 0x7fb24dfbda58 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:474:10
    #18 0x7fb24ef6c19a in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
    #19 0x7fb24eeb2d7f in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #20 0x7fb24eeb2d7f in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
    #21 0x7fb24eeb2d7f in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
    #22 0x7fb24dfb19ea in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:449:11
    #23 0x7fb2660cb666 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #24 0x7fb2692be163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)

Thread T30 (IPDL Background) created by T0 here:
    #0 0x559b8dba326d in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
    #1 0x7fb2660c8395 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
    #2 0x7fb2660c7f7e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
    #3 0x7fb24dfb3ce9 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:655:8
    #4 0x7fb24dfbcba0 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:414:12
    #5 0x7fb24dfc0769 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:127:57
    #6 0x7fb24ef34e89 in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:71:10
    #7 0x7fb24ef34e89 in (anonymous namespace)::ParentImpl::CreateBackgroundThread() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:944
    #8 0x7fb24ef0e867 in CreateActorForSameProcess /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:854:32
    #9 0x7fb24ef0e867 in GetOrCreateForCurrentThread /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1456
    #10 0x7fb24ef0e867 in mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread(nsIEventTarget*) /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:658
    #11 0x7fb2540da6a1 in mozilla::dom::ClientManager::ClientManager() /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:51:7
    #12 0x7fb2540dce40 in mozilla::dom::ClientManager::GetOrCreateForCurrentThread() /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:205:14
    #13 0x7fb2540d227d in mozilla::dom::ClientManager::CreateSource(mozilla::dom::ClientType, nsISerialEventTarget*, nsIPrincipal*) /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:261:31
    #14 0x7fb2598a5adf in nsDocShell::MaybeCreateInitialClientSource(nsIPrincipal*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:2435:26
    #15 0x7fb2598dfaf5 in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIURI*, bool, bool) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7091:5
    #16 0x7fb259960d4a in nsWebShellWindow::Initialize(nsIXULWindow*, nsIXULWindow*, nsIURI*, int, int, bool, nsITabParent*, mozIDOMWindowProxy*, nsWidgetInitData&) /builds/worker/workspace/build/src/xpfe/appshell/nsWebShellWindow.cpp:233:21
    #17 0x7fb25995b2e9 in nsAppShellService::JustCreateTopWindow(nsIXULWindow*, nsIURI*, unsigned int, int, int, bool, nsITabParent*, mozIDOMWindowProxy*, nsWebShellWindow**) /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp:667:25
    #18 0x7fb25995a5e4 in nsAppShellService::CreateHiddenWindowHelper(bool) /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp:130:7
    #19 0x7fb25a19a4bf in nsAppStartup::CreateHiddenWindow() /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:238:27
    #20 0x7fb25a3f8140 in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4592:22
    #21 0x7fb25a3fa6b4 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4839:8
    #22 0x7fb25a3fc1e0 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4923:21
    #23 0x559b8dbed1ec in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:214:22
    #24 0x559b8dbed1ec in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:293
    #25 0x7fb268dad09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:344:37 in Length
Shadow bytes around the buggy address:
  0x0c108006cdc0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c108006cdd0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c108006cde0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108006cdf0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c108006ce00: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
=>0x0c108006ce10: fa fa fa fa fd fd fd fd fd fd fd fd[fd]fd fd fd  
  0x0c108006ce20: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c108006ce30: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c108006ce40: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c108006ce50: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c108006ce60: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==23313==ABORTING

Alex Gaynor [:Alex_Gaynor]

Updated

•

5 years ago

Group: firefox-core-security → dom-core-security

Component: Security → DOM: Quota Manager

Product: Firefox → Core

Alex Gaynor [:Alex_Gaynor]

Updated

•

5 years ago

Keywords: csectype-uaf, sec-moderate

Daniel Veditz [:dveditz]

Comment 1

•

5 years ago

Both the free and re-use are during the QuotaManager Shutdown. Any chance for user events/scripts to be run in that gap?

Andrew Sutherland [:asuth] (he/him)

Assignee

Comment 2

•

5 years ago

•

Edited

Risk Analysis

There is no chance for user events/scripts to run in the QuotaManager shutdown gap. The code is executing on PBackground where no content runs and content is only able to interact via IPC messages, and no IPC messages are allowed to be processed until after all of this code finishes running and returns control flow to the message loop.

We expect this problem to occur in the following circumstances:

Content has some DOM Cache API requests in flight.
One of the following is happening, neither of which are things nefarious content can directly control.
- This one, shutdown under a DEBUG or ASAN build (or similar non-production build).
- The user triggers privacy data clearing/sanitization for the origin without first terminating the globals in question. (I think impacted tabs will be closed, but things are a little tricky with ServiceWorkers, and with races and such, this could happen.)

Defect Analysis

It's also worth noting that ASAN builds differ from (non-debug) production builds. In production, https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/dom/ipc/ContentChild.cpp#2142 (compiled in due to https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/xpcom/base/nscore.h#172) will have killed the content process during "profile-before-change". This will take down the PBackground channel and cause cleanup due to ActorDestroy invocations, avoiding what we're seeing above which happens during "profile-before-change-qm" which is strictly after "profile-before-change".

But in ASAN/DEBUG shutdown, PBackground continues to exist until it gets cleanly torn down from either end. The DOM Cache implementation makes things difficult for itself here by not just letting the WorkerPrivate completely shutdown and let the PBackground channel be torn down. It tries to keep going so pending requests can be fulfilled and/or actors are explicitly torn down (out of concern for the process being aborted due to a message arriving for a dead actor, something that I believe may no longer be fatal). Which is how we can find ourselves with QuotaManager being the one trying to tear down client-owned PBackground actors. (Note that there are also some races likely involved.)

The other good news is that ASAN is awesome and it helps illuminate the problem here that:

CacheStreamControlParent::Shutdown() is invoking Send__delete__. The class has an IPC-managed lifecycle which means that the instance (which also subclasses StreamControl).
This happens because of the cascade of StreamControl::CloseAllReadStreams -> (potentially multiple times) ReadStream::Inner::CloseStream -> ReadStream::Inner::Close -> ReadStream::Inner::NoteClosed -> (because already on current thread) ReadStream::Inner::NoteClosedOnOwningThread -> StreamControl::NoteClosed -> (virtual dispatched, differs between parent and child) CacheStreamControlParent::NoteClosedAfterForget -> CacheStreamControlParent::RecvNoteClosed -> StreamList::NoteClosed -> (because mList.IsEmpty() && mStreamControl) CacheStreamControlParent::Shutdown -> CacheStreamControlParent::Send__delete__ -> DEALLOCATED.
When we return to CloseAllReadStreams, the instance is deallocated and if we touch anything
of the instance, it's a use-after-free.
Note that StreamControl::CloseAllReadStreams is invoked in the following chain that's common to both our free and our use-after-free: Context::CancelAll -> (iterating over mActivityList) StreamList::Cancel -> StreamList::CloseAll -> CacheStreamControlParent::CloseAll -> CacheStreamControlParent::NotifyCloseAll -> StreamControl::CloseAllReadStreams.

In production, the case we expect to be invoked because of PBackground's abrupt termination is CacheStreamControlParent::ActorDestroy at https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/dom/cache/CacheStreamControlParent.cpp#96 which:

Invokes StreamControl::CloseAllReadStreamsWithoutReporting() -> (iterating, multiple invocations) ReadStream::Inner::CloseStreamWithoutReporting -> ReadStream::Inner::Forget -> (because already on the right thread) StreamControl::ForgetReadStream which just removes the stream from mReadStreamList.
- The above control flow notably differs in that we're just calling ForgetReadStream on StreamControl instead of calling NoteClosed which calls ForgetReadStream but then goes on to invoke NoteClosedAfterForget which is where things get all self-deallocatey.
invokes (cache) Manager::RemoveListener. Don't care.
invokes StreamList::RemoveStreamControl which clears its mStreamControl which precludes SCacheStreamControlParent::Shutdown being invoked.
invokes StreamList::NoteClosedAll. This is post facto cleanup via (cache) Manager::ReleaseBodyId followed by a conditional call to CacheStreamControlParent::Shutdown. Because StreamList::RemoveStreamControl was called immediately before NoteClosedAll, that conditional call to Shutdown is never made.

There's a number of questions that are begged here. Things are complicated because our DEBUG shutdown approach is very complicated and arguably a bit non-sensical. I think the right answer for what to do depends on what we do for privacy reasons where we abort operations for an origin. The stack for that looks like cache::QuotaClient::AbortOperations() -> cache::Manager::Abort -> cache::Manager::Factory::Abort -> cache::Context::CancelAll. Since that's also what's causing the problem here, this is something we need to fix as-is without just changing how shutdown works. (Although, it does also beg the question of whether AbortOperations should already have ensured all the relevant globals have terminated. But it's also the case that this is something we need to address from both tearing down the global and ensuring that we tear down all the activities they were up to.) We do want the actors torn down.

I think that makes the right place to clean this up StreamList::Cancel/CloseAll. Only Cancel calls CloseAll. (Cancel does have other callers that branch out a bit.) Because the expected outcome is a series of calls to StreamList::NoteClosed that will culminate in mStreamControl->Shutdown() being invoked, we can move that Shutdown call up the stack to a safe point by:

In StreamList::CloseAll() where mStreamControl is non-null, steal mStreamControl, leaving it null during the call to CloseAll(), avoiding the call to Shutdown() in NoteClosed.
Then invoke streamControl->Shutdown before returning. This avoids having CacheStreamControlParent on the stack but maintains the invariant that we've shutdown and deleted the stream control.

So, since I think I figured this out, I'm going to take this bug and put up a fix.

Assignee: nobody → bugmail

Status: NEW → ASSIGNED

Yaron Tausky [:ytausky]

Updated

•

5 years ago

Priority: -- → P2

Jens Stutte [:jstutte]

Comment 3

•

4 years ago

:ytausky this looks like a duplicate of bug 1507180 ?

Flags: needinfo?(ytausky)

Yaron Tausky [:ytausky]

Comment 4

•

4 years ago

Yes, it's the same thing.

Status: ASSIGNED → RESOLVED

Closed: 4 years ago

Flags: needinfo?(ytausky)

Resolution: --- → DUPLICATE

Daniel Veditz [:dveditz]

Updated

•

3 years ago

Group: dom-core-security

You need to log in before you can comment on or make changes to this bug.

Bugzilla

Quick Search

Use after free in function Length(), nsTArray.h, likely due to dom/cache/StreamControl.cpp

Categories

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

Tracking

()

People

(Reporter: hanno, Assigned: asuth)

References

Details

(Keywords: csectype-uaf, sec-moderate)

Crash Data

Security

(public)

User Story

Description

Updated

Updated

Comment 1

Comment 2

Risk Analysis

Defect Analysis

Updated

Comment 3

Comment 4

Updated