Closed Bug 1451376 Opened 6 years ago Closed 6 years ago

Use after free in ContentParent::AllocPPrintingParent

Categories

(Core :: Printing: Output, defect, P1)

Product:
Core
Component:
Printing: Output
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox-esr52 60+ fixed
firefox59 --- wontfix
firefox60 --- fixed
firefox61 + fixed

People

(Reporter: Alex_Gaynor, Assigned: bobowen)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-sandbox-escape, csectype-uaf, sec-high, Whiteboard: [adv-main60+][adv-esr52.8+][post-critsmash-triage])

Attachments

(1 file)

Properly enforce single PrintingParent per content process
1.71 KB, patch
jld
: review+
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr52+
abillings
: sec-approval+
Details | Diff | Splinter Review 
Discovered with my work-in-progress IPC fuzzer

Specifically the bug is: https://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#3434-3435

If you send two AllocPPrintingParent() messages in a row, then the second one will free the previous |mPrintingParent| value, however that Actor will still be registered in the routing table. The compromised child process can then send a message using the first printing parent's routing number, triggering teh UAF.


==18887==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000033c00 at pc 0x7f2dc8a0a275 bp 0x7ffc1b1af4b0 sp 0x7ffc1b1af4a8
READ of size 8 at 0x60e000033c00 thread T0
    #0 0x7f2dc8a0a274 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentParent.cpp:7458:28
    #1 0x7f2dd770aa86 in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/ProtocolFuzzer.h:39:17
    #2 0x7f2dd770a7b0 in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) /home/osboxes/mozilla-central/dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:29:3
    #3 0x5e655b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #4 0x5e3754 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #5 0x5e7db9 in fuzzer::Fuzzer::MutateAndTestOne() /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #6 0x5e9fa5 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #7 0x5cb8fa in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #8 0x7f2dd5df598f in mozilla::FuzzerRunner::Run(int*, char***) /home/osboxes/mozilla-central/tools/fuzzing/interface/harness/FuzzerRunner.cpp:60:10
    #9 0x7f2dd5d0c8da in XREMain::XRE_mainStartup(bool*) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:3960:35
    #10 0x7f2dd5d1f837 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:4896:12
    #11 0x7f2dd5d2139a in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:5003:21
    #12 0x51e145 in do_main(int, char**, char**) /home/osboxes/mozilla-central/browser/app/nsBrowserApp.cpp:231:22
    #13 0x51e145 in main /home/osboxes/mozilla-central/browser/app/nsBrowserApp.cpp:304
    #14 0x7f2deb8aa2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #15 0x425559 in _start (/home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/firefox+0x425559)

0x60e000033c00 is located 0 bytes inside of 152-byte region [0x60e000033c00,0x60e000033c98)
freed by thread T0 here:
    #0 0x4e3520 in __interceptor_cfree.localalias.0 (/home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/firefox+0x4e3520)
    #1 0x7f2dd0423fc2 in mozilla::embedding::PrintingParent::Release() /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/embedding/printingui/PrintingParent.h:30:5
    #2 0x7f2dd0423fc2 in mozilla::RefPtrTraits<mozilla::embedding::PrintingParent>::Release(mozilla::embedding::PrintingParent*) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:41
    #3 0x7f2dd0423fc2 in RefPtr<mozilla::embedding::PrintingParent>::ConstRemovingRefPtrTraits<mozilla::embedding::PrintingParent>::Release(mozilla::embedding::PrintingParent*) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:398
    #4 0x7f2dd0423fc2 in RefPtr<mozilla::embedding::PrintingParent>::assign_assuming_AddRef(mozilla::embedding::PrintingParent*) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:66
    #5 0x7f2dd0423fc2 in RefPtr<mozilla::embedding::PrintingParent>::assign_with_AddRef(mozilla::embedding::PrintingParent*) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:57
    #6 0x7f2dd0423fc2 in RefPtr<mozilla::embedding::PrintingParent>::operator=(mozilla::embedding::PrintingParent*) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:193
    #7 0x7f2dd0423fc2 in mozilla::dom::ContentParent::AllocPPrintingParent() /home/osboxes/mozilla-central/dom/ipc/ContentParent.cpp:3436
    #8 0x7f2dc89e3e42 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentParent.cpp:4022:21
    #9 0x7f2dd770aa3e in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/ProtocolFuzzer.h:41:17
    #10 0x7f2dd770a7b0 in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) /home/osboxes/mozilla-central/dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:29:3
    #11 0x5e655b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #12 0x5e3754 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #13 0x5e7db9 in fuzzer::Fuzzer::MutateAndTestOne() /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #14 0x5e9fa5 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #15 0x5cb8fa in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #16 0x7f2dd5df598f in mozilla::FuzzerRunner::Run(int*, char***) /home/osboxes/mozilla-central/tools/fuzzing/interface/harness/FuzzerRunner.cpp:60:10
    #17 0x7f2dd5d0c8da in XREMain::XRE_mainStartup(bool*) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:3960:35
    #18 0x7f2dd5d1f837 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:4896:12
    #19 0x7f2dd5d2139a in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:5003:21
    #20 0x51e145 in do_main(int, char**, char**) /home/osboxes/mozilla-central/browser/app/nsBrowserApp.cpp:231:22
    #21 0x51e145 in main /home/osboxes/mozilla-central/browser/app/nsBrowserApp.cpp:304
    #22 0x7f2deb8aa2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

