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

bindHandlers broken when in cross-origin iframe within file: page #124

Closed
j-f-g opened this issue Apr 22, 2016 · 3 comments
Closed

bindHandlers broken when in cross-origin iframe within file: page #124

j-f-g opened this issue Apr 22, 2016 · 3 comments

Comments

@j-f-g
Copy link

@j-f-g j-f-g commented Apr 22, 2016

This possibly applies to other non-http(s): schemes as well (applewebdata:?) but I haven't confirmed. I've reproduced on the latest Chrome and Firefox.

Scenario:

The top page is an html document loaded from local disk (file://.../mypage.html). This page creates a cross-origin iframe on some remote server (https://www.mydomain.com/iframe.html). The following code inside both bindHandlers methods is broken in this case:

var a = document.createElement('a');
a.href = document.referrer;
if (a.host === window.location.host && window.parent !== window) {
  oCanvas.addDOMEventHandler(core, window.parent.document, type, function (e) {
    self.docHandler(e);
  }, false);
}

It's trying to use the anchor tag to detect if it's in a cross-origin iframe or not*, to determine whether accessing the parent document is safe. However, in this case, document.referrer is "", because of the file: scheme. The value of a.href is "", but the value of a.host just becomes the host of the current iframe**. So now, you've been fooled into thinking you're running inside a friendly iframe and can safely access the parent context, when that is not actually the case and the code breaks.

FWIW, my preferred method for testing cross-origin-ness is just to try to access what I want inside a try-catch. But if that's not preferred, I guess you want to check that document.referrer isn't the empty string here. I'm not totally sure that's sufficient (the try-catch is the most rigorous technique), as a.host would resolve to the current iframe's host for any .href value that didn't start with an actual scheme. However, I don't know if document.referrer is ever going to do that outside of this scenario, so maybe you're ok.

* It's also worth noting that the host equality check isn't actually sufficient to determine cross-iframe access. You have to match the entire origin (including protocol and port). You could use a.origin === window.location.origin except a few older browsers (IE) don't support the origin property. For the most part though, it would be pretty rare for this to issue to bite you. Using ports is not very common, and it would be fairly odd to have an https iframe inside an http page on the same domain (although certainly not impossible).

** For any a.href value that doesn't start with a scheme/look like an absolute URL, the browser assumes it's a relative path. So it uses the base URL, which in this case is just the URL of the iframe we're running inside of, to resolve the path. That's how we end up with the host of the current iframe as a.host.

@koggdal
Copy link
Owner

@koggdal koggdal commented Apr 22, 2016

Thanks for reporting it, and with such detailed information! 👍

The try..catch solution seems to be a good fit here. I'll add that and release a patch. Thanks!

koggdal added a commit that referenced this issue Apr 22, 2016
The previous method of checking for cross-origin was insufficient
(#124). A better way is to
try performing the action and just ignore the exception if thrown.
@koggdal
Copy link
Owner

@koggdal koggdal commented Apr 22, 2016

Fixed in 2.8.4! It's on the website for download and in npm. It will come to the CDN soon.

http://ocanvas.org/download#v2.8.4

Thanks for reporting!

@koggdal koggdal closed this Apr 22, 2016
@j-f-g
Copy link
Author

@j-f-g j-f-g commented Apr 22, 2016

No problem, thanks for the quick fix. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.