1827439 - Crash [@ nsFrameLoader::GetOwnerDoc]

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev 948cf466f3f2 (built with: --enable-address-sanitizer --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build 948cf466f3f2 --asan --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

[@ nsFrameLoader::GetOwnerDoc]

    =================================================================
    ==265933==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000098 (pc 0x7f383cf3d581 bp 0x7ffedc580ea0 sp 0x7ffedc580ea0 T0)
    ==265933==The signal is caused by a READ memory access.
    ==265933==Hint: address points to the zero page.
        #0 0x7f383cf3d581 in nsFrameLoader::GetOwnerDoc() const /dom/base/nsFrameLoader.cpp:2846:10
        #1 0x7f383cf3d3ce in nsFrameLoader::PropagateIsUnderHiddenEmbedderElement(bool) /dom/base/nsFrameLoader.cpp:2480:28
        #2 0x7f383cf4a997 in nsFrameLoaderOwner::ChangeFrameLoaderCommon(mozilla::dom::Element*, bool) /dom/base/nsFrameLoaderOwner.cpp:225:17
        #3 0x7f383cf4a0f6 in nsFrameLoaderOwner::ChangeRemotenessCommon(nsFrameLoaderOwner::ChangeRemotenessContextType const&, mozilla::dom::NavigationIsolationOptions const&, bool, bool, mozilla::dom::BrowsingContextGroup*, std::function<void ()>&, mozilla::ErrorResult&) /dom/base/nsFrameLoaderOwner.cpp:198:3
        #4 0x7f383cf4b7ef in nsFrameLoaderOwner::ChangeRemotenessWithBridge(mozilla::dom::BrowserBridgeChild*, mozilla::ErrorResult&) /dom/base/nsFrameLoaderOwner.cpp:295:3
        #5 0x7f3841895e7e in mozilla::dom::WindowGlobalChild::RecvMakeFrameRemote(mozilla::dom::MaybeDiscarded<mozilla::dom::BrowsingContext> const&, mozilla::ipc::ManagedEndpoint<mozilla::dom::PBrowserBridgeChild>&&, mozilla::dom::IdType<mozilla::dom::BrowserParent> const&, mozilla::layers::LayersId const&, std::function<void (bool const&)>&&) /dom/ipc/WindowGlobalChild.cpp:423:8
        #6 0x7f3841c28f5b in mozilla::dom::PWindowGlobalChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PWindowGlobalChild.cpp:1185:85
        #7 0x7f3841a167bf in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentChild.cpp:8767:32
        #8 0x7f383b0bf669 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /ipc/glue/MessageChannel.cpp:1800:25
        #9 0x7f383b0bc67d in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /ipc/glue/MessageChannel.cpp:1725:9
        #10 0x7f383b0bd24e in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /ipc/glue/MessageChannel.cpp:1525:3
        #11 0x7f383b0be47e in mozilla::ipc::MessageChannel::MessageTask::Run() /ipc/glue/MessageChannel.cpp:1623:14
        #12 0x7f38398ac6b9 in mozilla::RunnableTask::Run() /xpcom/threads/TaskController.cpp:553:16
        #13 0x7f38398a2a4c in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:869:26
        #14 0x7f383989fd18 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:700:15
        #15 0x7f38398a0431 in mozilla::TaskController::ProcessPendingMTTask(bool) /xpcom/threads/TaskController.cpp:464:36
        #16 0x7f38398b23d4 in operator() /xpcom/threads/TaskController.cpp:194:37
        #17 0x7f38398b23d4 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_4>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:548:5
        #18 0x7f38398d5e8e in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1239:16
        #19 0x7f38398e0474 in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:479:10
        #20 0x7f383b0c7263 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:107:5
        #21 0x7f383af45117 in RunInternal /ipc/chromium/src/base/message_loop.cc:369:10
        #22 0x7f383af45117 in RunHandler /ipc/chromium/src/base/message_loop.cc:362:3
        #23 0x7f383af45117 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:344:3
        #24 0x7f384265dbe9 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:148:27
        #25 0x7f3847636388 in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:738:20
        #26 0x7f383af45117 in RunInternal /ipc/chromium/src/base/message_loop.cc:369:10
        #27 0x7f383af45117 in RunHandler /ipc/chromium/src/base/message_loop.cc:362:3
        #28 0x7f383af45117 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:344:3
        #29 0x7f3847635b1f in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:673:34
        #30 0x557122ec0684 in content_process_main(mozilla::Bootstrap*, int, char**) /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #31 0x557122ec0b47 in main /browser/app/nsBrowserApp.cpp:353:18
        #32 0x7f385c029d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #33 0x7f385c029e3f in __libc_start_main csu/../csu/libc-start.c:392:3
        #34 0x557122dff108 in _start (/home/jkratzer/builds/m-c-20230411035719-fuzzing-asan-opt/firefox+0x113108) (BuildId: 4d9fbc4fc066ad3f5aa4a4ce2d96f6116408d564)
    
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV /dom/base/nsFrameLoader.cpp:2846:10 in nsFrameLoader::GetOwnerDoc() const
    ==265933==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20230411092751-25045b498bff.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: 0bcad14b3c3a03913379b5dd5a5eadfd5f048234 (20220412035701)
End: 948cf466f3f2e0db976af227c20d222a971d8293 (20230411035719)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

Comment 3

[Kris Maglione [:kmag]]

I'm guessing that the crash is from a null this from a null mFrameLoader in nsFrameLoaderOwner::ChangeFrameLoaderCommon.

I don't think this should be exploitable, and it doesn't seem likely to happen much in the wild. But causing a parent process crash from a timing issue in a content process isn't great. We probably just need to add a null check and bail out early. We have similar null checks for null owner document and null owner content lower down in the call stack.

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Kris Maglione [:kmag] from comment #3)

I'm guessing that the crash is from a null this from a null mFrameLoader in nsFrameLoaderOwner::ChangeFrameLoaderCommon.

I don't think this should be exploitable, and it doesn't seem likely to happen much in the wild. But causing a parent process crash from a timing issue in a content process isn't great. We probably just need to add a null check and bail out early. We have similar null checks for null owner document and null owner content lower down in the call stack.

Hi Kris, looks there's a way moving forward in this bug. Can you take this please?

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

We talked this a bit in the team meeting that S3 is a more proper setting as bug 1770998. However, we still want to give it a priority to fix, given that there seems to be a way to reproduce it and move forward.

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1827439: Null check mFrameLoader in ChangeFrameLoaderCommon. r=edgar

When we're called from IPC, we can race the child for frame loader
destruction, which means we can wind up with a null mFrameLoader here.
Adding a null check is consistent with the other similar null checks higher
and lower in the call chain.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b75bb35fd06
Null check mFrameLoader in ChangeFrameLoaderCommon. r=edgar

Comment 8

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4b75bb35fd06

Comment 9

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20230425093238-4e9d0ab74fc3.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.