New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 742049 link

Starred by 3 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Update the browser's notion of a frame's origin when for window.open('about:blank')+document.write

Project Member Reported by raymes@chromium.org, Jul 13 2017

Issue description

Currently if you do window.open('about:blank') and document.write into it, the origin of the about:blank page changes inside blink to match the origin of the opener. However currently this isn't reflected in the RenderFrameHost's notion of the origin. This has the potential to cause bugs, e.g.  issue 740540 .

We should update the browser's notion of the origin to match blink in this case.
 

Comment 1 by raymes@chromium.org, Jul 13 2017

creis/nasko is this something that would be hard to do? We've hit up against it a few times now.

Comment 2 by raymes@chromium.org, Jul 13 2017

Note that as discussed previously, the origin already updated when an in-page navigation happens in this case.

Comment 3 by dcheng@chromium.org, Jul 13 2017

Cc: dcheng@chromium.org
Does the origin actually change? An about:blank iframe inherits the origin of its opener to begin with, so document.write() shouldn't actually change it.

In addition, even if it did change it, document.write() requires same-origin access, so it would just change to the same origin?

Comment 4 by raymes@chromium.org, Jul 13 2017

Ahh true, the document.write part is all just a red-herring. I guess what we want is just that the origin is reflected in the RFH after a window.open('about:blank')

Comment 5 by raymes@chromium.org, Jul 13 2017

Actually now I'm not so sure. Testing it locally, if I do window.open('about:blank') in the console, I get this in the new window's console:
> location.href
"about:blank"
> location.origin
"null"

But if I do document.write they change.

In comment #50 of https://bugs.chromium.org/p/chromium/issues/detail?id=466422#c50 creis said that the location.href doesn't change if (for example) document.write isn't used and innerHTML is set. I think that was where I originally got this idea from. Am I missing something?

Comment 6 by raymes@chromium.org, Jul 13 2017

Either way I think the main concern for us is just that the origin in the browser matches the renderer if possible :) 

Comment 7 by dcheng@chromium.org, Jul 13 2017

Are you testing window.open() from a top-level about:blank page by any chance?

Comment 8 by raymes@chromium.org, Jul 14 2017

Interesting I did some more testing:

