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

Add allow-same-origin policy to pad iframes #2044

Closed
wants to merge 1 commit into from

Conversation

srawlins
Copy link
Member

Work towards #1946.

My only concern is running with this in embedded pads. Security concern?

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/sandbox

I don't... think so?

@parlough
Copy link
Member

I think this is mostly fine, but one potential question. Could this potentially allow someone to send a DartPad gist link to a user and then extract that user's saved DartPad/gist code?

Copy link
Contributor

@johnpryan johnpryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -Since this gives the Flutter iframe access to the users cookies and data associated with dartpad.dev, it's possible that the Flutter app might be able to modify the user's snippet.

Embeds don't store the user's edits in the same way, but we should test to make sure #1946 is fixed in embeds too.

@parlough
Copy link
Member

I'm not so worried about the app modifying it, but rather reading it and sending it off somewhere. I know it's not really DartPad's intended use case, but a user's stored code could potentially contain proprietary or personal information.

If this possibility is acceptable for DartPad's purposes, that's fine, I just want to make sure the potential security/privacy issues are kept in mind.

@johnpryan
Copy link
Contributor

johnpryan commented Oct 19, 2021

That's a good point - we should make sure this is the best fix available and that it doesn't compromise the user's privacy / security in any way.

@srawlins
Copy link
Member Author

Here's a visual of the embedded pad:

*------ evil user's website -----------------*
|                                            |
|  *---- embedded pad iframe -------------*  |
|  |                                      |  |
|  |      (A)      *-- render iframe --*  |  |
|  |  dart source  |        (B)        |  |  |
|  |               |                   |  |  |
|  *---------------*-------------------*--*  |
|                                            |
*--------------------------------------------*

So the question is whether JS running in evil user's website can access (A) the dart source in embedded pad iframe or from (B) the rendered content in render iframe.

My experiments tell me that this is completely based on the sandbox properties of the iframe which provides the boundary between the outer page and the inner page. This means it is already possible for users to access (A), if they:

  • write an evil website that embeds a pad,
  • use the allow-same-origin policy on the iframe,
  • and host the pad at their same host, so that the pad and the website have the same origin.

They cannot access (A) if (A) is loaded from https://dartpad.dev though.

If they can access (A), then they are hosting their own pad, and can obviously change the render iframe's security to be whatever they want, and already have access to (B).

If they cannot access (A), they cannot access (B).

@srawlins
Copy link
Member Author

Ah, but one fun loophole. Is there any way for an evil user to host an evil site at dartpad.dev? Maybe: gists.

https://dartpad.dev/?id=... could load in a pad (which is set to execute on loading, right?). When this pad executes, it does visual trickery to replace the entire window to be their own imitation of dartpad.dev/, wherein they invite users to put confidential code.

I believe with allow-same-origin, playground pad code could reach outside of the render frame and manipulate DOM in the parent frame. And I believe without allow-same-origin, playground pad code cannot reach outside of the render frame.

@johnpryan
Copy link
Contributor

johnpryan commented Oct 20, 2021

Closing in favor of #2048

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

Successfully merging this pull request may close these issues.

None yet

3 participants