previously allocated by thread T0 here:
    #0 0x4e36e8 in malloc (/home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/firefox+0x4e36e8)
    #1 0x51f29d in moz_xmalloc /home/osboxes/mozilla-central/memory/mozalloc/mozalloc.cpp:70:17
    #2 0x7f2dd0423e66 in operator new(unsigned long) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/mozalloc.h:156:12
    #3 0x7f2dd0423e66 in mozilla::dom::ContentParent::AllocPPrintingParent() /home/osboxes/mozilla-central/dom/ipc/ContentParent.cpp:3436
    #4 0x7f2dc89e3e42 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentParent.cpp:4022:21
    #5 0x7f2dd770aa3e in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long) /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/ProtocolFuzzer.h:41:17
    #6 0x7f2dd770a7b0 in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) /home/osboxes/mozilla-central/dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:29:3
    #7 0x5e655b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #8 0x5e3754 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #9 0x5e7db9 in fuzzer::Fuzzer::MutateAndTestOne() /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #10 0x5e9fa5 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #11 0x5cb8fa in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/osboxes/mozilla-central/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #12 0x7f2dd5df598f in mozilla::FuzzerRunner::Run(int*, char***) /home/osboxes/mozilla-central/tools/fuzzing/interface/harness/FuzzerRunner.cpp:60:10
    #13 0x7f2dd5d0c8da in XREMain::XRE_mainStartup(bool*) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:3960:35
    #14 0x7f2dd5d1f837 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:4896:12
    #15 0x7f2dd5d2139a in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/osboxes/mozilla-central/toolkit/xre/nsAppRunner.cpp:5003:21
    #16 0x51e145 in do_main(int, char**, char**) /home/osboxes/mozilla-central/browser/app/nsBrowserApp.cpp:231:22
    #17 0x51e145 in main /home/osboxes/mozilla-central/browser/app/nsBrowserApp.cpp:304
    #18 0x7f2deb8aa2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

SUMMARY: AddressSanitizer: heap-use-after-free /home/osboxes/mozilla-central/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentParent.cpp:7458:28 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&)
Shadow bytes around the buggy address:
  0x0c1c7fffe730: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1c7fffe740: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1c7fffe750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1c7fffe760: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1c7fffe770: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
=>0x0c1c7fffe780:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1c7fffe790: fd fd fd fa fa fa fa fa fa fa fa fa 00 00 00 00
  0x0c1c7fffe7a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c1c7fffe7b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1c7fffe7c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1c7fffe7d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==18887==ABORTING
