1577558 - IPC: crash [@mozilla::AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess]

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect)

Description

[Christoph Diehl [:posidron]]

Task

Item	Description
Crash Type	Null-dereference READ
Sanitizer	address (ASAN)
Platform	linux
Job Type	libfuzzer_asan_firefox
Fuzz Target	ContentParentIPC

Callstack

==1==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f738af3f5e9 bp 0x7ffe7502c7f0 sp 0x7ffe7502c500 T0)
==1==The signal is caused by a READ memory access.
==1==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7f738af3f5e8 in mozilla::AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(nsIPrincipal*, nsIPrincipal*, nsTString<char> const&, nsTString<char> const&, int) mozilla-central/toolkit/components/antitracking/AntiTrackingCommon.cpp:1164:31
    #1 0x7f738817cb7b in mozilla::dom::ContentParent::RecvFirstPartyStorageAccessGrantedForOrigin(IPC::Principal const&, IPC::Principal const&, nsTString<char> const&, nsTString<char> const&, int const&, std::function<void (bool const&)>&&) mozilla-central/dom/ipc/ContentParent.cpp:5760:3
    #2 0x7f738338c5f2 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /work/obj-fuzz/ipc/ipdl/PContentParent.cpp:10568:57
    #3 0x7f7381baded2 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
    #4 0x7f7381bad7e8 in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) mozilla-central/dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:27:3
    #5 0x5641869d529f in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)
    #6 0x5641869c19de in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long)
    #7 0x5641869c3c99 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))
    #8 0x7f738b3f2791 in mozilla::FuzzerRunner::Run(int*, char***) mozilla-central/tools/fuzzing/interface/harness/FuzzerRunner.cpp:61:10
    #9 0x7f738b3436ae in XREMain::XRE_mainStartup(bool*) mozilla-central/toolkit/xre/nsAppRunner.cpp:3758:35
    #10 0x7f738b34be15 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) mozilla-central/toolkit/xre/nsAppRunner.cpp:4698:12
    #11 0x7f738b34c7c1 in XRE_main(int, char**, mozilla::BootstrapConfig const&) mozilla-central/toolkit/xre/nsAppRunner.cpp:4792:21
    #12 0x5641868d9b4a in do_main(int, char**, char**)
    #13 0x5641868d9332 in main
    #14 0x7f739d06582f in __libc_start_main
    #15 0x5641867fb028 in _start
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/mnt/scratch0/clusterfuzz/bot/builds/clusterfuzz-builds_firefox_6180546f5e5f1d75138bf6cea3784693af7a2aa9/revisions/firefox/libxul.so+0x142fc5e8)
==1==ABORTING

Comment 1

[(no longer active)]

How can I get access to the test case for this bug?

Comment 2

[(no longer active)]

Nevermind.

Comment 3

[(no longer active)]

Attachment: Bug 1577558 - Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess();

Comment 4

[(no longer active)]

[Tracking Requested - why for this release]: A compromised content process can crash the parent process.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Is there any reason to believe that we're seeing this bug in the wild?

Comment 6

[(no longer active)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

Is there any reason to believe that we're seeing this bug in the wild?

None as far as I can tell. But also note that the fix is virtually risk-free. The impact of this bug is just a DOS of the parent process in case a malicious web page takes over the content process, it's the kind of fix that I'd recommend to take as potential ride-alongs to a dot-release for the release branch as well as as an IPC hardening fix on ESR.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63cf143e208d
Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(); r=johannh

Comment 8

[Ryan VanderMeulen [:RyanVM]]

I think it makes sense to take this fix in 70/68.2esr, but I'm not seeing the urgency to take it in a dot release in the mean time.

Comment 9

[(no longer active)]

Comment on attachment 9089571 [details]
Bug 1577558 - Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess();

Beta/Release Uplift Approval Request

• User impact if declined: A malicious web page can cause Firefox (the entire browser) to crash.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• 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): This patch only adds a null check for the arguments we receive through IPC from the content process.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: A malicious web page can cause Firefox (the entire browser) to crash.
• User impact if declined: See above.
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only adds a null check for the arguments we receive through IPC from the content process.
• String or UUID changes made by this patch: None

Comment 10

[(no longer active)]

Hey Christoph, can oss-fuzz verify the fix? Thanks!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9089571 [details]
Bug 1577558 - Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess();

Per comment 8.

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/63cf143e208d

Comment 13

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Following up in email with cdiehl.

Comment 14

[Christoph Diehl [:posidron]]

:ehsan, this seems fixed.

Comment 15

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 9089571 [details]
Bug 1577558 - Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess();

Verified in nightly, let's uplift for beta 5.

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/67939ba14176

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9089571 [details]
Bug 1577558 - Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess();

Fixes an IPC crash. Approved for 68.2esr.

Comment 18

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/da25e3f701cc