1102224 - The one permitted sandboxed navigator should be set when a link is opened in a new tab from a sanboxed iframe by middle click/Ctrl(Shift)+click/Ctrl(Shift)+ENTER

Status: ASSIGNED

(Firefox :: Tabbed Browser, defect)

Description

[Bob Owen (:bobowen)]

This is a follow-on from questions raised by bug 1037766.
This was determining if ctrl-clicking (etc.) a link in a sandboxed iframe should propagate the sandboxing flags.

After getting clarification of the spec (see [1]), the outcome was that the flags should not be propagated and that this represented a deliberately allowable escape from the sandbox.

The original source browsing context remains the same (i.e. the browsing context that contains the link).
Because of the way that sandboxing affects the navigation algorithm, the spec was also changed so that the "one permitted sandboxed navigator" was set on the new top-level browsing context (see [2] step 3 and [3] step 8->2).
Otherwise you would open a new tab and the sandboxed source browsing context would not be able to navigate it.

In Firefox ctrl-clicking (etc.) a link acts as if the user had opened the new tab and typed the URL themselves.
This still gives you the (deliberate) escape from the sandbox, but the one permitted sandboxed navigator does not get set.

[1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=26317
[2] https://html.spec.whatwg.org/multipage/semantics.html#following-hyperlinks-2
[3] https://html.spec.whatwg.org/multipage/browsers.html#dom-open

Comment 1

[Bob Owen (:bobowen)]

I thought I'd get views from people more used to thinking through the implications of these things.
I know you've both raised and commented on iframe sandbox bugs before.

As explained in the description, ctrl-click etc. is now an explicitly allowable escape of the sandbox in the spec.
I have pushed Hixie a couple of times on that already.
(For what it's worth Chrome blocks this giving a sandboxing error message.)

Personally, I think if we are going to allow the escape, the way that Firefox currently does it is better than having to set the one permitted sandboxed navigator.
As that way means that the sandboxed browsing context can continue to navigate the new unsandboxed top-level browsing sandbox.
Whereas the current Firefox way just means the sandboxed link is escaped into a new top-level browsing context, but there is no reference back to the sandboxed browsing context.

What do you think?
Should we look at fixing this or should we push back on the spec again?
Either on the one permitted sandboxed navigator part or the whole escape.

Comment 2

[Frederik Braun [:freddy]]

Well, right-clicking a frame and clicking "open this iframe in a new tab" is quite similar or do we disallow this?

This is especially interesting when the site doesn't send a CSP sandbox directive or the user agent doesn't support this (i.e. Firefox doesn't).

Comment 3

[Bob Owen (:bobowen)]

(In reply to Frederik Braun [:freddyb] from comment #2)
> Well, right-clicking a frame and clicking "open this iframe in a new tab" is
> quite similar or do we disallow this?
> 
> This is especially interesting when the site doesn't send a CSP sandbox
> directive or the user agent doesn't support this (i.e. Firefox doesn't).

We seem to allow that as well, it doesn't look like the sandbox flags are propagated.

Chrome (which blocks ctrl-click) allows right-click open in new tab on a link.
It doesn't seem to have the "open frame in a new tab" option.

Comment 4

[Daniel Veditz [:dveditz]]

In Firefox Ctrl-click (or middle click) is just a short-cut for the "Open in New Tab" context menu and should work the same. The meaning is "I don't care what this author thinks should happen when I click this, copy the URL and open it in a fresh context". I think it's perfectly fine for Firefox to leave those equivalent for sandboxed content and Chrome to decide users might be fooled into Ctrl-clicking in dangerous ways. Those are UA decisions and don't need to be explicit in the spec.
 
Unless we see some evidence of attacks I'd go ahead and leave them equivalent.

Comment 5

[Daniel Veditz [:dveditz]]

... and "Open in New Tab" is itself just a UA shortcut for 
* Copy Link Location
* Ctrl-T (new tab)
* Ctrl-V (paste)
* Enter  (load URL)

Comment 6

[Bob Owen (:bobowen)]

(In reply to Daniel Veditz [:dveditz] from comment #4)
> In Firefox Ctrl-click (or middle click) is just a short-cut for the "Open in
> New Tab" context menu and should work the same. The meaning is "I don't care
> what this author thinks should happen when I click this, copy the URL and
> open it in a fresh context". I think it's perfectly fine for Firefox to
> leave those equivalent for sandboxed content and Chrome to decide users
> might be fooled into Ctrl-clicking in dangerous ways. Those are UA decisions
> and don't need to be explicit in the spec.
>  
> Unless we see some evidence of attacks I'd go ahead and leave them
> equivalent.

Thanks Dan.
This bug is more specifically about what happens as part of the escape, i.e. using the original browsing context as the source browsing context for navigation and setting it as the one permitted sandboxed navigator.

Given your comment in bug 1037766:
'"Open in New Tab (Window)" or the "Open frame in new tab (window)" options are UA escapes outside the context of what the author has created. As long as we are opening the content in a fresh browsing context not linked to the original source in any way we're fine.'

I take it that you think we should push back on the changes to the spec that state that the original browsing context should be set as the one permitted sandboxed navigator of the new browsing context?
I've linked to the places in the spec, where this is now stated, in the description.
For what it's worth, as I said in comment 1, I agree, but if you have any concrete examples of why you think this is bad, that would be useful to put to Hixie to try and get this changed.


freddyb - I think that you were agreeing that the escape should be allowable.
Do you also think that we shouldn't however have a link back to the original source?
Again, any issues you feel this causes would be welcome.

Comment 7

[Frederik Braun [:freddy]]

So, linking back here is about the "opener" reference, right?
I'm not entirely sure, but I'd lean towards not linking it from the new window (or tab).

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Bob, is this bug still relevant? Should the needinfo request for dveditz get renewed?

Comment 9

[Bob Owen (:bobowen)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Bob, is this bug still relevant? Should the needinfo request for dveditz get
> renewed?

I think we are happy with our behaviour, I was leaving this open as we don't quite do what the spec suggests (if I remember correctly).

Things have shifted a bit in the spec world, so maybe we could get the spec changed to fit our implementation as well.

Anne, might have some views on this ...

Comment 10

[Anne (:annevk)]

This seems similar to bug 325550 which I came across this morning. We should probably make a consistent decision.

As for "then source must be set as the new browsing context's one permitted sandboxed navigator", I think removing that is fine given what browsers seem to be doing. File an issue at https://github.com/whatwg/html/issues/new? (Although Chrome blocks ctrl+click, it doesn't block "Open in New Tab" which is what the specification cites as example.)

I also agree with your suggestions in the W3C bug that we should do lots of refactoring to this part of the specification. It being a fragile setup as Ian said is exactly the reason we keep running into issues. Unfortunately doing that is lots of work.

Comment 11

[Bob Owen (:bobowen)]

I've filed https://github.com/whatwg/html/issues/1526 over getting the spec changed to allow Firefox's behaviour.