MS: 1 ShuffleBytes-; base unit: 06fe662718cd06add4fba4aa5f2a2e123cbd1b7e
0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0xef,0xfd,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x29,0x0,0x9b,0xe6,0xa,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x4,0x0,0x0,0x0,0xff,0xff,0xff,0x7f,0x79,0x0,0x2d,0x0,0x0,0x0,0xff,0xff,0xff,0x2,0x0,0x78,0x22,0xff,0x0,0x0,0x0,0x0,0x0,
\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xef\xfd\x00\x00\x00\x00\x00\x00\x00)\x00\x9b\xe6\x0a\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\xff\xff\xff\x7fy\x00-\x00\x00\x00\xff\xff\xff\x02\x00x\"\xff\x00\x00\x00\x00\x00
artifact_prefix='./'; Test unit written to ./crash-06fe662718cd06add4fba4aa5f2a2e123cbd1b7e
Base64: AAAAAAAAAAAAAAAAAAAAAO/9AAAAAAAAACkAm+YKAAAAAAAAAAQAAAD///9/eQAtAAAA////AgB4Iv8AAAAAAA==
Group: toolkit-core-security → layout-core-security
Component: Printing → Printing: Output
Keywords: csectype-uaf, sec-high
Product: Toolkit → Core
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Priority: -- → P1
I've also included taking and releasing references on behalf of IPDL code.
The release asserts render that unnecessary, but either way it's good to have in case other people copy my mistake.
Attachment #8965638 - Flags: review?(jld)
Attachment #8965638 - Flags: review?(jld) → review+
Comment on attachment 8965638 [details] [diff] [review]
Properly enforce single PrintingParent per content process

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
This is an issue in the parent process and requires an already compromised content process.
So, in theory it might be used as part of a sandbox escape, although I imagine turning the UAF into code execution wouldn't be easy, but I'm far from an expert on such things.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
It's pretty obvious what the issue is from the patch.

Which older supported branches are affected by this flaw?
Change landed in 49, so all currently supported branches.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch applies cleanly to mozilla-beta, mozilla-release and mozilla-esr52.

How likely is this patch to cause regressions; how much testing does it need?
The only slight possibility is if we get crashes from the MOZ_RELEASE_ASSERTs, but the MOZ_ASSERTs have been there for a long time, so that seems unlikely.
Attachment #8965638 - Flags: sec-approval?
sec-approval+ for trunk.
Please nominate for Beta and ESR52 so we can take it everywhere.
status-firefox59: affected → wontfix
tracking-firefox60: + → ---
tracking-firefox-esr52: ? → 60+
Attachment #8965638 - Flags: sec-approval? → sec-approval+
Comment on attachment 8965638 [details] [diff] [review]
Properly enforce single PrintingParent per content process

This is sec-high so I think the beta questions cover esr as well.

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1189846.

[User impact if declined]:
Use after free with potential utility as a sandbox escape.

[Is this code covered by automated tests?]:
Not directly although the code is executed on every content process start-up.

[Has the fix been verified in Nightly?]:
Not landed yet.

[Needs manual test from QE? If yes, steps to reproduce]: 
No, but I imagine it could be tested using the fuzzer.

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
Very slightly

[Why is the change risky/not risky?]:
The only real risk is that we might hit the release asserts and crash, but they have been there as debug asserts for a long time, so that seems very unlikely and it is something we would want to catch anyway.

[String changes made/needed]:
None
Attachment #8965638 - Flags: approval-mozilla-esr52?
Attachment #8965638 - Flags: approval-mozilla-beta?
Backed out changeset c905d37ed54a (bug 1451376) for bustage on workspace/build/src/dom/ipc/ContentParent.cpp
Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/3d3b88d74ad49ffe1365cf02ff96303d3fc969e8
Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c905d37ed54a1cc5cb6fc067b30dd17260aa6945

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=172864518&repo=mozilla-inbound&lineNumber=25334

Log snippet:
[task 2018-04-10T13:06:58.928Z] 13:06:58     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dom/ipc/Unified_cpp_dom_ipc0.cpp:47:
[task 2018-04-10T13:06:58.928Z] 13:06:58     INFO -  /builds/worker/workspace/build/src/dom/ipc/ContentParent.cpp:3418:3: error: 'AddRef' cannot be called on the return value of 'operator->'
Flags: needinfo?(bobowencode)
Thanks, looks like I can just .get() first.
I re-land once I've got my Linux compile going locally.
Flags: needinfo?(bobowencode)
https://hg.mozilla.org/mozilla-central/rev/6e0fe40d8a55
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8965638 [details] [diff] [review]
Properly enforce single PrintingParent per content process

Low-risk sec-high fix. Approved for 60.0b12 and ESR 52.8.0.
Attachment #8965638 - Flags: approval-mozilla-esr52?
Attachment #8965638 - Flags: approval-mozilla-esr52+
Attachment #8965638 - Flags: approval-mozilla-beta?
Attachment #8965638 - Flags: approval-mozilla-beta+
Group: layout-core-security → core-security-release
Whiteboard: [adv-main60+][adv-esr52.8+]
Flags: qe-verify-
Whiteboard: [adv-main60+][adv-esr52.8+] → [adv-main60+][adv-esr52.8+][post-critsmash-triage]
Blocks: libfuzzer-ipc
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: