Skip to content
This repository has been archived by the owner on Mar 8, 2019. It is now read-only.

Iframe optional #33

Closed
julesbou opened this issue Apr 2, 2012 · 10 comments
Closed

Iframe optional #33

julesbou opened this issue Apr 2, 2012 · 10 comments

Comments

@julesbou
Copy link

julesbou commented Apr 2, 2012

The iframe does not seems to be optional, i've hacked quite a lot to replace it by a div.

Is it possible in a next release to make it optional?

Thank you

@tiff
Copy link
Owner

tiff commented Apr 2, 2012

Hi @gordonslondon,

I don't think that this will soon be part of wysihtml5.
wysihtml5 uses an advanced security concept where the editor's iframe acts as a sandbox (<iframe security="restricted">).
It's needed to avoid unwanted script execution (xss) while working with the editor.

Scenario:
User copies content from an external page and inserts it into the editor. Now the copied content could contain hidden script tags that possibly access cookies and do other harm to the user.

I did some extensive research on this topic. Check this test suite:
http://tifftiff.de/contenteditable/onpaste_test.html

and the resulting code:
https://github.com/xing/wysihtml5/blob/master/src/dom/sandbox.js

wysihtml5 is by now the only editor with such an approach on security.

However maybe I can help you with your problem: Why did remove the iframe?

@julesbou
Copy link
Author

julesbou commented Apr 2, 2012

My use case: i'm building a cms where user can style his page (this will generate css in a <style> tag) and edit content in the same time, for the moment I haven't find a way to allow the user to do both: design the page and edit content (cuz the css generated in the page cannot be accessed throw the iframe).

I've looked at the stylesheets option, allowing me to pass an array of css files. It's not enough for me, when the user edit the design, css files are not generated until the user save the page. (so my content inside wysihtml5 editor is still unstyled).

I was aware about your security concerns, also your tests shows that only IE is affected, is there any way to strip all javascript from user input in this browser ?

@tiff
Copy link
Owner

tiff commented Apr 2, 2012

To first answer your question: No there isn't a solution to that.

However if you need to apply styles dynamically to the editor then I think I might help:
wysihtml5.dom.insertCSS(["div { color: red; }"]).into(editor.composer.sandbox.getDocument());

('editor' is in this case the instance)

Find the source code here: https://github.com/xing/wysihtml5/blob/master/src/dom/insert_css.js

@julesbou
Copy link
Author

julesbou commented Apr 2, 2012

Interesting, will try asap. I'm closing the issue for now. Thanks.

@julesbou julesbou closed this as completed Apr 2, 2012
@julesbou
Copy link
Author

julesbou commented Apr 2, 2012

@tiff I don't understand something, if we use an iframe to prevent javascript from being executed, does it mean we have to render the content in an iframe too (for a user without editing privilege). I have 10+ instances of wysihtml5, does it mean i've 10 iframes on my page ?

@tiff
Copy link
Owner

tiff commented Apr 3, 2012

Hell no :)

As with any other input where users can enter html you have to strip malicious code on the server side before saving it by using a html sanitizer.
The iframe is only for catching security threats while the user is working with the editor.

There's even one html5 sanitizer that was built in perl and uses the wysihtml5 parser rules set:
https://github.com/xing/html5-sanitizer

@julesbou
Copy link
Author

julesbou commented Apr 3, 2012

ooh, haven't seen the html5-sanitizer, looks good for me, thanks for the infos and your patience @tiff.

@huemorgan
Copy link

Correction
only this worked for me - add [] because the function insertCSS expects an array - not a string.

wysihtml5.dom.insertCSS(["div { color: red; }"]).into(editor.composer.sandbox.getDocument());

@tiff
Copy link
Owner

tiff commented May 2, 2012

Thanks and sorry!

melvinsembrano pushed a commit to melvinsembrano/wysihtml5 that referenced this issue Jul 30, 2014
@vigneshkumarmr
Copy link

Hi @tiff

This XSS issue needs to be fixed though I guess.
xss

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

No branches or pull requests

4 participants