1765336 - Crash [@ nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_RelocateUsingMemutils>::Length] through [@ mozilla::dom::indexedDB::Database::RecvBlocked]

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect, P3)

Description

[Christian Holler (:decoder)]

In experimental IPC fuzzing, we found the following crash on mozilla-central revision 4d80f4e1809a+:

=================================================================
==1730==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000e8 (pc 0x7fffdbf9a6a6 bp 0x7fffb805e330 sp 0x7fffb805e220 T6)
    #0 0x7fffdbf9a6a6 in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_RelocateUsingMemutils>::Length() const objdir-ff-asan/dist/include/nsTArray.h:410:37
    #1 0x7fffdbf9a6a6 in nsTArray_Impl<mozilla::dom::indexedDB::(anonymous namespace)::FactoryOp::MaybeBlockedDatabaseInfo, nsTArrayInfallibleAllocator>::end() objdir-ff-asan/dist/include/nsTArray.h:1266:57
    #2 0x7fffdbf9a6a6 in mozilla::dom::indexedDB::(anonymous namespace)::FactoryOp::NoteDatabaseBlocked(mozilla::dom::indexedDB::(anonymous namespace)::Database*) dom/indexedDB/ActorsParent.cpp:15072:19
    #3 0x7fffdbf9a6a6 in mozilla::dom::indexedDB::(anonymous namespace)::Database::RecvBlocked() dom/indexedDB/ActorsParent.cpp:10179:28
    #4 0x7fffdc570af9 in mozilla::dom::indexedDB::PBackgroundIDBDatabaseParent::OnMessageReceived(IPC::Message const&) objdir-ff-asan/ipc/ipdl/PBackgroundIDBDatabaseParent.cpp:702:28
    #5 0x7fffccc653ac in mozilla::ipc::PBackgroundParent::OnMessageReceived(IPC::Message const&) objdir-ff-asan/ipc/ipdl/PBackgroundParent.cpp:3445:32
    #6 0x7fffcca8e1c0 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) ipc/glue/MessageChannel.cpp:1739:25
    [...]

We don't have a testcase for this issue yet and I can't reproduce it in replay mode either. It seems the crash is caused by a missing null check here:

https://searchfox.org/mozilla-central/rev/4b3039b48c3cb67774270ebcc2a7d8624d888092/dom/indexedDB/ActorsParent.cpp#10155

Note that we are sending IPC events directly (so this is not a regular Firefox run but instead assuming a compromised content child sending bogus data).

This issue looks harmless but IPC fuzzing hasn't landed yet, so marking s-s until the parent bug is landed.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Jens Stutte [:jstutte]]

So the normal way we call SendBlocked is checking, if we are closed already. I assume the IPC fuzzer just calls it out of the expected order. We should probably just transform this into a real if and return an IPC_FAIL in case.

Comment 3

[Christian Holler (:decoder)]

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

So the normal way we call SendBlocked is checking, if we are closed already. I assume the IPC fuzzer just calls it out of the expected order. We should probably just transform this into a real if and return an IPC_FAIL in case.

Yea, this sounds good. We can also still leave the MOZ_ASSERT in there to detect problems that should not occur outside of fuzzing.

Comment 4

[Jens Stutte [:jstutte]]

(In reply to Christian Holler (:decoder) from comment #3)

Yea, this sounds good. We can also still leave the MOZ_ASSERT in there to detect problems that should not occur outside of fuzzing.

IPC_FAIL does already do MOZ_ASSERT_UNLESS_FUZZING.

Comment 5

[Jens Stutte [:jstutte]]

Attachment: Bug 1765336: Add IPC_FAIL conditions to RecvBlocked. r?#dom-storage-reviewers

Comment 6

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

Add IPC_FAIL conditions to RecvBlocked. r=dom-storage-reviewers,janv
https://hg.mozilla.org/integration/autoland/rev/583898d054c27f611fd4a7f787f43555aef89e06
https://hg.mozilla.org/mozilla-central/rev/583898d054c2