(Window 1: https://bugs.chromium.org)
> var x = window.open('about:blank')

(Window 2: about:blank)
> document.origin
"https://bugs.chromium.org"
> document.location.origin
"null"
> document.location.href
"about:blank"

(Window 1: https://bugs.chromium.org)
> x.document.write('hello');

(Window 2: about:blank)
> document.origin
"https://bugs.chromium.org"
> document.location.href
"https://bugs.chromium.org
> document.location.origin
"https://bugs.chromium.org"

(Window 1: https://bugs.chromium.org)
> x.document.location.href = '#test'

(Window 2: about:blank)
<At this point the omnibox changes from about:blank to https://bugs.chromium.org>


---
So the initial window.open changes document.origin but not document.location :( This is really confusing :(


Comment 9 by dcheng@chromium.org, Jul 14 2017

Cc: nick@chromium.org jochen@chromium.org foolip@chromium.org mkwst@chromium.org
So window.open() doesn't change the document's origin: the initial Document in a window created with window.open() always inherits the origin*. So that's why in all the test cases, document.origin consistently = https://bugs.chromium.org.

Now to explain the oddities...

Case 1: just window.open('about:blank');
Per https://html.spec.whatwg.org/multipage/history.html#dom-location-origin, location.origin will "[r]eturn the serialization of this Location object's url's origin."

The url of the Location object is about:blank. Per https://url.spec.whatwg.org/#concept-url-origin, the origin of this url is an opaque origin, which serializes to "null" per https://html.spec.whatwg.org/multipage/origin.html#ascii-serialisation-of-an-origin.

Case 2: document.write()
Calling document.write will invoke the document open steps (since document.open() hasn't been implicitly or explicitly called yet). Per step 23 of https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps, this changes the document's URL to the "URL of the responsible document specified by the entry settings object", which is just a complicated way of saying that it sets it to the URL of the document that called document.open(). As window 1 called document.open(), window 2's document's URL changes to match it.

Case 3: Navigate to '#test'
As this navigation is a partial URL, it gets resolved to the full URL of window 1 + the hash, and navigates there. So this is an actual cross-document navigation. Thus the omnibox gets updated. Of course, since we're navigated to a same origin site, document.origin won't change either.

I'm not sure whether or not we want document.open() to affect the URL in the omnibox. There's some debate over this in https://github.com/whatwg/html/issues/1698 and whether or not we should be treating document.open() as a "navigation" (like Firefox does) or pretending it doesn't exist (like Chrome and Safari). There's also some active debate over how this should affect any document abstraction we'd want to implement on the browser side (see issue 729021)
Thanks for the detailed explanation dcheng!

The main question from my perspective is what security origin we should use for security checks. Currently in blink we use the opener document's origin. In the browser we have "about:blank" and that's also what we display in the omnibox. That means that security checks done on the blink side will succeed whereas in the browser they will tend to fail :( For now I'm thinking that we should just block all permissions on about:blank to reflect that fact that we don't propagate the origin to the browser and display it in the omnibox (we already do this for most permissions).

From my very very naive perspective it seems like if the new window has a document.location and document.origin set to a valid origin and the page is fully under the control of that origin only, there's no reason not to display that origin in the omnibox. But I'm sure I'm missing many details :) 

Would it be possible to at least make the in-page navigation not cause an omnibox change? i.e either update the omnibox on document.open or not update it when the in-page navigation happens? It feels inconsistent to do so. 
Cc: mea...@chromium.org
Hmm... if we're not propagating the correct origin for about:blank, that does seem like a bug. We should fix that.

I've CCed meacer@, who's thought about omnibox display in the past. Note that our logic for this is pretty complex: for example, when a page uses window.open(), we actually show the URL of the *pending* navigation, so the user doesn't see about:blank in a page that's likely going to navigate. (We do have some logic to revert it to about:blank if the opener touches the new window's DOM in a way that could cause spoofing)

As for the in-page navigation, confusingly enough, it's not actually an in-page navigation. It's doing an actual navigation from about:blank to <the opener's URL>#hash--so it goes through the network stack, etc. It'd be the same as if the user had typed that manually.
I looked at how a few things are plumbed through, and it *appears* they're using security origins (originally sent from the renderer). But I didn't audit anything, and it appears the origins are treated as URLs for part of the trip (for example, https://cs.chromium.org/chromium/src/chrome/browser/permissions/permission_context_base.h?rcl=1ef3492242e14636cbf7c5f2bb19097330525c66&l=79). As it appears there are sometimes multiple conversions between URLs and origins before finally going into PermissionContextBase, I wonder if there's somewhere where the code is passing in the document URL instead of the security origin by accident?

Another thing I noticed is that some of the permission request IPCs are plumbing origins through from the renderer. I think we likely shouldn't be doing that: the browser side has the committed origin already (see RenderFrameHost::GetLastCommittedOrigin), so we shouldn't be trusting the origin sent from the renderer... though if we're using Mojo, there are some subtle synchronization issues if the interface isn't associated...
Thanks dcheng!

> Another thing I noticed is that some of the permission request IPCs are plumbing origins through from the renderer.

I agree - we're tracking that in issue 698985 :)


> As it appears there are sometimes multiple conversions between URLs and origins before finally going into PermissionContextBase

We're actually using WebContents::GetLastCommittedUrl. But I would expect that to match document.href ?

> As for the in-page navigation, confusingly enough, it's not actually an in-page navigation. It's doing an actual navigation from about:blank to <the opener's URL>#hash--so it goes through the network stack, etc. It'd be the same as if the user had typed that manually.

As that true? The content of the page doesn't change at all though? All that happens is the omnibox changes (the lock icon doesn't actually get displayed either). The result at least feels very different to typing the URL manually. You can try it:
> var x = window.open('about:blank'); x.document.write('hello'); x.document.location.href = '#test';
I'm looking at bug 466422. The current fix I have in mind for that bug is to update the virtual URL when document.open happens, but that will introduce an inconsistency between last committed URL and the virtual URL. Hopefully nobody is using the virtual URL to make security decisions, but I'm not quite comfortable with this fix. Is there anything I can do to help with this bug in order to have a proper fix for 466422?

Re #15 that sounds like an interesting idea and a potential incremental step in the right direction.

To be honest though I think to make real progress here we really need a bunch of people (web platform, site isolation/navigation, security enamel) to sit down in a room and come up with what we believe to be the best/most coherent behavior, document it, broadcast it and work towards it.

The problem with doing that is that about:blank never seems high enough priority. It's just a thing that gets in the way. But maybe we need to do it and have an "about:blank summit" or something.
Cc: -foolip@chromium.org
Project Member

Comment 18 by sheriffbot@chromium.org, Nov 22

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
I believe we still want this fixed and it might happen as part of tracking precursor origin in  issue 882053 . Removing the label for now.

Sign in to add a comment