Closed Bug 1599438 Opened 2 years ago Closed 2 years ago

Sandbox flags should be snapshotted at start of navigation

Categories

(Core :: DOM: Navigation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: iclelland, Assigned: mattwoodrow)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.108 Safari/537.36

Steps to reproduce:

Create an iframe element, insert into a document to kick off a navigation. Set the iframe's sandbox flags after navigation starts, but before the response is returned and the document created.

See WPT at https://github.com/web-platform-tests/wpt/pull/20411

Actual results:

Document gets sandbox flags from the state of the iframe container when the navigation completes.

Expected results:

Document sandboxing flags should come from the state of the iframe container as the navigation starts. See recent HTML spec change at https://github.com/whatwg/html/pull/5098.

Component: Untriaged → Layout: Images, Video, and HTML Frames
Product: Firefox → Core
Component: Layout: Images, Video, and HTML Frames → DOM: Navigation

I chatted with kmag about this.

We're currently grabbing the sandbox flags from the browsing context when we construct the loaded Document.

We should instead make the sandbox flags a property of the LoadInfo (still read from the BrowsingContext), which we construct when initiating a load. We can then read the value from the LoadInfo into the Document when it is created.

I wonder if we may need to add another step of copying it to the DocShellLoadState in whatever process initiates the load, and then copying that value to the LoadInfo. In practice, I think it shouldn't make a difference, but theoretically, under Fission, a frame with a cross-process parent could initiate a load in a cross-process sibling frame at the same time as the sandbox flags are changed in the parent. But in principal, that frame should also know nothing about the sandbox flags of its cross-origin sibling of a cross-origin parent, so... the more I think about it, it shouldn't have any say in the matter, and it shouldn't make a difference from its perspective, anyway.

Type: defect → enhancement
Priority: -- → P3

We're currently grabbing the sandbox flags from the browsing context when we construct the loaded Document.

It's a lot more messed up than that. We're currently sort-of grabbing the SANDBOXED_ORIGIN flag when the loadinfo is created (see the SEC_SANDBOXED flag in the loadinfo's mSecurityFlags). We grab all the other flags when we create the document. And yes, I think the right fix is to change those places where we set that up right now to store the whole sandbox flag word in the loadinfo....

I'd certainly label this a "defect", not an "enhancement", in that we can actually end up with things that have a sandboxed principal but not the corresponding sandbox flag, or the other way around!

Assignee: nobody → matt.woodrow
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3538b7016aa
Store sandbox flags on the LoadInfo when creating a channel for a docshell, so that we don't read a stale value from the BrowsingContext later. r=bzbarsky
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d55ddfe4e0fa
Store sandbox flags on the LoadInfo when creating a channel for a docshell, so that we don't read a stale value from the BrowsingContext later. r=bzbarsky
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
Flags: needinfo?(matt.woodrow)
You need to log in before you can comment on or make changes to this bug.