Allowing scripts in Thimble still requires security measures on the preview iframe.

RESOLVED DUPLICATE of bug 863737

Status

RESOLVED DUPLICATE of bug 863737
6 years ago
5 years ago

People

(Reporter: pomax, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
friendlycode does no iframe sandboxing (even if it did, a disheartening number of browsers won't do anything with that), so if we allow script publication for authenticated users, user X can create a page with cookie-stealing scripts, and publish that legally; if an authenticated user Y then hits "remix", I'm pretty sure that would lead to a compromised session.

we could set up a safety mechanism so that if someone remixes a page with script, they first get a warning saying "this page has scripts, you better know what you're doing", but people click throught that anyway; Another option is to load a fully sanitized version of a page on remix (i.e. if a not-owning user loads it in the editor) and then for each <script> element ask "do you know what this does?" and put it back as a normal script element into the page source if they indicate they know what they're doing. Then again, for a page with 10 or 20 scripts, that gets quite tedious.

Short of a sandboxed iframe, I'm not sure what the safe solution is, here. Is there a way to create an iframe with a template from another domain, and then modify its content so that scripts inside that iframe run on a different domain, and can't steal editor session cookies?
Group: core-security → websites-security
(Reporter)

Comment 1

6 years ago
Created attachment 768317 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/164

This is a stub pull request to enable scripts in Thimble, by making the preview pane an actually loader.html file that lives on a different domain, updating the preview using postmessages.

This is not so much a final code suggestion, but a proposal on how we can do this, locking down the preview in a way that allows scripts.
(Reporter)

Updated

6 years ago
Flags: needinfo?(jon)
Flags: needinfo?(david.humphrey)
Group: websites-security
This looks pretty sane as-is. We could move forward by enabling the preview loader to live within Friendly Code for starters, then look at deploying it somewhere else on the web.
Flags: needinfo?(jon)
I made some comments in both PRs, and I agree with Jon that this looks like a solid plan.  It might be useful to show it to mgoodwin too?  I can't really evaluate it from a security POV well enough to say "yes", and that's really what this patch is about.
Flags: needinfo?(david.humphrey)
(Reporter)

Comment 5

6 years ago
I'm marking this as a duplicate, since at this point it pretty much is.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 863737
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.