1859030 - AddressSanitizer: heap-use-after-free [@ OtherPidMaybeInvalid] with READ of size 4

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Found while fuzzing mozilla-central rev 0db2502d9e95 (built with: --enable-address-sanitizer --enable-fuzzing). The testcase isn't very reliable but I've managed to get a pernosco recording of it. I will include the link for that shortly.

AddressSanitizer: heap-use-after-free [@ OtherPidMaybeInvalid] with READ of size 4

    =================================================================
    ==641==ERROR: AddressSanitizer: heap-use-after-free on address 0x5170000b8e30 at pc 0x7fcee7f5b4e6 bp 0x7fce50e97170 sp 0x7fce50e97168
    READ of size 4 at 0x5170000b8e30 thread T6
        #0 0x7fcee7f5b4e5 in OtherPidMaybeInvalid /builds/worker/workspace/obj-build/dist/include/mozilla/ipc/ProtocolUtils.h:521:57
        #1 0x7fcee7f5b4e5 in OtherPid /builds/worker/workspace/obj-build/ipc/ipdl/PSharedWorkerParent.cpp:44:56
        #2 0x7fcee7f5b4e5 in mozilla::dom::(anonymous namespace)::WorkerManagerCreatedRunnable::Run() /gecko/dom/workers/sharedworkers/SharedWorkerService.cpp:81:9
        #3 0x7fcedd9f359f in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1192:16
        #4 0x7fcedda00e8a in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:480:10
        #5 0x7fcedf60f779 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:330:5
        #6 0x7fcedf437d7a in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:370:10
        #7 0x7fcedf437d7a in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:363:3
        #8 0x7fcedf437d7a in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:345:3
        #9 0x7fcedd9ea6a0 in nsThread::ThreadFunc(void*) /gecko/xpcom/threads/nsThread.cpp:370:10
        #10 0x7fcf047eb10f in _pt_root /gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
        #11 0x56541a77c68a in asan_thread_start(void*) /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:225:31
        #12 0x7fcf04f83ac2 in start_thread nptl/pthread_create.c:442:8
        #13 0x7fcf05014bf3 in __clone misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:100
    
    0x5170000b8e30 is located 48 bytes inside of 728-byte region [0x5170000b8e00,0x5170000b90d8)
    freed by thread T0 here:
        #0 0x56541a77ff46 in free /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
        #1 0x7fcedf5b779b in operator()<> /gecko/xpcom/threads/nsThreadUtils.h:1164:18
        #2 0x7fcedf5b779b in __invoke_impl<void, (lambda at /xpcom/threads/nsThreadUtils.h:1163:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:60:14
        #3 0x7fcedf5b779b in __invoke<(lambda at /xpcom/threads/nsThreadUtils.h:1163:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:95:14
        #4 0x7fcedf5b779b in __apply_impl<(lambda at /xpcom/threads/nsThreadUtils.h:1163:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:1678:14
        #5 0x7fcedf5b779b in apply<(lambda at /xpcom/threads/nsThreadUtils.h:1163:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:1687:14
        #6 0x7fcedf5b779b in apply<(anonymous namespace)::ParentImpl, void ((anonymous namespace)::ParentImpl::*)()> /gecko/xpcom/threads/nsThreadUtils.h:1162:12
        #7 0x7fcedf5b779b in mozilla::detail::RunnableMethodImpl<(anonymous namespace)::ParentImpl*, void ((anonymous namespace)::ParentImpl::*)(), false, (mozilla::RunnableKind)0>::Run() /gecko/xpcom/threads/nsThreadUtils.h:1213:13
        #8 0x7fcedd9c165a in mozilla::RunnableTask::Run() /gecko/xpcom/threads/TaskController.cpp:549:16
        #9 0x7fcedd9abc58 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:876:26
        #10 0x7fcedd9a8667 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:699:15
        #11 0x7fcedd9a8f49 in mozilla::TaskController::ProcessPendingMTTask(bool) /gecko/xpcom/threads/TaskController.cpp:485:36
        #12 0x7fcedd9c9131 in operator() /gecko/xpcom/threads/TaskController.cpp:211:37
        #13 0x7fcedd9c9131 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /gecko/xpcom/threads/nsThreadUtils.h:548:5
        #14 0x7fcedd9f31ca in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1198:16
        #15 0x7fcedda00e8a in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:480:10
        #16 0x7fcedf60dd6e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:85:21
        #17 0x7fcedf437d7a in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:370:10
        #18 0x7fcedf437d7a in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:363:3
        #19 0x7fcedf437d7a in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:345:3
        #20 0x7fcee8ac5759 in nsBaseAppShell::Run() /gecko/widget/nsBaseAppShell.cpp:148:27
        #21 0x7fceee23a2fb in nsAppStartup::Run() /gecko/toolkit/components/startup/nsAppStartup.cpp:296:30
        #22 0x7fceee55b284 in XREMain::XRE_mainRun() /gecko/toolkit/xre/nsAppRunner.cpp:5675:22
        #23 0x7fceee55d3f4 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /gecko/toolkit/xre/nsAppRunner.cpp:5884:8
        #24 0x7fceee55e611 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /gecko/toolkit/xre/nsAppRunner.cpp:5940:21
        #25 0x56541a7bfcc2 in do_main /gecko/browser/app/nsBrowserApp.cpp:227:22
        #26 0x56541a7bfcc2 in main /gecko/browser/app/nsBrowserApp.cpp:445:16
        #27 0x7fcf04f18d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    
    previously allocated by thread T6 here:
        #0 0x56541a7801ee in malloc /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
        #1 0x56541a7c5305 in moz_xmalloc /gecko/memory/mozalloc/mozalloc.cpp:52:15
        #2 0x7fcedf55dd0c in operator new /builds/worker/workspace/obj-build/dist/include/mozilla/cxxalloc.h:33:10
        #3 0x7fcedf55dd0c in mozilla::ipc::BackgroundStarterParent::RecvInitBackground(mozilla::ipc::Endpoint<mozilla::ipc::PBackgroundParent>&&) /gecko/ipc/glue/BackgroundImpl.cpp:1092:23
        #4 0x7fcedf733b5d in mozilla::ipc::PBackgroundStarterParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundStarterParent.cpp:111:91
        #5 0x7fcedf604765 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /gecko/ipc/glue/MessageChannel.cpp:1800:25
        #6 0x7fcedf60018b in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /gecko/ipc/glue/MessageChannel.cpp:1725:9
        #7 0x7fcedf601539 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /gecko/ipc/glue/MessageChannel.cpp:1525:3
        #8 0x7fcedf602ab3 in mozilla::ipc::MessageChannel::MessageTask::Run() /gecko/ipc/glue/MessageChannel.cpp:1623:14
        #9 0x7fcedd9f359f in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1192:16
        #10 0x7fcedda00e8a in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:480:10
        #11 0x7fcedf60f779 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:330:5
        #12 0x7fcedf437d7a in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:370:10
        #13 0x7fcedf437d7a in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:363:3
        #14 0x7fcedf437d7a in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:345:3
        #15 0x7fcedd9ea6a0 in nsThread::ThreadFunc(void*) /gecko/xpcom/threads/nsThread.cpp:370:10
        #16 0x7fcf047eb10f in _pt_root /gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
        #17 0x56541a77c68a in asan_thread_start(void*) /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:225:31
    
    Thread T6 created by T0 here:
        #0 0x56541a765e2d in pthread_create /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:237:3
        #1 0x7fcf047d9834 in _PR_CreateThread /gecko/nsprpub/pr/src/pthreads/ptthread.c:458:14
        #2 0x7fcf047c742e in PR_CreateThread /gecko/nsprpub/pr/src/pthreads/ptthread.c:533:12
        #3 0x7fcedd9ee159 in nsThread::Init(nsTSubstring<char> const&) /gecko/xpcom/threads/nsThread.cpp:619:20
        #4 0x7fcedd9fea14 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, nsIThreadManager::ThreadCreationOptions, nsIThread**) /gecko/xpcom/threads/nsThreadManager.cpp:597:22
        #5 0x7fcedda0c1f5 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, already_AddRefed<nsIRunnable>, nsIThreadManager::ThreadCreationOptions) /gecko/xpcom/threads/nsThreadUtils.cpp:176:57
        #6 0x7fcedf55b113 in NS_NewNamedThread<16UL> /gecko/xpcom/threads/nsThreadUtils.h:76:10
        #7 0x7fcedf55b113 in CreateBackgroundThread /gecko/ipc/glue/BackgroundImpl.cpp:878:7
        #8 0x7fcedf55b113 in (anonymous namespace)::ParentImpl::AllocStarter(mozilla::dom::ContentParent*, mozilla::ipc::Endpoint<mozilla::ipc::PBackgroundStarterParent>&&, bool) /gecko/ipc/glue/BackgroundImpl.cpp:810:30
        #9 0x7fcedf55bb5f in Startup /gecko/ipc/glue/BackgroundImpl.cpp:1153:5
        #10 0x7fcedf55bb5f in mozilla::ipc::BackgroundChild::Startup() /gecko/ipc/glue/BackgroundImpl.cpp:677:35
        #11 0x7fcee77d5c2c in mozilla::dom::ContentParent::StartUp() /gecko/dom/ipc/ContentParent.cpp:673:3
        #12 0x7fcee9f0ab2d in nsLayoutStatics::Initialize() /gecko/layout/build/nsLayoutStatics.cpp:147:3
        #13 0x7fcee9f0a909 in nsLayoutModuleInitialize() /gecko/layout/build/nsLayoutModule.cpp:104:7
        #14 0x7fcedd97edfb in nsComponentManagerImpl::Init() /gecko/xpcom/components/nsComponentManager.cpp:382:5
        #15 0x7fcedda723a5 in NS_InitXPCOM /gecko/xpcom/build/XPCOMInit.cpp:444:51
        #16 0x7fceee545a85 in ScopedXPCOMStartup::Initialize(bool) /gecko/toolkit/xre/nsAppRunner.cpp:1996:8
        #17 0x7fceee55d3d9 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /gecko/toolkit/xre/nsAppRunner.cpp:5880:22
        #18 0x7fceee55e611 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /gecko/toolkit/xre/nsAppRunner.cpp:5940:21
        #19 0x56541a7bfcc2 in do_main /gecko/browser/app/nsBrowserApp.cpp:227:22
        #20 0x56541a7bfcc2 in main /gecko/browser/app/nsBrowserApp.cpp:445:16
        #21 0x7fcf04f18d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    
    SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/obj-build/dist/include/mozilla/ipc/ProtocolUtils.h:521:57 in OtherPidMaybeInvalid
    Shadow bytes around the buggy address:
      0x5170000b8b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x5170000b8c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x5170000b8c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x5170000b8d00: 00 00 fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x5170000b8d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    =>0x5170000b8e00: fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd
      0x5170000b8e80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x5170000b8f00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x5170000b8f80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x5170000b9000: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x5170000b9080: fd fd fd fd fd fd fd fd fd fd fd 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
    ==641==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Detailed Crash Information

Comment 2

[Andrew McCreight [:mccr8]]

It kind of looks like ParentImpl::MainThreadActorDestroy() runs on the main thread, destroying the ParentImpl, then WorkerManagerCreatedRunnable::Run() runs, then runs PSharedWorkerParent::OtherPid(), which tries to call OtherPidMaybeInvalid() on the dead ParentImpl.

Comment 3

[Bugmon [:jkratzer for issues]]

A pernosco session for this bug can be found here.

Comment 4

[Jens Stutte [:jstutte]]

(In reply to Bugmon [:jkratzer for issues] from comment #3)

A pernosco session for this bug can be found here.

I added some notes. IIUC we keep an actor alive via a RefPtr in two (!) runnables that was intended to be destroyed by the dying top level actor. It might be enough to check here and maybe also here if mActor->IsActorDestroyed() ?

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jens Stutte [:jstutte] from comment #4)

I added some notes. IIUC we keep an actor alive via a RefPtr in two (!) runnables that was intended to be destroyed by the dying top level actor. It might be enough to check here and maybe also here if mActor->IsActorDestroyed() ?

I think the QM/IDB-specific IsActorDestroyed idiom has been replaced by just directly calling CanSend() which should work, yeah. But I believe it's strictly only safe to call IPC actor methods on their associated thread, so mozilla::dom::SharedWorkerService::GetOrCreateWorkerManagerOnMainThread is not an okay place to call mActor->CanSend() but mozilla::dom::WorkerManagerCreatedRunnable::Run is okay.

edit: That said, I think only having the check in WorkerManagerCreatedRunnable::Run as an early return seems fine. There's no need to try and stop things earlier on the main thread that I immediately see.

Comment 6

[Jens Stutte [:jstutte]]

Attachment: Bug 1859030 - Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r?#dom-worker-reviewers

Comment 7

[Jens Stutte [:jstutte]]

Comment on attachment 9358665 [details]
Bug 1859030 - Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r?#dom-worker-reviewers

Security Approval Request

• How easily could an exploit be constructed based on the patch?: It is probably not easy, as it is not very clear from the patch that and even less where we might UAF.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Should be very easy, the code in question has not changed since 2019 and we just add a condition to the if. The patch might even apply cleanly on ESR.
• How likely is this patch to cause regressions; how much testing does it need?: Very unlikely. If the actor is dead, nothing can or should be done with it anymore.
• Is Android affected?: Yes

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9358665 [details]
Bug 1859030 - Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r?#dom-worker-reviewers

Approved to land and uplift when ready

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49b36f40d55b
Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r=dom-worker-reviewers,asuth

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/49b36f40d55b

Comment 11

[Jens Stutte [:jstutte]]

Comment on attachment 9358665 [details]
Bug 1859030 - Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r?#dom-worker-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: sec-high bug, potential for UAF.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We just check if the actor is dead, and if it is, nothing can or should be done with it anymore.
• String changes made/needed:
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: sec-high bug, potential for UAF.
• Fix Landed on Version: 121
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We just check if the actor is dead, and if it is, nothing can or should be done with it anymore.

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9358665 [details]
Bug 1859030 - Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r?#dom-worker-reviewers

Approved for 120.0b3

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/e8477a337aaf

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9358665 [details]
Bug 1859030 - Exit early in WorkerManagerCreatedRunnable::Run if the actor cannot send. r?#dom-worker-reviewers

Approved for 115.5esr

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/a301997400cc

Comment 16

[Daniel Veditz [:dveditz]]

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter