1578458 - MOZ_CRASH("Origin must be available when deserialized") on malicious IPC input [@mozilla::ipc::PrincipalInfoToPrincipal]

Status: RESOLVED DUPLICATE of bug 1609951

(Core :: DOM: Content Processes, defect, P5)

Description

[Christoph Diehl [:posidron]]

Attachment: clusterfuzz-testcase-ContentParentIPC-5749790154948608

Task

Item	Description
Crash Type	Null-dereference WRITE
Sanitizer	address (ASAN)
Platform	linux
Job Type	libfuzzer_asan_firefox
Fuzz Target	ContentParentIPC
Reliably Reproduces	YES

Callstack

==1==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f801f065fb7 bp 0x7ffde3dc4350 sp 0x7ffde3dc3f20 T0)
==1==The signal is caused by a WRITE memory access.
==1==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7f801f065fb6 in mozilla::ipc::PrincipalInfoToPrincipal(mozilla::ipc::PrincipalInfo const&, nsresult*) mozilla-central/ipc/glue/BackgroundUtils.cpp:98:9
    #1 0x7f801f07032f in mozilla::ipc::LoadInfoArgsToLoadInfo(mozilla::Maybe<mozilla::net::LoadInfoArgs> const&, nsILoadInfo**) mozilla-central/ipc/glue/BackgroundUtils.cpp:614:7
    #2 0x7f8020103a66 in mozilla::dom::ExternalHelperAppParent::Init(mozilla::Maybe<mozilla::net::LoadInfoArgs> const&, nsTString<char> const&, bool const&, mozilla::Maybe<mozilla::ipc::URIParams> const&, mozilla::dom::PBrowserParent*) mozilla-central/uriloader/exthandler/ExternalHelperAppParent.cpp:91:3
    #3 0x7f802417b1ab in mozilla::dom::ContentParent::RecvPExternalHelperAppConstructor(mozilla::dom::PExternalHelperAppParent*, mozilla::Maybe<mozilla::ipc::URIParams> const&, mozilla::Maybe<mozilla::net::LoadInfoArgs> const&, nsTString<char> const&, nsTString<char> const&, unsigned int const&, nsTString<char16_t> const&, bool const&, long const&, bool const&, mozilla::Maybe<mozilla::ipc::URIParams> const&, mozilla::dom::PBrowserParent*) mozilla-central/dom/ipc/ContentParent.cpp:3736:49
    #4 0x7f801f399da1 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /work/obj-fuzz/ipc/ipdl/PContentParent.cpp:7830:57
    #5 0x7f801dbc8b02 in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long, nsTArray<nsTString<char> > const&) /work/obj-fuzz/dist/include/ProtocolFuzzer.h:96:18
    #6 0x7f801dbc8418 in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) mozilla-central/dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:27:3
    #7 0x55770843429f in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)
    #8 0x5577084209de in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long)
    #9 0x557708422c99 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))
    #10 0x7f8027409621 in mozilla::FuzzerRunner::Run(int*, char***) mozilla-central/tools/fuzzing/interface/harness/FuzzerRunner.cpp:61:10
    #11 0x7f802735a45e in XREMain::XRE_mainStartup(bool*) mozilla-central/toolkit/xre/nsAppRunner.cpp:3759:35
    #12 0x7f8027362bc5 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) mozilla-central/toolkit/xre/nsAppRunner.cpp:4708:12
    #13 0x7f8027363571 in XRE_main(int, char**, mozilla::BootstrapConfig const&) mozilla-central/toolkit/xre/nsAppRunner.cpp:4802:21
    #14 0x557708338b4a in do_main(int, char**, char**)
    #15 0x557708338332 in main
    #16 0x7f80390b682f in __libc_start_main
    #17 0x55770825a028 in _start
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/mnt/scratch0/clusterfuzz/bot/builds/clusterfuzz-builds_firefox_6180546f5e5f1d75138bf6cea3784693af7a2aa9/revisions/firefox/libxul.so+0xc463fb6)
==1==ABORTING

Comment 1

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

Like most writes to address 0, this is a deliberate crash. And in this case it's one that's enabled on all builds:

MOZ_CRASH("Origin must be available when deserialized");

So it looks like just denial-of-service by a hostile content process, which isn't a very interesting finding. The ownership around PBackground is fuzzy, but I'm moving this to DOM: Content Processes because that's probably where people who know about this code are (see also bug 1024098).

Comment 2

[Wayne Mery (:wsmwk)]

Friday had a massive spike. See the graph https://crash-stats.mozilla.org/signature/?signature=mozilla%3A%3Aipc%3A%3APrincipalInfoToPrincipal&date=%3E%3D2019-08-23T13%3A36%3A00.000Z&date=%3C2019-09-23T13%3A36%3A00.000Z#graphs

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[(no longer active)]

(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #1)

Like most writes to address 0, this is a deliberate crash. And in this case it's one that's enabled on all builds:

MOZ_CRASH("Origin must be available when deserialized");

So it looks like just denial-of-service by a hostile content process

That indeed seems to be the case here.

which isn't a very interesting finding.

Why isn't that an interesting finding? A compromised content process shouldn't be able to crash the parent process, that reverts some of the stability advantages that e10s provides for us. Generally we should treat IPC input from the content process as potentially garbage data and not crash the parent process when the validation of that data fails.

Comment 6

[Jim Mathies [:jimm]]

-> P3, crash / DOS but not an exploit. DOS issues are secondary right now to hardening our ipc exposure.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

A compromised content process shouldn't be able to crash the parent process, that reverts some of the stability advantages that e10s provides for us.

There's stability advantages for unintentional bugs, but this is an intentional crash when we detect a possible compromise. While in theory it would be nice if we would just ignore/kill off compromised children, I think we're fine with not getting the entire machine compromised as a first step. (It also makes attacks a little more annoying because you don't get to retry - the user will notice something is wrong when the entire browser dies)

In terms of fuzzing searching for actual vulnerabilities, this is indeed not interesting. It shows that the security controls work.

Comment 8

[Christian Holler (:decoder)]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #7)

A compromised content process shouldn't be able to crash the parent process, that reverts some of the stability advantages that e10s provides for us.

There's stability advantages for unintentional bugs, but this is an intentional crash when we detect a possible compromise.

Why would this not be an unintentional bug, if the child (unintentionally) sends the wrong data here? According to crash-stats (see comment 2) this even happens in production.

Comment 9

[Wayne Mery (:wsmwk)]

question comment 8

Comment 10

[Gian-Carlo Pascutto [:gcp]]

The parent can't tell the difference. If we're not sure what of the options it is, the safe thing to do is to crash. The best thing to do is to fix the child to not send wrong data. A second option is to make the parent ignore the invalid data - if we're confident we're correcting dealing with all cases. But I don't see why we'd pick option 2 over option 1. The reason why we have this crash is exactly because option 2 is hard to do right and the consequences for not doing it right are Bad.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

decoder told me in chat that the problem is that libfuzzer is in-process, so the process dying means a very expensive full restart, which means these MOZ_CRASH make fuzzing prohibitive.

About the best we could come up with is to simply fail and bail in the simplest possible manner if FUZZING is enabled, instead of proceeding to the MOZ_CRASH.

Comment 12

[Gian-Carlo Pascutto [:gcp]]

Re-triaging for sandboxing/hardening team as this is a fuzzing blocker.

Comment 13

[Gian-Carlo Pascutto [:gcp]]

Looking at crash-stats, the crash is hit in the field from a reasonable variety of callers - indicating the calling code here is buggy in quite a few places. So after we fix the fuzzing issue I think we can close this bug - or at least separate bugs should be filed for all the callers. Then there's still a call whether to crash/fail those...

Comment 14

[Gian-Carlo Pascutto [:gcp]]

The different callers. Does seem like ~3 are responsible for the majority of crashes:
https://crash-stats.mozilla.com/search/?signature=~PrincipalInfoToPrincipal&date=%3E%3D2019-11-01T16%3A19%3A00.000Z&date=%3C2019-11-25T16%3A19%3A00.000Z&_facets=signature&_facets=proto_signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=proto_signature#facet-proto_signature

Comment 15

[Gian-Carlo Pascutto [:gcp]]

With the fuzzing blocker going into a split off bug this is not urgent any more.

Comment 16

[Pascal Chevrel:pascalc]

P5, wontfix for release

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 18

[Jens Stutte [:jstutte]]

Is this to be considered "early IPC fuzzing" ? Could you check if this is still a thing with our current tool chain?

Comment 19

[Jens Stutte [:jstutte]]

I believe this is a duplicate of bug 1609951 which has been fixed by bug 1635399.