1723954 - IPC Parent Crash [@ mozilla::ipc::(anonymous namespace)::CheckPrincipalRunnable::Run]

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Christian Holler (:decoder)]

In experimental IPC fuzzing, we found the following crash on mozilla-central revision 160071ad9de0+ (--enable-address-sanitizer --enable-optimize --enable-fuzzing --disable-debug build):

==1687==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fffdc8a8876 bp 0x7fffffffabf0 sp 0x7fffffffaa80 T0)
==1687==The signal is caused by a READ memory access.
==1687==Hint: address points to the zero page.
    #0 0x7fffdc8a8876 in mozilla::ipc::(anonymous namespace)::CheckPrincipalRunnable::Run() ipc/glue/BackgroundParentImpl.cpp:902:44
    #1 0x7fffdb1b65b2 in mozilla::RunnableTask::Run() xpcom/threads/TaskController.cpp:502:16
    #2 0x7fffdb16dc60 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) xpcom/threads/TaskController.cpp:805:26
    #3 0x7fffdb169d27 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) xpcom/threads/TaskController.cpp:641:15
    #4 0x7fffdb1aa609 in mozilla::TaskController::ProcessPendingMTTask(bool) xpcom/threads/TaskController.cpp:425:36
    #5 0x7fffdb1aa609 in mozilla::TaskController::InitializeInternal()::$_0::operator()() const xpcom/threads/TaskController.cpp:135:37
    #6 0x7fffdb1aa609 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() xpcom/threads/nsThreadUtils.h:532:5
    #7 0x7fffdb18f28e in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1148:16
    #8 0x7fffdb19c810 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:466:10
    #9 0x7fffdc94de97 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:85:21
    [...]

We don't have a testcase for this issue yet but I looked into the crash:

We seem to crash when trying to unwrap principalOrErr in line 902, but a few lines before that, we checked for isErr() without returning in that error case:

https://searchfox.org/mozilla-central/rev/064a1e6a2a6f6aa30be8bf4edea2f8408f779d4d/ipc/glue/BackgroundParentImpl.cpp#896-899,902

Given the surrounding code, it seems to me that there is a return NS_OK; missing after line 898.

Not a security bug, but marking s-s because we currently don't want to disclose our efforts around IPC fuzzing (until we have it in production).

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Andrew McCreight [:mccr8]]

This looks like a regression from bug 1635399. Christoph, do you know who might be able to look at this? Thanks.

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Andrew McCreight [:mccr8] from comment #2)

This looks like a regression from bug 1635399. Christoph, do you know who might be able to look at this? Thanks.

Niklas, since you are working on improving parts of our principal handling - can you take a look at this one please? Thank you!

Comment 4

[Niklas]

Sure i will have a look.

Comment 5

[Niklas]

Attachment: Bug 1723954: Return NS_OK if PrincipalInfoToPrincipal fails. r=ckerschb

Comment 6

[Niklas]

What Christian suggested seems to be the cause for this crash.
Result::unwrap has a MOZ_ASSERT(isOk()) which would have been hit in a debug build.
Given the Result::isErr() check here we need to return early if Result::isErr() returns true, otherwise we call Result::unwrap even though Result::isOk() returns false.

Comment 7

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

https://hg.mozilla.org/integration/autoland/rev/f5921ffeaee4b8b096f9999d4171fee1c3e1c57a
https://hg.mozilla.org/mozilla-central/rev/f5921ffeaee4