Skip to content
Please note that GitHub no longer supports Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

Learn more
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

Should "familiar with" use same origin or same origin-domain? #3747

Open
annevk opened this issue Jun 8, 2018 · 9 comments
Open

Should "familiar with" use same origin or same origin-domain? #3747

annevk opened this issue Jun 8, 2018 · 9 comments

Comments

@annevk
Copy link
Member

@annevk annevk commented Jun 8, 2018

https://bugzilla.mozilla.org/show_bug.cgi?id=1459671 was filed against Firefox because it uses a same origin check. Per investigation from @bzbarsky it appears that other browsers use same origin-domain. This means that you can target a cross-origin <iframe>'s name if you are same origin-domain with it.

It seems better if this concept does not depend on document.domain.

@whatwg/security interested in fixing this in your respective implementations?

Either way, we should probably also add a test for this as I don't think it's covered.

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

@bzbarsky bzbarsky commented Jun 8, 2018

Per investigation from @bzbarsky it appears that other browsers use same origin-domain.

Or don't do a security check at all. Or do something completely unrelated to the spec.

All I can tell from my investigation to the extent I did it is that they don't do the thing the spec says, whatever they do.

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

@bzbarsky bzbarsky commented Jun 8, 2018

For example, the testcase in https://bugzilla.mozilla.org/show_bug.cgi?id=1459671 would "pass" if you can target a frame if you're same origin-domain with its parent; that's the check in the fourth bullet of "familiar with". So it could also be that the non-Firefox browsers do "same origin" in the first bullet and "same origin-domain" in the fourth bullet. More tests needed to tell what they actually do.

@muodov

This comment has been minimized.

Copy link

@muodov muodov commented Jun 8, 2018

I'm the original reporter of the issue.

Indeed, as per current spec, it seems that Firefox is the only browser strictly following the algorithm. However, I am still trying to find a "bad" scenario when "same origin-domain" check would lead to a security flaw.

It seems to me, that the fourth bullet in "familiar with" intended to restrict access from a browsing context A to a browsing context B, which is located under a different top-level context. For example, if you open https://x.zok.pw/ff_frame_target/two-roots/victim-root.html and https://y.zok.pw/ff_frame_target/two-roots/evil-root.html in separate browser tabs, the form in evil-root will not be able to navigate the frame inside victim-root.
This is already the case both in Chrome and in Firefox.

In the case of https://x.zok.pw/ff_frame_target/index.html however, I see no additional security benefit in "same origin" over "same origin-domain". Both siblings and root contexts set their document.domain which effectively unlocks DOM access and location API. Moreover, "allowed to navigate" is much less strict than "familiar with", so one frame can traverse and relocate all the others.

One thing that is tricky with "same origin", it that one frame cannot simply set form target to the sibling frame. But having full access to the sibling's DOM, it's possible to craft that request by other means.

However, the current spec explicitly states "same origin", so I assume there's some edge case I'm missing here. Looking forward to hearing the reasoning from @whatwg/security.

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

@bzbarsky bzbarsky commented Jun 8, 2018

The basic premise in the spec is that document.domain is deprecated and aiming to be removed, and that therefore as much as possible should ignore it...

@muodov

This comment has been minimized.

Copy link

@muodov muodov commented Jun 12, 2018

I'm all for removing document.domain as long as there's another API to allow such cross-origin interactions. At the moment, some things can be implemented with postMessage but this case (form and link targets) is tricky.

@annevk

This comment has been minimized.

Copy link
Member Author

@annevk annevk commented Mar 22, 2019

This is basically a subset of the algorithm/problem outlined in #313 (comment).

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

@bzbarsky bzbarsky commented Nov 5, 2019

@mikewest @domenic Do you know what Chrome is doing here or who might know?

@domenic

This comment has been minimized.

Copy link
Member

@domenic domenic commented Nov 5, 2019

@zetafunction pointed me to https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/page/frame_tree.cc?rcl=db540489f9a4864385c80e3968c175cd6e335266&l=228 . It looks like the relevant bits are in CanNavigate which mashes together the spec's "allowed to navigate" and "familiar with", I think. In particular that has a lot of origin->CanAccess() calls which is documented as

  // Returns true if this SecurityOrigin can script objects in the given
  // SecurityOrigin. For example, call this function before allowing
  // script from one security origin to read or write objects from
  // another SecurityOrigin.

i.e. it seems to be concerned with same-origin-domain, not same-origin.

@muodov

This comment has been minimized.

Copy link

@muodov muodov commented Nov 6, 2019

Thanks @domenic!
Considering the above, is there anything that blocks the spec change ("same origin" -> "same origin-domain"), and, eventually, the change in Firefox? The deprecation of document.domain should, arguably, not be a blocker, since there is no newer API that allows such cross-origin frame targeting. In my opinion, this is basically a compat issue of Firefox at the moment.

@zcorpan zcorpan added the needs tests label Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.