1544526 - IPC: heap-use-after-free crash [@mozilla::net::nsHttpHandler::EnsureHSTSDataReadyNative]

Reporter

Description

•

5 years ago

The following testcase crashes on mozilla-central revision 20190412-412447b6149e

See attachment.

Backtrace:

==51==ERROR: AddressSanitizer: heap-use-after-free on address 0x6040031c09e8 at pc 0x7f3f38c7e638 bp 0x7ffd90d1a690 sp 0x7ffd90d1a688
READ of size 8 at 0x6040031c09e8 thread T0
SCARINESS: 51 (8-byte-read-heap-use-after-free)
    #0 0x7f3f38c7e637 in _M_empty clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:1694:37
    #1 0x7f3f38c7e637 in operator() clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:2125
    #2 0x7f3f38c7e637 in DoCallback netwerk/protocol/http/nsHttpHandler.h:781
    #3 0x7f3f38c7e637 in mozilla::net::nsHttpHandler::EnsureHSTSDataReadyNative(mozilla::net::HSTSDataCallbackWrapper*) netwerk/protocol/http/nsHttpHandler.cpp:2615
    #4 0x7f3f390ba7aa in mozilla::net::NeckoParent::RecvEnsureHSTSData(std::function<void (bool const&)>&&) netwerk/ipc/NeckoParent.cpp:985:17
    #5 0x7f3f39bf4ad1 in mozilla::net::PNeckoParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PNeckoParent.cpp:3123:55
    #6 0x7f3f3980fac8 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PContentParent.cpp:4807:28
    #7 0x7f3f48aeb35a in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long, nsTArray<nsTString<char> > const&) obj-firefox/dist/include/ProtocolFuzzer.h:107:18
    #8 0x7f3f48aea90a in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:27:3
    #9 0x5588086ac3ad in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #10 0x5588086abc05 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #11 0x5588086ad05d in fuzzer::Fuzzer::MutateAndTestOne() tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #12 0x5588086ad935 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&) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #13 0x5588086a54c6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #14 0x7f3f47018f19 in mozilla::FuzzerRunner::Run(int*, char***) tools/fuzzing/interface/harness/FuzzerRunner.cpp:61:10
    #15 0x7f3f46f2281d in XREMain::XRE_mainStartup(bool*) toolkit/xre/nsAppRunner.cpp:3740:35
    #16 0x7f3f46f3b30a in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4697:12
    #17 0x7f3f46f3cf59 in XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4791:21
    #18 0x55880860c64c in do_main browser/app/nsBrowserApp.cpp:212:22
    #19 0x55880860c64c in main browser/app/nsBrowserApp.cpp:291
    #20 0x7f3f5eddfb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #21 0x558808531ebc in _start (/home/worker/firefox/firefox+0x2debc)

DEDUP_TOKEN: _M_empty
0x6040031c09e8 is located 24 bytes inside of 40-byte region [0x6040031c09d0,0x6040031c09f8)
freed by thread T0 here:
    #0 0x5588085d99e2 in free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7f3f38c81cc4 in operator delete obj-firefox/dist/include/mozilla/mozalloc.h:151:10
    #2 0x7f3f38c81cc4 in Release netwerk/protocol/http/nsHttpHandler.h:772
    #3 0x7f3f38c81cc4 in Release obj-firefox/dist/include/mozilla/RefPtr.h:46
    #4 0x7f3f38c81cc4 in Release obj-firefox/dist/include/mozilla/RefPtr.h:363
    #5 0x7f3f38c81cc4 in ~RefPtr obj-firefox/dist/include/mozilla/RefPtr.h:77
    #6 0x7f3f38c81cc4 in ~ netwerk/protocol/http/nsHttpHandler.cpp:2608
    #7 0x7f3f38c81cc4 in _M_destroy clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:1586
    #8 0x7f3f38c81cc4 in std::_Function_base::_Base_manager<mozilla::net::nsHttpHandler::EnsureHSTSDataReadyNative(mozilla::net::HSTSDataCallbackWrapper*)::$_0>::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:1610
    #9 0x7f3f38c7e35a in ~_Function_base clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:1690:2
    #10 0x7f3f38c7e35a in mozilla::net::nsHttpHandler::EnsureHSTSDataReadyNative(mozilla::net::HSTSDataCallbackWrapper*) netwerk/protocol/http/nsHttpHandler.cpp:2611
    #11 0x7f3f390ba7aa in mozilla::net::NeckoParent::RecvEnsureHSTSData(std::function<void (bool const&)>&&) netwerk/ipc/NeckoParent.cpp:985:17
    #12 0x7f3f39bf4ad1 in mozilla::net::PNeckoParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PNeckoParent.cpp:3123:55
    #13 0x7f3f3980fac8 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PContentParent.cpp:4807:28
    #14 0x7f3f48aeb35a in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long, nsTArray<nsTString<char> > const&) obj-firefox/dist/include/ProtocolFuzzer.h:107:18
    #15 0x7f3f48aea90a in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:27:3
    #16 0x5588086ac3ad in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #17 0x5588086abc05 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #18 0x5588086ad05d in fuzzer::Fuzzer::MutateAndTestOne() tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #19 0x5588086ad935 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&) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #20 0x5588086a54c6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #21 0x7f3f47018f19 in mozilla::FuzzerRunner::Run(int*, char***) tools/fuzzing/interface/harness/FuzzerRunner.cpp:61:10
    #22 0x7f3f46f2281d in XREMain::XRE_mainStartup(bool*) toolkit/xre/nsAppRunner.cpp:3740:35
    #23 0x7f3f46f3b30a in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4697:12
    #24 0x7f3f46f3cf59 in XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4791:21
    #25 0x55880860c64c in do_main browser/app/nsBrowserApp.cpp:212:22
    #26 0x55880860c64c in main browser/app/nsBrowserApp.cpp:291
    #27 0x7f3f5eddfb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

