2003449 - Crash in [@ mozilla::dom::VerifyStoragePrincipalMatchesDocumentPrincipal ]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

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

32 crashes from 31 installs of Firefox 147.0a1 on Windows.

Crash report: https://crash-stats.mozilla.org/report/index/e27aa933-88ad-42e8-a564-357830251202

MOZ_CRASH Reason:

MOZ_RELEASE_ASSERT(((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1)))) (Must succeed in setting storage principal)

Top 10 frames:

0  xul.dll  mozilla::dom::WindowGlobalParent::CreateDisconnected(mozilla::dom::WindowGlob...  dom/ipc/WindowGlobalParent.cpp:142
1  xul.dll  mozilla::dom::BrowserParent::RecvNewWindowGlobal(mozilla::ipc::ManagedEndpoin...  dom/ipc/BrowserParent.cpp:1418
1  xul.dll  mozilla::dom::PBrowserParent::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PBrowserParent.cpp:7444
2  xul.dll  mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PContentParent.cpp:6702
3  xul.dll  mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecyc...  ipc/glue/MessageChannel.cpp:1793
3  xul.dll  mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecyclePro...  ipc/glue/MessageChannel.cpp:1719
3  xul.dll  mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, ...  ipc/glue/MessageChannel.cpp:1508
3  xul.dll  mozilla::ipc::MessageChannel::MessageTask::Run()  ipc/glue/MessageChannel.cpp:1610
4  xul.dll  mozilla::RunnableTask::Run()  xpcom/threads/TaskController.cpp:705
4  xul.dll  mozilla::TaskController::RunTask(mozilla::Task*)  xpcom/threads/TaskController.cpp:196

Comment 1

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Windows-only, mostly Windows 10, the reports say no extensions (not even built-in ones) installed, which looks to me like crash reporting isn't working fully.

Comment 2

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Add-ons do show up under Telemetry environment.

Comment 3

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

There seem to be fewer distinct submitters than it first seems, if graphics info duplication is any indication.

All submitters with distict graphics info have [e-mail]. All but one have [e-mail]. In general, there seem to be many add-ons.

Comment 4

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

32 crashes from 31 installs

How trustworthy is "31 installs"? I have a very hard time believing it from how duplicative the specifics are,.

Comment 5

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Per gsvelto's answer on Matrix, this looks like a very small number of users experiencing the crash with multiple Nightly versions from different days.

Comment 6

[Dianna Smith [:diannaS]]

This looks like it started in build 20251124214625 so possibly caused by bug 543435

Comment 7

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

The plan for 147 is to back bug 543435 out of beta in bug 2003720.

Comment 8

[Vincent Hilla [:vhilla]]

Clearing my ni? for now, we're working on prioritizing and addressing the various regressions.

Comment 9

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

I'm pretty confident that there are 9 distinct configurations (likely just 9 distinct computers) out there experiencing this crash.

All submitters with distict graphics info have [e-mail]. All but one have [e-mail]. In general, there seem to be many add-ons.

Still holds.

I think we should not prioritize this one, but if we want to look into this, [e-mail] is probably the best place to start, followed by [e-mail].

Comment 10

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Perhaps farre has insight into why this might fail.

Comment 11

[Ryan VanderMeulen (PTO, back July 27)]

Bug 543435 was reverted from Fx147. Updating the flags accordingly.

Comment 12

[Andreas Farre [:farre]|PTO -> Aug 7th]

This seems to be a storage principal mismatch, and I don't know that much of the internals of that. Hsin-Yi, do you know someone that we could ask. My first bet would be :asuth, but maybe you know someone else?

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Andreas Farre [:farre] from comment #12)

This seems to be a storage principal mismatch, and I don't know that much of the internals of that. Hsin-Yi, do you know someone that we could ask. My first bet would be :asuth, but maybe you know someone else?

The confusing thing here is that the most likely storage principal mismatch would involve the partition key, but we use EqualsIgnoringPartitionKey here in mozilla::dom::WindowGlobalParent::SetDocumentStoragePrincipal, which means we presumably are failing the earlier check of whether the result of calling GetOriginNoSuffix on the document principal and the storage principal matches, which is a much more significant and unexpected mismatch.

Comment 14

[Vincent Hilla [:vhilla]]

Attachment: Bug 2003449 - Assert storage and document principal match when creating window global. r=#dom-core

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4a687553db2f
https://hg.mozilla.org/integration/autoland/rev/9bda2d888229
Assert storage and document principal match when creating window global. r=dom-core,smaug

Comment 16

[amarc]

https://hg.mozilla.org/mozilla-central/rev/9bda2d888229

Comment 17

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

There's now one release-channel (145) crash listed with matching crash location, but the stack is very different (yet about:blank-related):
https://crash-stats.mozilla.org/report/index/83eeeb5d-4891-40e1-814e-5f4860251212#tab-details

Comment 18

[Vincent Hilla [:vhilla]]

There are a few crashes with the new signature @ mozilla::dom::VerifyStoragePrincipalMatchesDocumentPrincipal

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #13)

we presumably are failing the earlier check of whether the result of calling GetOriginNoSuffix on the document principal and the storage principal matches, which is a much more significant and unexpected mismatch.

This is now verified.

The crashes originate from session history, often close to startup. This is possibly related to bug 2004165 and this review comment.

Perhaps we can construct a test case or add some assertions around session history.

Comment 19

[Vincent Hilla [:vhilla]]

Looking at WindowGlobalActor::WindowInitializer,

• It's quite clear that init.principal() is the node principal aLoadState->PrincipalToInherit()
• init.storagePrincipal() is less straight forward, but it should be mDoc->EffectiveStoragePrincipal() and if it's not the node principal, it should be aLoadState->PartitionedPrincipalToInherit().

It's surprising to me if these don't match, presumably they would already be mismatching in session history. Bug 543435 didn't change load state or session history, but maybe it changed what was stored in session history. Assertions in session history might help pinpoint the issue.

It's not obvious to me how we could fail gracefully. We could skip the synchronous load and hope the channel can deal with this. Or we use PrincipalToInherit for both if they don't match, but I don't know enough to say if this is safe.

Comment 20

[Vincent Hilla [:vhilla]]

Try push with some more assertions: https://treeherder.mozilla.org/jobs?repo=try&revision=0d62c97e2d2d6ce436d445fc021f35889ef419e3

Comment 21

[Vincent Hilla [:vhilla]]

It frequently fails to assert that if !aLoadState->PartitionedPrincipalToInherit() then doc->GetPrincipal()->Equals(doc->PartitionedPrincipal()). Perhaps this inheritance for nsFrameLoader is too general? (no, see c26) If I compare the partitioned principal of an iframe before and after bug 543435, they have the same origin (incl suffix).

PartitionedPrincipalToInherit is determined later by the channel. So we might not be able to assert anything meaningful at sync load completion.

Comment 22

[Vincent Hilla [:vhilla]]

Pernosco: https://pernos.co/debug/3NOzJokgc4oIaBoJWVCreA/index.html

I added some asserts to SessionHistory.cpp, but unfortunately mismatching principals were possible before bug 543435 and maybe not uncommon. E.g. already on opening example.com, I hit one of my asserts in the SessionHistoryInfo constructor.

• DocumentLoadListener::Open sets mLoadingSessionHistoryInfo with example.com as URI. Presumably, this is where a partitioned principal with example.com is set.
• On redirect to https, the channel sets a null principal to inherit here

Comment 23

[Vincent Hilla [:vhilla]]

And another mismatch, also before bug 543435, can occur in ParamTraits<mozilla::dom::SessionHistoryInfo>::Read here with the following test. Principal to inherit has example.com, the partitioned one has example.org:

add_task(async function test_nested_iframe_simple() {
  // Edit: url should be https://example.com
  await BrowserTestUtils.withNewTab({ gBrowser, url }, async browser => {
    const info = await SpecialPowers.spawn(browser, [], async () => {
      const iframe = content.document.createElement("iframe");
      iframe.src = "https://example.org/";
      content.document.body.append(iframe);
      await new Promise(res => iframe.addEventListener("load", res, { once: true }));

      SpecialPowers.spawn(iframe, [], async () => {
        const iframe = content.document.createElement("iframe");
        content.document.body.append(iframe);
        await new Promise(res => iframe.addEventListener("load", res, { once: true }));
      });
    })
  });
});

Edit: Pernosco https://pernos.co/debug/j9CK1u50H8Rf6sb0KffEZA/index.html

It appears that in this case, content kicks off the load for example.org and sets nsDocShellLoadState::mPrincipalToInherit to the one from example.com. The parent process will set mPartitionedPrincipalToInherit in DocumentLoadListener::Open based on the loaded URI. So we end up with mismatching principals in the load state and successively also in session history.

Comment 24

[Vincent Hilla [:vhilla]]

Having mismatching principals to inherit seems fragile, but I guess they are more suggestions than mandatory values. Looking at mPartitionedPrincipalToInheritForAboutBlank, it's only used for frames. So we never inherit one with window.open, I'm unsure if that is right. Perhaps we should add some methods to StoragePrincipalHelper and determine the partitioned principal directly in nsDocShell::CompleteInitialAboutBlankLoad?

Comment 25

[Vincent Hilla [:vhilla]]

:tihuang, can you take a look at this crash and at how bug 543435 determines the partitioned principal for initial about:blank? Specifically, this code might be relevant

• We always inherit the partitioned principal into iframes: https://searchfox.org/firefox-main/rev/14c08f0368ead8bfdddec62f43e0bb5c8fd61289/dom/base/nsFrameLoader.cpp#2277,2296
• If something goes wrong, we create about:blank with aLoadState->PartitionedPrincipalToInherit() as principal: https://searchfox.org/firefox-main/rev/14c08f0368ead8bfdddec62f43e0bb5c8fd61289/docshell/base/nsDocShell.cpp#11260-11261,11267
  • This occurs e.g. with sandboxing or data urls.

Comment 26

[Tim Huang[:timhuang]]

Inheriting the partitioned principal for the initial about:blank iframe is correct. Similar to inheriting node principal, the partitioned principal should inherit from the document that creates the initial about:blank iframe.

However, I am uncertain about the second case. Do we know where the aLoadState->PartitionedPrincipalToInherit() came from?

Comment 27

[Vincent Hilla [:vhilla]]

(In reply to Tim Huang[:timhuang] from comment #26)

However, I am uncertain about the second case. Do we know where the aLoadState->PartitionedPrincipalToInherit() came from?

I think it's probably wrong that we use aLoadState->PartitionedPrincipalToInherit(), but I don't understand it's purpose. It's set for session history loads or in DocumentLoadListener::Open. Unlike the name suggests, it's probably not always the actual partitioned principal we want to inherit. Maybe some of the uses that existed before bug 543435 are incorrect too?

I've been trying to understand where the partitioned principal comes from during normal loads. It seems to be this securityManager->GetChannelResultPrincipals call in Document::Reset. And this method will create the partitioned principal based on the non-partitioned principal and it's origin attributes? So aLoadState->PartitionedPrincipalToInherit() would not be used. Maybe we should do something similar in CompleteInitialAboutBlankLoad?

Here's a try run where I use StoragePrincipalHelper::Create in the principal mismatch case. That passes tests and would fix this crash, but this edge case probably doesn't have good coverage: https://treeherder.mozilla.org/jobs?repo=try&revision=3635f862cafd1bcab0fa12878131fc0c1306c8be
Another run to more generally ensure the about:blank partitioned principal corresponds to the result of StoragePrincipalHelper::Create: https://treeherder.mozilla.org/jobs?repo=try&revision=0a00bf73a96cbe87515d074bec375fa001437f35

Comment 28

[Vincent Hilla [:vhilla]]

(In reply to Tim Huang[:timhuang] from comment #26)

Inheriting the partitioned principal for the initial about:blank iframe is correct.

I'm afraid it's not. If I open a page <html><iframe></iframe></html> as file:// URI, the top level doesn't have a partition key. But prior to bug 543435, the iframe would have one. So the iframe doesn't just inherit the partitioned principal from the embedder.

Perhaps an issue can be reproduced already before bug 543435 here. I wonder what the iframe initially has as partitioned principal, before the async about:blank load completes.

Comment 29

[Vincent Hilla [:vhilla]]

(In reply to Vincent Hilla [:vhilla] from comment #28)

Perhaps an issue can be reproduced already before bug 543435 here. I wonder what the iframe initially has as partitioned principal, before the async about:blank load completes.

Yes: https://pernos.co/debug/bdOfxvW_-EJAuqSMVJ6-Zw/index.html

• nsDocShell::EnsureDocumentViewer creates an initial viewer for the about:blank iframe. And partitionedPrincipal has no partitionKey.
• For the async about:blank, Document::Reset gets a partitioned principal that does have a partitionKey.

Perhaps we should track this as a separate issue. We can fix the use of aLoadState->PartitionedPrincipalToInherit() anyway.

Comment 30

[Vincent Hilla [:vhilla]]

It's racy what partitioned principal window.open("about:blank") gets, this seems to relate to bug 1618557. If I open a popup right after load from example.com, the about:blank has no LoadInfo::mTopLevelPrincipal and partitionKey="". If I wait a bit, mToplevelPrincipal is not null and partitionKey="(https,example.com)".
Since bug 543435, top level about:blank doesn't seem to respect BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN for the network.cookie.cookieBehavior pref.

Comment 31

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Vincent Hilla [:vhilla] from comment #30)

Since bug 543435, top level about:blank doesn't seem to respect BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN for the network.cookie.cookieBehavior pref.

I assume this top-level about:blank is a case that is an auxiliary browsing context AKA has an opener browsing context?

Comment 32

[Vincent Hilla [:vhilla]]

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #31)

(In reply to Vincent Hilla [:vhilla] from comment #30)

Since bug 543435, top level about:blank doesn't seem to respect BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN for the network.cookie.cookieBehavior pref.

I assume this top-level about:blank is a case that is an auxiliary browsing context AKA has an opener browsing context?

Right, e.g. window.open() from example.com.

Comment 33

[Vincent Hilla [:vhilla]]

I still find this crash confusing. From the stack, we see that we're doing a history load for an iframe. It's the initial about:blank load, so we are presumably restoring the top document from SH. The principal of the eager about:blank and the history load are mismatching, so we clobber the document. Due to bug 2004165, the mismatching principals must be associated with the same process. Clobbering causes the principal and partitioned principal to have different origins without suffix. Looking at CreateAboutBlankDocumentViewer, this means the iframe is not sandboxed.

I can't come up with an example / test case that complies with these constraints. But we know that using aLoadState->PartitionedPrincipalToInherit() is incorrect, so I'll put up patches.

Comment 34

[Vincent Hilla [:vhilla]]

Attachment: Bug 2003449 - Part 1: Refactor CompleteInitialAboutBlankLoad principalMismatch. r=#dom-core

Comment 35

[Vincent Hilla [:vhilla]]

Attachment: Bug 2003449 - Part 2: Get expectedPartitionedPrincipal from channel. r=timhuang,#dom-core

Comment 36

[Vincent Hilla [:vhilla]]

Filed bug 2010596 as follow-up for the partitioned principal mismatches.

Comment 37

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Vincent Hilla [:vhilla] from comment #32)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #31)

(In reply to Vincent Hilla [:vhilla] from comment #30)

Since bug 543435, top level about:blank doesn't seem to respect BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN for the network.cookie.cookieBehavior pref.

I assume this top-level about:blank is a case that is an auxiliary browsing context AKA has an opener browsing context?

Right, e.g. window.open() from example.com.

So a real structural problem here is the third-party logic doing this in mozilla::AntiTrackingUtils::ComputeIsThirdPartyToTopWindow:

// When a top-level load is opened by window.open, BrowsingContext from
// LoadInfo is its opener, which may make the third-party caculation code
// below returns an incorrect result. So we use TYPE_DOCUMENT to
// ensure a top-level load is not considered 3rd-party.
auto policyType = loadInfo->GetExternalContentPolicyType();
if (policyType == ExtContentPolicy::TYPE_DOCUMENT) {
  loadInfo->SetIsThirdPartyContextToTopWindow(false);
  return;
}

If we recompute this state as we do in mozilla::net::nsHttpChannel::AsyncOpen, we end up with this situation where inheriting the controller into window per spec will be storage-partitioned but all of the anti-tracking APIs will be telling us that the storage should not be partitioned. While a bunch of code has side-stepped this issue through liberal use of mozilla::OriginAttributes::EqualsIgnoringPartitionKey it's a real structural problem for storage APIs since the partition key informs the storage key and creates two completely distinct families of agent clusters.

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/99b0f101a767
https://hg.mozilla.org/integration/autoland/rev/4dc8bf6510f5
Part 1: Refactor CompleteInitialAboutBlankLoad principalMismatch. r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/640fc639e73c
https://hg.mozilla.org/integration/autoland/rev/1f3640b2a8f3
Part 2: Get expectedPartitionedPrincipal from channel. r=timhuang

Comment 39

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4dc8bf6510f5
https://hg.mozilla.org/mozilla-central/rev/1f3640b2a8f3

Comment 40

[Donal Meehan [:dmeehan]]

:vhilla, does still need the leave-open keyword? Or is there still follow-up work planned under this?
I'm wondering if it should be resolved, and about beta uplift requests on the lastest two patches?

Comment 41

[Vincent Hilla [:vhilla]]

I forgot to clear the keyword and was waiting a bit to be sure crashes are gone. But right, we should uplift, thanks for the reminder.

Comment 42

[Vincent Hilla [:vhilla]]

Attachment: Bug 2003449 - Part 1: Refactor CompleteInitialAboutBlankLoad principalMismatch.

Original Revision: https://phabricator.services.mozilla.com/D279161

Comment 43

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Users might experience a crash in presumably rare edge cases due to code that is now known to be incorrect.
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: It's a small change and edge case. But we are unsure about the specifics of when it occurs. So while the code is generally covered by tests, none create the specific circumstances leading to the crash.
• String changes made/needed: no
• Is Android affected?: yes

Comment 44

[Vincent Hilla [:vhilla]]

Attachment: Bug 2003449 - Part 2: Get expectedPartitionedPrincipal from channel.

Original Revision: https://phabricator.services.mozilla.com/D279162

Comment 45

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/ea54eb10f505
https://hg.mozilla.org/releases/mozilla-beta/rev/9dc9566af242
https://github.com/mozilla-firefox/firefox/commit/a27245ecd38b
https://hg.mozilla.org/releases/mozilla-beta/rev/bb27ccdfb1f8

Comment 47

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

Copying crash signatures from duplicate bugs.