1978727 - Hit MOZ_CRASH(DocumentLoadListener::OpenDocument: Unexpected load flags: 290005 vs. 290001 (differing 4 vs. 0)) at /builds/worker/checkouts/gecko/netwerk/ipc/DocumentLoadListener.cpp:1037

Status: ASSIGNED

(Core :: DOM: Networking, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing 20250525-a0c6745660c1 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Hit MOZ_CRASH(DocumentLoadListener::OpenDocument: Unexpected load flags: 290005 vs. 290001 (differing 4 vs. 0)) at /builds/worker/checkouts/gecko/netwerk/ipc/DocumentLoadListener.cpp:1037

#0 0x7a3c82063402 in MOZ_CrashSequence /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:248:3
#1 0x7a3c82063402 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:381:3
#2 0x7a3c82063402 in mozilla::net::DocumentLoadListener::OpenDocument(nsDocShellLoadState*, unsigned int, unsigned int, mozilla::Maybe<unsigned long> const&, mozilla::TimeStamp const&, nsDOMNavigationTiming*, mozilla::Maybe<mozilla::dom::ClientInfo>&&, bool, mozilla::Maybe<bool>, mozilla::dom::ContentParent*, nsresult*) /builds/worker/checkouts/gecko/netwerk/ipc/DocumentLoadListener.cpp:1061:7
#3 0x7a3c82062165 in mozilla::net::DocumentChannelParent::Init(mozilla::dom::CanonicalBrowsingContext*, mozilla::net::DocumentChannelCreationArgs const&) /builds/worker/checkouts/gecko/netwerk/ipc/DocumentChannelParent.cpp:69:40
#4 0x7a3c82080d3a in mozilla::net::NeckoParent::RecvPDocumentChannelConstructor(mozilla::net::PDocumentChannelParent*, mozilla::dom::MaybeDiscarded<mozilla::dom::BrowsingContext> const&, mozilla::net::DocumentChannelCreationArgs const&) /builds/worker/checkouts/gecko/netwerk/ipc/NeckoParent.cpp:270:11
#5 0x7a3c8210dae4 in mozilla::net::PNeckoParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PNeckoParent.cpp:1871:79
#6 0x7a3c86a7f5c8 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp:6439:32
#7 0x7a3c8229f2ce in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1795:25
#8 0x7a3c8229c850 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, std::unique_ptr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1721:9
#9 0x7a3c8229d257 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1512:3
#10 0x7a3c8229e239 in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1612:14
#11 0x7a3c816cfee7 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:703:16
#12 0x7a3c816c8fee in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:1310:20
#13 0x7a3c816c7d27 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:1133:15
#14 0x7a3c816c81a5 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:639:36
#15 0x7a3c816d6f26 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:333:37
#16 0x7a3c816d6f26 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#17 0x7a3c816e8b23 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1159:16
#18 0x7a3c816ef24f in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#19 0x7a3c822a4a67 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#20 0x7a3c822005b1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:362:3
#21 0x7a3c822005b1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:344:3
#22 0x7a3c8707af48 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#23 0x7a3c8713f354 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:471:33
#24 0x7a3c87f3cc84 in nsAppStartup::Run() /builds/worker/checkouts/gecko/toolkit/components/startup/nsAppStartup.cpp:291:30
#25 0x7a3c8805ba4e in XREMain::XRE_mainRun() /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5893:22
#26 0x7a3c8805ced7 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:6138:8
#27 0x7a3c8805d9f8 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:6211:21
#28 0x648ff8f8ef66 in do_main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:232:22
#29 0x648ff8f8ef66 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:464:16
#30 0x7a3c96a72d8f in __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#31 0x7a3c96a72e3f in __libc_start_main ./csu/../csu/libc-start.c:392:3
#32 0x648ff8f62588 in _start ??:0:0

Comment 1

[Tyson Smith [:tsmith]]

This has also been reported by live site testing.

Comment 2

[Mayank Bansal]

Bisection:
Bug 1921972 - Allow to propagate loadgroup flags to parent process. r=nika,necko-reviewers,valentin

As per discussion. Test is in the other patch, assuming this is green
I'll incorporate it, but gotta go for the day.

Differential Revision: https://phabricator.services.mozilla.com/D224824

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1921972

:emilio, since you are the author of the regressor, bug 1921972, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(Please no rush responding, I don't think this is super prioritary as it's a DIAGNOSTIC_ASSERT, doesn't crash on release)

Ok, so the differing flag is LOAD_DOCUMENT_NEEDS_COOKIE, and we're not getting it set from the child, but are getting it set from the parent.

So there's a mismatch between what DocumentLoadListener says the sandbox flags should be vs what they really are. But afaict they come from the same place (BrowsingContext::GetSandboxFlags):

• https://searchfox.org/mozilla-central/rev/1838f847aa3bf909c3d34a94a8f0cd7e37fca086/docshell/base/nsDocShell.cpp#10822,10849-10852

I think this is a pre-existing race condition that my patch exposes (because it added the assert). The test-case does:

  frame.src = "...";
  setTimeout(() => {
    frame.srcdoc = "<div></div>" // nsDocShell::DoURILoad happens here. No sandbox flags.
    frame.setAttribute("sandbox", "allow-top-navigation") // BrowsingContext sandbox flags are updated in both the child and parent processes.
  }, 750);

So by the time DocumentLoadListener::Open runs, the sandbox flags are different from the time when the load started.

Andreas, Nika: Am I missing something? Does that diagnostic make sense to you?

If so, as for the fix: we could allow to set this bit from the child. If this is only about access to document.cookie it might be fine?

diff --git a/netwerk/base/nsLoadGroup.h b/netwerk/base/nsLoadGroup.h
index 18d6066e7fa89..ca308899508c0 100644
--- a/netwerk/base/nsLoadGroup.h
+++ b/netwerk/base/nsLoadGroup.h
@@ -70,8 +70,9 @@ class nsLoadGroup : public nsILoadGroup,
    * thus nothing security-critical should be allowed here.
    */
   static constexpr nsLoadFlags kInheritedLoadFlags =
-      LOAD_BACKGROUND | LOAD_BYPASS_CACHE | LOAD_FROM_CACHE | VALIDATE_ALWAYS |
-      VALIDATE_ONCE_PER_SESSION | VALIDATE_NEVER;
+      LOAD_BACKGROUND | LOAD_DOCUMENT_NEEDS_COOKIE | LOAD_BYPASS_CACHE |
+      LOAD_FROM_CACHE | VALIDATE_ALWAYS | VALIDATE_ONCE_PER_SESSION |
+      VALIDATE_NEVER;

  protected:
   virtual ~nsLoadGroup();

Alternatively, we could not fatally crash, I think that'd reject some loads incorrectly, which is what happens on release now. Given this is hit on the wild I'm not sure it's fine...

Any other potential fix I might've overlooked? I'll submit the fix I have tentatively, for now.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1978727 - Allow setting LOAD_DOCUMENT_NEEDS_COOKIE from the child process. r=nika,farre

As the sandbox flags may have changed by the time we compute the flags
on the parent.

Comment 6

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20250722162531-73304b4f70e7.
The bug appears to have been introduced in the following build range:

Start: 04e9738d508bf5eb0acf306e2671da47bab0ae7c (20241008210917)
End: 2a8c1b7d39e3a513cfb32fe857592d0c87fd945a (20241008230122)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=04e9738d508bf5eb0acf306e2671da47bab0ae7c&tochange=2a8c1b7d39e3a513cfb32fe857592d0c87fd945a

Comment 7

[Jason Kratzer [:jkratzer]]

Bugmon was unable to record a pernosco session for this bug.

Comment 8

[Bugmon [:jkratzer for issues]]

A pernosco session for this bug can be found here.