DEDUP_TOKEN: free
previously allocated by thread T0 here:
    #0 0x5588085d9d63 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x55880860e5fd in moz_xmalloc memory/mozalloc/mozalloc.cpp:68:15
    #2 0x7f3f390ba562 in operator new obj-firefox/dist/include/mozilla/mozalloc.h:131:10
    #3 0x7f3f390ba562 in mozilla::net::NeckoParent::RecvEnsureHSTSData(std::function<void (bool const&)>&&) netwerk/ipc/NeckoParent.cpp:986
    #4 0x7f3f39bf4ad1 in mozilla::net::PNeckoParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PNeckoParent.cpp:3123:55
    #5 0x7f3f3980fac8 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PContentParent.cpp:4807:28
    #6 0x7f3f48aeb35a in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long, nsTArray<nsTString<char> > const&) obj-firefox/dist/include/ProtocolFuzzer.h:107:18
    #7 0x7f3f48aea90a in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:27:3
    #8 0x5588086ac3ad in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #9 0x5588086abc05 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #10 0x5588086ad05d in fuzzer::Fuzzer::MutateAndTestOne() tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #11 0x5588086ad935 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&) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #12 0x5588086a54c6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #13 0x7f3f47018f19 in mozilla::FuzzerRunner::Run(int*, char***) tools/fuzzing/interface/harness/FuzzerRunner.cpp:61:10
    #14 0x7f3f46f2281d in XREMain::XRE_mainStartup(bool*) toolkit/xre/nsAppRunner.cpp:3740:35
    #15 0x7f3f46f3b30a in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4697:12
    #16 0x7f3f46f3cf59 in XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4791:21
    #17 0x55880860c64c in do_main browser/app/nsBrowserApp.cpp:212:22
    #18 0x55880860c64c in main browser/app/nsBrowserApp.cpp:291
    #19 0x7f3f5eddfb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

DEDUP_TOKEN: __interceptor_malloc
SUMMARY: AddressSanitizer: heap-use-after-free clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:1694:37 in _M_empty
Shadow bytes around the buggy address:
  0x0c08806300e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c08806300f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880630100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880630110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880630120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0880630130: fa fa fa fa fa fa fa fa fa fa fd fd fd[fd]fd fa
  0x0c0880630140: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c0880630150: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c0880630160: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x0c0880630170: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c0880630180: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd 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
  Shadow gap:              cc

Command: /home/worker/firefox/firefox ./corpora/ -handle_segv=0 -handle_bus=0 -handle_abrt=0 -handle_ill=0 -handle_fpe=0 -print_pcs=1

==51==ABORTING

Christoph Diehl [:posidron]

Reporter

Comment 1

•

5 years ago

Attached file Testcase — Details

Alex Gaynor [:Alex_Gaynor]

Updated

•

5 years ago

Group: core-security → network-core-security

Alex Gaynor [:Alex_Gaynor]

Comment 2

•

5 years ago

Ok I think I understand what's going on here (maybe):

func is created. It std::moves callback, which nulls out it's reference. It no longer has a strong reference to aCallback: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#2608-2610
func is moved into NS_ShouldSecureUpgrade, now func on the stack is nulled out.
Somewhere inside of NS_ShouldSecureUpgrade we drop that too func.

At this point callback (in EnsureHSTSDataReadyNative) has no more live references, it's reference count goes to 0, and we free the contents, aCallback.

And then we try to use aCallback: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#2615

UAF.

Here's a roughly minimized version of this that might make it easier to follow:

#include <functional>


bool f(std::function<void(void)>&& x) {
    auto cb = std::move(x);
    return false;
}

int main() {
    int* x = new int(4);
    std::shared_ptr<int> s(x);
    auto func = [s{std::move(s)}]() {
        printf("%d\n", *s);
    };
    bool success = f(std::move(func));
    if (!success) {
        printf("%d\n", *x);
    }
}

My suggest solution would be not use std::move with callback -- make a copy (and increment the reference count) and then use callack at line 2615, not aCallback.

:dkeeler, not 100% sure who owns this part of the code, but starting with you :-)

Flags: needinfo?(dkeeler)

Alex Gaynor [:Alex_Gaynor]

Updated

•

5 years ago

Blocks: libfuzzer-ipc

Keywords: csectype-uaf

Dana Keeler (she/her) (use needinfo) [:keeler]

Comment 3

•

5 years ago

-> Kershaw

Flags: needinfo?(dkeeler) → needinfo?(kershaw)

Kershaw Chang [:kershaw]

Assignee

Updated

•

5 years ago

Assignee: nobody → kershaw

Flags: needinfo?(kershaw)

Priority: -- → P1

Whiteboard: [necko-triaged]

Kershaw Chang [:kershaw]

Assignee

Comment 4

•

5 years ago

Attached file Bug 1544526 - Copy the callback instead of moving — Details

Alex Gaynor [:Alex_Gaynor]

Updated

•

5 years ago

Keywords: sec-high

Kershaw Chang [:kershaw]

Assignee

Comment 5

•

5 years ago

Comment on attachment 9058951 [details]
Bug 1544526 - Copy the callback instead of moving

Security Approval Request

How easily could an exploit be constructed based on the patch?: I would say it's not easy, because this requires NS_ShouldSecureUpgrade to return an error.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
Which older supported branches are affected by this flaw?: none
If not all supported branches, which bug introduced the flaw?: Bug 1521729
Do you have backports for the affected branches?: No
If not, how different, hard to create, and risky will they be?: I would say it's not risky, since you need to somehow make NS_ShouldSecureUpgrade failed.
How likely is this patch to cause regressions; how much testing does it need?: This patch will not cause any regression, since the code in this patch is only used for test files.

Attachment #9058951 - Flags: sec-approval?

Al Billings [:abillings - ex-MoCo]

Comment 6

•

5 years ago

(In reply to Kershaw Chang [:kershaw] from comment #5)

Which older supported branches are affected by this flaw?: none

Based on this comment and Bug 1521729, this doesn't need sec-approval as it is trunk only. You can check in...

status-firefox67: --- → unaffected

status-firefox68: --- → affected

status-firefox-esr60: --- → unaffected

tracking-firefox68: --- → +

Al Billings [:abillings - ex-MoCo]

Updated

•

5 years ago

Attachment #9058951 - Flags: sec-approval?

Kershaw Chang [:kershaw]

Assignee

Updated

•

5 years ago

Keywords: checkin-needed

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

Comment 7

•

5 years ago

https://hg.mozilla.org/integration/autoland/rev/6d240e54b7ce9e47eb6f65785c8506f9dec39c4a

Keywords: checkin-needed

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

Comment 8

•

5 years ago

https://hg.mozilla.org/mozilla-central/rev/6d240e54b7ce

Group: network-core-security → core-security-release

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox68: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla68

Bogdan Maris, Desktop QA

Updated

•

5 years ago

Flags: qe-verify+

Whiteboard: [necko-triaged] → [necko-triaged][post-critsmash-triage]

Ciprian Georgiu, Desktop QA

Comment 9

•

5 years ago

•

Edited

After chatting with :kershaw on Slack about reproducing this crash using the attached test case, the conclusion was that the bug doesn't imply manual verification, since the code cannot be accessed by any .html page or .js code.

In this case, I'll remove the qe+ flag, based on the above.

Flags: qe-verify+

Daniel Veditz [:dveditz]

Updated

•

4 years ago

Group: core-security-release

Tom Ritter [:tjr] (OOTO until mid-August)

Updated

•

3 years ago

Keywords: csectype-sandbox-escape

Marco Castelluccio [:marco]

Updated

•

3 years ago

Has Regression Range: --- → yes

Keywords: regression

Regressed by: 1521729

Testcase 5 years ago Christoph Diehl [:posidron] 32 bytes, application/octet-stream		Details
Bug 1544526 - Copy the callback instead of moving 5 years ago Kershaw Chang [:kershaw] 47 bytes, text/x-phabricator-request		Details \| Review