Closed Bug 1827439 Opened 1 year ago Closed 1 year ago

Crash [@ nsFrameLoader::GetOwnerDoc]

Categories

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

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
114 Branch
Tracking Status
firefox114 --- verified

People

(Reporter: jkratzer, Assigned: kmag)

References

(Blocks 1 open bug)

Details

(Keywords: testcase, Whiteboard: [bugmon:bisected,confirmed])

Crash Data

Attachments

(2 files)

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
Attached file Testcase
See Also: → 1770998

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)

Whiteboard: [bugmon:confirm] → [bugmon:bisected,confirmed]

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.

Severity: -- → S2

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

Flags: needinfo?(kmaglione+bmo)

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.

Severity: S2 → S3
Priority: -- → P2
Assignee: nobody → kmaglione+bmo
Flags: needinfo?(kmaglione+bmo)

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.

Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4b75bb35fd06
Null check mFrameLoader in ChangeFrameLoaderCommon. r=edgar
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch

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.

Status: RESOLVED → VERIFIED
Keywords: bugmon
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: