1445440 - IPC: Assertion failure: mMyPid == base::GetCurrentProcId() [@mozilla::ipc::Endpoint]

Status: RESOLVED WORKSFORME

(Core :: IPC, defect, P2)

Description

[Christoph Diehl [:posidron]]

Assertion failure: mMyPid == base::GetCurrentProcId(), at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ipc/ProtocolUtils.h:666


==26941==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f5d4efe9713 bp 0x7f5d2ca3be50 sp 0x7f5d2ca3bd80 T43)
==26941==The signal is caused by a WRITE memory access.
==26941==Hint: address points to the zero page.
    #0 0x7f5d4efe9712 in mozilla::ipc::Endpoint<mozilla::ipc::PBackgroundParent>::Bind(mozilla::ipc::PBackgroundParent*) /builds/worker/workspace/build/src/ipc/glue/ProtocolUtils.h:665:9
    #1 0x7f5d4efe8a8a in (anonymous namespace)::ParentImpl::ConnectActorRunnable::Run() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1293:17
    #2 0x7f5d4e1406a6 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #3 0x7f5d4e15bc40 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:517:10
    #4 0x7f5d4f01416c in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:364:5
    #5 0x7f5d4ef618c9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #6 0x7f5d4ef618c9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #7 0x7f5d4ef618c9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #8 0x7f5d4e13b309 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:423:11
    #9 0x7f5d6c17547e in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #10 0x7f5d6f7d77fb in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x77fb)
    #11 0x7f5d6e805b5e in clone /build/glibc-itYbWN/glibc-2.26/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/ipc/glue/ProtocolUtils.h:665:9 in mozilla::ipc::Endpoint<mozilla::ipc::PBackgroundParent>::Bind(mozilla::ipc::PBackgroundParent*)
Thread T43 (IPDL Background) created by T0 here:
    #0 0x4b065d in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:204:3
    #1 0x7f5d6c1721cf in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
    #2 0x7f5d6c171dbe in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
    #3 0x7f5d4e13d103 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:594:8
    #4 0x7f5d4e1461fa in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:471:22
    #5 0x7f5d4e159cb4 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:143:45
    #6 0x7f5d4efe5737 in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:72:10
    #7 0x7f5d4efe5737 in (anonymous namespace)::ParentImpl::CreateBackgroundThread() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1015
    #8 0x7f5d4efc2f20 in CreateActorForSameProcess /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:925:32
    #9 0x7f5d4efc2f20 in GetOrCreateForCurrentThread /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1479
    #10 0x7f5d4efc2f20 in mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:721
    #11 0x7f5d53f4cf50 in mozilla::dom::ClientManager::ClientManager() /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:37:35
    #12 0x7f5d53f4f2f0 in mozilla::dom::ClientManager::GetOrCreateForCurrentThread() /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:188:14
    #13 0x7f5d53f439c1 in mozilla::dom::ClientManager::CreateSource(mozilla::dom::ClientType, nsISerialEventTarget*, nsIPrincipal*) /builds/worker/workspace/build/src/dom/clients/manager/ClientManager.cpp:237:31
    #14 0x7f5d596fca20 in nsDocShell::MaybeCreateInitialClientSource(nsIPrincipal*) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:2845:5
    #15 0x7f5d5973aeec in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIURI*, bool, bool) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7781:5
    #16 0x7f5d597bddcc in nsWebShellWindow::Initialize(nsIXULWindow*, nsIXULWindow*, nsIURI*, int, int, bool, nsITabParent*, mozIDOMWindowProxy*, nsWidgetInitData&) /builds/worker/workspace/build/src/xpfe/appshell/nsWebShellWindow.cpp:238:21
    #17 0x7f5d597b8631 in nsAppShellService::JustCreateTopWindow(nsIXULWindow*, nsIURI*, unsigned int, int, int, bool, nsITabParent*, mozIDOMWindowProxy*, nsWebShellWindow**) /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp:736:25
    #18 0x7f5d597b79b0 in nsAppShellService::CreateHiddenWindowHelper(bool) /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp:140:8
    #19 0x7f5d59fa5b4b in nsAppStartup::CreateHiddenWindow() /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:254:27
    #20 0x7f5d5a1ba82f in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4628:22
    #21 0x7f5d5a1bd538 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4872:8
    #22 0x7f5d5a1bec14 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4964:21
    #23 0x4f6d45 in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:231:22
    #24 0x4f6d45 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:304
    #25 0x7f5d6e7121c0 in __libc_start_main /build/glibc-itYbWN/glibc-2.26/csu/../csu/libc-start.c:308

==26941==ABORTING

Comment 1

[Christoph Diehl [:posidron]]

Attachment: session.txt

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

The fuzzer changed mMyPid, which is checked to make sure the Endpoint was sent to the correct process:

[time: 1520826270437225][27066->26941] [PBackgroundChild] Sending  PBackground::Msg_PIPCBlobInputStreamConstructor
[Faulty] pickle field {int} of value: 26941 changed to: 26940
[time: 1520826270437257][27066->26941] [PContentChild] Sending  PContent::Msg_InitBackground
[...]
Assertion failure: mMyPid == base::GetCurrentProcId(), at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ipc/ProtocolUtils.h:666

But… I have some concerns about what happens if mOtherPid is hostile, specifically on Windows.  If this is used unchecked to open that process and DuplicateHandle to/from it, there might be some way to manipulate the parent process to attack another child, or itself.

Bob, do you have a sense of how plausible that might be?

Comment 3

[Bob Owen (:bobowen)]

(In reply to Jed Davis [:jld] (⏰UTC-6) from comment #2)
... 
> But… I have some concerns about what happens if mOtherPid is hostile,
> specifically on Windows.  If this is used unchecked to open that process and
> DuplicateHandle to/from it, there might be some way to manipulate the parent
> process to attack another child, or itself.
> 
> Bob, do you have a sense of how plausible that might be?

It's been a while since I've looked at this IPC code, so I'm not quite sure what you think the attack is here.

From what I remember the creator of the Endpoints creates the two fd/HANDLEs and attempts to duplicate the destination one as it sends it over IPC.
On Windows, if that fails the destination would try and duplicate the HANDLE into itself from the source.

On posix, as I understand it, there isn't a concept of a process owning an fd so the initial dup always works, so I don't quite see how Windows is more exposed here.

I suppose that there might be a way for a compromised child to set up a rogue protocol, but posix would presumably be just as vulnerable to that.
I guess the destination must in some way at least be "expecting" the endpoint in order to bind with it.
It would probably be better if the master process always set up top level protocols.

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

Do we actually need to support serializing mOtherPid? It seems like the only case that'd be needed is if we supported sending Endpoints to a third-process, that was neither the creator nor the recipient. It seems like in the simple two process case you can always compute the pids: one is yourself and the other is whoever just sent you the Endpoint.

Comment 6

[Alex Gaynor [:Alex_Gaynor]]

From re-reading the comments here, it seems like the consensus is that there probably isn't a security issue here.

Christoph: are you still seeing these crashes though? Do we need to make this crash more fuzzer friendly somehow?

Comment 7

[Christoph Diehl [:posidron]]

Hm, so this has not been seen for quite some time.

Comment 8

[Alex Gaynor [:Alex_Gaynor]]

Looks like the code was rejiggered a bit: https://searchfox.org/mozilla-central/source/ipc/glue/ProtocolUtils.h#809-818

Going to declare this as worksforme at this point.