Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

History traversal and the iframe sandbox. #880

Closed
bobowen opened this issue Mar 15, 2016 · 8 comments · Fixed by #4787
Closed

History traversal and the iframe sandbox. #880

bobowen opened this issue Mar 15, 2016 · 8 comments · Fixed by #4787

Comments

@bobowen
Copy link

bobowen commented Mar 15, 2016

I have a few questions about the current specification for history traversal [1] and in particular how browsing context sandboxing [2] affects it.
I'll number them for referencing.

Suppose we have a top level browsing context A (with history) that contains a sandboxed iframe B without allow-top-navigation but with allow-scripts and also without any history.
Due to the joint session history being used, if B contains the script history.go(-1);, then at step 2 of [1] the specified entry selected would be the previous entry of A, even though B is not allowed to navigate A directly.
(1) Should the sandbox affect history traversal triggered by the sandboxed browsing context as well?

(2) If, as history traversal is different from navigation, it is decided that it shouldn't, then what about when history traversal triggers navigation at step 1 of [3]?
Here the original source browsing context is used and so B could indirectly cause navigation of A.

This leads to a few questions about step 1 of [3].
(3) Is the original source browsing context supposed to be kept alive by any history entries that it may cause to be created?
(4) If not, what happens if it no longer exists?
(5) Also, is it the current state of the original browsing context that should be used? (This is sort of what the spec implies, by not mentioning this.)
(6) Or should the history entry, on its creation, merely store the state of the source browsing context that affects navigation? (That would make (3) and (4) irrelevant.)

As an example for (5) and (6), suppose we have a top-level browsing context with two iframes C and D.
D has a previous history entry (which no longer has a document associated with it) and C was used as source browsing context when that history entry was created.
C has subsequently had a sandbox attribute set, with no keywords and been navigated, so its active document has picked up the new sandbox flags.
D does history.go(-x); and that refers to its previous history entry which no longer has a document, so needs navigation.
If the current state of C were used then in theory this navigation should fail.

Thanks,
Bob

[1] https://html.spec.whatwg.org/multipage/browsers.html#traverse-the-history-by-a-delta
[2] https://html.spec.whatwg.org/multipage/browsers.html#sandboxing
[3] https://html.spec.whatwg.org/multipage/browsers.html#traverse-the-history

@annevk
Copy link
Member

annevk commented Mar 15, 2016

Paging @mikewest.

@mikewest
Copy link
Member

I think the "sandboxed top-level navigation browsing context flag" should mean that the frame can't change the location of the top-level window, either directly via window.top.location or indirectly via history.go. Similarly, I think that the "sandboxed navigation browsing context flag" should mean that the frame can't change the location of various other non-nested browsing contexts to which it might be able to obtain a handle.

That seems like the interpretation closest to the colloquial meaning of the sandboxing flag, though I agree that it makes history weird in these scenarios. I don't have a good answer for your questions about 5 and 6. I don't honestly know what Chrome does for any of these scenarios. :)

@bobowen
Copy link
Author

bobowen commented Mar 15, 2016

Thanks Mike.
I agree that it generally makes sense for the sandboxing flags to block these navigations, but I'm not sure if there are any knock-on problems with the way this changes the behaviour of history.
As iframe sandbox is relatively new I'd hope that it wouldn't matter.

Chrome allows the history.go at the moment, as does gecko.

I'm not at all sure about questions 3 to 6.
Gecko currently effectively uses the target browsing context as the source browsing context, I think.
Not sure what Chrome does.

But maybe when the history traversal is done by script, it should be the browsing context of that script that should be used.
Or if that causes problems, the initial check to see if history traversal is allowed should use the script's browsing context and if a subsequent navigation is required then the original source browsing context is used.

@bobowen
Copy link
Author

bobowen commented May 9, 2016

@bzbarsky @smaug---- what are you thoughts about this?
Should we make navigation caused by history.go subject to browsing context sandboxing?

@smaug----
Copy link

Based on what is about to happen with iframe in shadow DOM, yes. history.go() in sandbox iframe shouldn't affect the global session history state. So I assume history.go() should throw or be no-op inside sandbox, or if we get the per-iframe session history defined for the shadow DOM case, perhaps sandbox could use the same.

@bzbarsky
Copy link
Contributor

bzbarsky commented May 9, 2016

Yeah, this seems like a hole in the sandbox setup....

@annevk
Copy link
Member

annevk commented May 4, 2017

@hayatoito how did you end up solving iframe in shadow DOM? Or is that still unsolved?

@smaug----
Copy link

WICG/webcomponents#184 is still open. IIRC the plan was that rniwa would try to implement what we (I, @rniwa, @hayatoito, @TakayoshiKochi) agreed at TPAC.

dtapuska added a commit to dtapuska/html that referenced this issue Jul 18, 2019
The spec was a little unclear whether sandboxed navigation was prevented
if it causes a top-level navigation via the history API. The check for
the navigation was after the unload steps of the history traversal.

Fixes whatwg#880
domenic pushed a commit that referenced this issue Aug 13, 2019
Previously, sandboxed navigation did not prevent navigation via the history API;
the check for the navigation was after the unload steps of the history
traversal. This adds an explicit check in those methods to prevent such
navigation.

Fixes #880.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants