Should the iframe point to a different domain?

[phkavitha]

Firstly, the app is really cool. Hats off!!

In order to prevent XSS attacks and other security issues, should the iframe in ember-twiddle point to a different domain? Similar to JSFiddle. Currently, we can access the parent window from the iframe code and can access the browser cookies as well.

Correct me if my understanding is wrong.

[Gaurav0]

Not only can we, we do access the parent window. (in order to update the address bar)

[Gaurav0]

We are also running without a content security policy in place.

[joostdevries]

Valid point. I'm not sure we could keep the current architecture (using doc.write) if we host the iframe on another domain (right now, it's about:blank).

Could you write a test that demonstrates your point?

[joostdevries]

We might be able to use a different domain, combined with postmessage..

[Gaurav0]

All versions of IE (other than Edge) have serious bugs in their postMessage support. Therefore we would likely have to limit our support to non IE browsers if we use postMessage. http://caniuse.com/#search=postMessage

Is a different domain absolutely necessary? What about sandboxed iframes? http://caniuse.com/#feat=iframe-sandbox

[joostdevries]

Whatever approach works is fine by me. The postMessage issues in IE mostly refer to other windows/tabs and lack of support for sending objects (which is not really an issue).

Whether we use a different origin or a sandbox, we will still need a way to communicate with the frame which will most likely be postMessage (which is also what the Ember Inspector uses).

[Gaurav0]

Plan of action:

1. Write an acceptance test that fails demonstrating that the parent window can be accessed from a twiddle.
2. Sandbox the iframe. Allow forms and allow scripts.
3. Make the test run. This will involve changes to the acceptance test helper run-gist which currently relies on access to parent window.
4. Verify the test passes.
5. Write a unit or acceptance test for the now non-functional address bar
6. Fix the address bar using post message
7. Make sure the test from (5) passes.
8. Look for any bugs and fix.
9. Create a content security policy. Make sure it allows everything the user expects to be able to do.
10. Look for any bugs and fix.
11. Write code such that if the iframe sandbox does not work (for example in IE < 10), refuse to run twiddle.

Request for comments.

[joostdevries]

:+1:

[Gaurav0]

@rwjblue @stefanpenner Comments on plan of action?

[stefanpenner]

All versions of IE (other than Edge) have serious bugs in their postMessage support. Therefore we would likely have to limit our support to non IE browsers if we use postMessage. http://caniuse.com/#search=postMessage

Although this is true, they will work fine for our use-case.

Is a different domain absolutely necessary? What about sandboxed iframes? http://caniuse.com/#feat=iframe-sandbox

Yes, I believe so. Sandboxing a frame (without just reenabling the parts that are dangerous) will break things that users are used to, like document.cookies, localstorage, indexdb, WebWoker etc.

for example:

Uncaught SecurityError: Failed to read the 'cookie' property from 'Document': The document is sandboxed and lacks the 'allow-same-origin' flag.

Uncaught SecurityError: Failed to read the 'localStorage' property from 'Window': The document is sandboxed and lacks the 'allow-same-origin' flag.(anonymous function) @ foo.html:2

[stefanpenner]

@hjdivad is likely a good person to pull in, as he has done copious work with iframes postMessage/messageChannel

[stefanpenner]

@Gaurav0 i do like your plan of attack, I would suggest it may need to be revised due to ^^.

Lets also see what @hjdivad has to say

[Gaurav0]

I am ok with breaking document.cookies and localStorage. @joostdevries ?

[hjdivad]

@Gaurav0's plan looks good to me.

I would add IE conditional comments to make sure unprotected iframes aren't run in IE8

Also, I'd recommend using MessageChannel over window.postMessage. We had some users run into bugs with Oasis.js because some libraries don't use window.postMessage properly (ie they failed to check that they were processing their own messages). There's a polyfill for FF and IE9 support.

Also we might be able to get cookie and localStorage working with proxies (that sync-d async over MessageChannel).

[joostdevries]

The vulnerability is now fixed and the fix is released but I'll keep this open because using a different domain is probably a better solution than the current fix (using sandbox/srcdoc)

[joostdevries]

@phkavitha Thanks for pointing this out! In the future though, any suspected vulnerabilities are best reported to security@emberjs.com so we can work on a fix before disclosing.