1687104 - Crash in [@ IPCError-browser | RecvStoreUserInteractionAsPermission receiving unexpected principal]

Status: RESOLVED DUPLICATE of bug 1687890

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

Description

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

8 crashes on 5 installations so far.

Crash report: https://crash-stats.mozilla.org/report/index/0803324c-12be-4b35-b222-d50510210116

Reason: EXCEPTION_BREAKPOINT

Top 10 frames of crashing thread:

0 xul.dll mozilla::ScrollFrameHelper::DecideScrollableLayer layout/generic/nsGfxScrollFrame.cpp:4281
1  @0x26e800008213 
2 xul.dll trunc 
3 xul.dll trunc 
4 xul.dll trunc 
5 xul.dll trunc 
6 xul.dll trunc 
7 xul.dll trunc 
8 xul.dll nsDisplayListBuilder::AdjustWindowDraggingRegion layout/painting/nsDisplayList.cpp:1790
9 xul.dll nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4304

Comment 1

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

Nika, it seems we are facing a crash within ContentParent::RecvStoreUserInteractionAsPermission which currently simply validates the received Content-Principal.

Here are the options:

• Maybe we receive a NullPtr (not NullPrincipal, nullptr)?
  Unlikely to me, because the only callsite is ContentBlockingUserInteraction which performs a nullptr check.
• Maybe we receive a SystemPrincipal?
  This is the more likley option to me.
• Or the origin of the stored and the received principal do not match.

It's hard to decipher from the crash-stat what really happen.

What's your take?

Comment 2

[Pascal Chevrel (relman team) -> :pascalc]

Tracking for 86, not all crashes have Fission enabled so some of these crashes may happen in beta after the merge next week. One of the reporters comments mentions reproducing the issue consistently by looking at view-source: of the page.

Comment 3

[Anne (:annevk)]

I found some steps to reproduce this (I have Fission enabled):

1. Open view-source:https://html.spec.whatwg.org/404.html
2. Click.
3. Crash? Don't crash please.

Comment 4

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

Attachment: Bug 1687104: Update siteOrigin, siteOriginNoSuffix to handle view-source correctly.

Comment 5

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

In fact, this is rather a problem that Principal::GetSiteOrigin and Principal::GetSiteOriginNoSuffix introduced in Bug 1678310 can not handle view-source correctly. I just uploaded a testcase (comment 4) which illustrates the problem.

In more detail view-source:https://test1.test2.example.com^userContextId=101 translates to https://test1.test2.example.com instead of https://example.com.

Not entirely where the problem is right now, but probably somewhere here within ContentPrincipal::GetSiteOriginNoSuffix.

Comment 6

[Mike Hoye [:mhoye]]

I can reproduce this at will by playing with: https://potch.github.io/demos/boomerpunk/

STR's are: After uploading an image and playing with that for a while - it's fun! - I can right-click the displayed image and select "view-image", and then right-click anywhere but the image in the tab content, and immediately crash the tab with the error mentioned above.

The URL displayed is "blob:null/[UUID]" when the crash occurs. I've reported:

bp-29905318-284a-48f4-9d18-bcf6d0210118
bp-b31c8119-f2ed-424c-a49e-030e40210118

I'm on Win10 on Intel current, Nightly current.

Comment 7

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #5)

In fact, this is rather a problem that Principal::GetSiteOrigin and Principal::GetSiteOriginNoSuffix introduced in Bug 1678310 can not handle view-source correctly. I just uploaded a testcase (comment 4) which illustrates the problem.

In more detail view-source:https://test1.test2.example.com^userContextId=101 translates to https://test1.test2.example.com instead of https://example.com.

Not entirely where the problem is right now, but probably somewhere here within ContentPrincipal::GetSiteOriginNoSuffix.

No more need for the ni? - everything is explained in the patch: https://phabricator.services.mozilla.com/D102200

Comment 8

[Pascal Chevrel (relman team) -> :pascalc]

Christoph, will that patch be reviewed and land before the merge to beta to avoid a potential uplift?

Comment 10

[Andrew McCreight [:mccr8]]

85% of these crashes are happening with Fission enabled, so we want to make sure it gets fixed for Fission purposes even if it somehow doesn't end up getting fixed with non-Fission. There was some speculation that this regression might be contributing to increase instability in Fission relative to non-Fission.

Comment 11

[u673061]

Yes, I have the Fission experiment enabled and I tried to load resource at https://bin.kv2.dev (which is a famous pastebin service) . At that time, my browser tab has ~6 opened and the pastebin tab suddenly crashes, and then, the browser does not respond and it is consuming huge amounts of memory and my CPU resource. I had to force close the Firefox Nightly browser and start again.

Comment 12

[u673061]

Mr. Kerschbaumer, So I reported the bug crashes for #1688018, but only discovered that it is the duplicate of this bug.
Crash report: https://crash-stats.mozilla.org/report/index/9d0072d4-dd11-4914-97be-c5a970210121
I am not a professional developer, only a high school student, so I can't tell much about this.
Problems with the memory stack and CPU for the DOM security? I have no idea.

Comment 14

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

Here is the plan: we are going to relax the Principal validation within Bug 1687890 and also going to uplift Bug 1687890. This strategy gives us more time to find a profound solution for this bug.

Comment 15

[u673061]

Christoph:
Would it be possible to the estimated date and time when this bug can be fixed?
Please do not hesitate to contact me.
Thank you very much for your hard work.
It would be really appreciated that this bug can be fixed soon.
Again, thanks!

By David (4n0nymou2)

Comment 16

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

(In reply to David Hu from comment #15)

Would it be possible to the estimated date and time when this bug can be fixed?

Working on a fix for this bug. Please note that we relaxed the validation within Bug 1687890, so it shouldn't be a problem at the moment.

Comment 17

[Chris Peterson [:cpeterson]]

(In reply to Andrew McCreight [:mccr8] from comment #10)

85% of these crashes are happening with Fission enabled, so we want to make sure it gets fixed for Fission purposes even if it somehow doesn't end up getting fixed with non-Fission. There was some speculation that this regression might be contributing to increase instability in Fission relative to non-Fission.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #16)

(In reply to David Hu from comment #15)

Would it be possible to the estimated date and time when this bug can be fixed?

Working on a fix for this bug. Please note that we relaxed the validation within Bug 1687890, so it shouldn't be a problem at the moment.

Tracking for Fission M6c since this is a new crash spike that seems to affect Fission users more than non-Fission users. However, it sounds like this crash has been addressed by bug 1687890.

Comment 18

[Chris Peterson [:cpeterson]]

Christoph, since this bug is specifically about the RecvStoreUserInteractionAsPermission crash, can we resolve this bug as fixed by bug 1687890 ("Relax Principal Validation") and move your new view-source patches to a separate bug? This bug is flagged by Release Management as a blocker for Fx86. Since this bug is still open, even though the crash has been fixed by bug 1687890, this bug will stay on Release Management's radar unnecessarily.

If your view-source patches will land very soon and you prefer to keep working on them in this bug, I can change this bug to no longer block Fission or Fx86.

Comment 19

[Christoph Kerschbaumer [:ckerschb, back on Aug 10th}]

Works for me, here we go:

• Marking this bug as a duplicate of bug 1687890 because the actual crash was resolved with Bug 1687890.
• Let's take the view-source handling of siteOrigin over to Bug 1688815.

Comment 20

[Phabricator Automation]

Comment on attachment 9197759 [details]
Bug 1687104: Update siteOrigin, siteOriginNoSuffix to handle view-source correctly.

Revision D102200 was moved to bug 1688815. Setting attachment 9197759 [details] to obsolete.