Closed Bug 1577558 Opened 4 months ago Closed 3 months ago

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

Categories

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

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 - fixed
firefox68 - wontfix
firefox69 - wontfix
firefox70 + fixed
firefox71 + verified

People

(Reporter: posidron, Assigned: ehsan)

References

(Regression, )

Details

(Keywords: oss-fuzz, regression)

Attachments

(1 file)

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

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

Flags: needinfo?(cdiehl)

Nevermind.

Flags: needinfo?(cdiehl)

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

Assignee: nobody → ehsan
Status: NEW → ASSIGNED

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

Flags: needinfo?(ehsan)

(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.

Flags: needinfo?(ehsan)
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/63cf143e208d
Add proper argument checking in AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(); r=johannh

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 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
Attachment #9089571 - Flags: approval-mozilla-release?
Attachment #9089571 - Flags: approval-mozilla-esr68?
Attachment #9089571 - Flags: approval-mozilla-beta?

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

Flags: needinfo?(cdiehl)

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

Per comment 8.

Attachment #9089571 - Flags: approval-mozilla-release? → approval-mozilla-release-
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Following up in email with cdiehl.

:ehsan, this seems fixed.

Flags: needinfo?(cdiehl)

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

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

Attachment #9089571 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Fixes an IPC crash. Approved for 68.2esr.

Attachment #9089571 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
You need to log in before you can comment on or make changes to this bug.