Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

XSS possible by firstly disabling Javascript and entering some script #242

Open
duncan-at-hiveit opened this issue Feb 26, 2016 · 1 comment

Comments

@duncan-at-hiveit
Copy link

Hi there,

First off, thanks for the excellent WYSIWYG! It's been a real help to us.

Unfortunately we've run across a case in version 1.1.1 where it is possible to inject Javascript into the page.

Steps to reproduce:

  1. Visit a page with a WYSIWYG editor
  2. Disable Javascript
  3. Refresh the page
  4. Add some script e.g. <script>alert("Hallo!")</script> to the now-plain textarea.
  5. Save
  6. Enable Javascript and refresh the page
  7. A Javascript alert will pop up saying "Hallo!"

It seems to be able to do this by the fact that Hallo assumes that its content has been entered by itself. When it converts HTML to markdown it additionally adds a backslash to any ">" symbols it finds, thus rendering any HTML and Javascript entered in the WYSIWYG "disabled". So <script>alert("Hallo!")</script> would actually be cleansed to <script>alert("Hallo!")</script>

The problem therefore can occur when displaying text that hasn't been through Hallo's own save mechanism.

Many thanks,
Duncan

@bdekok
Copy link

bdekok commented Feb 29, 2016

Sorry, I don't think this really relates to the hallo editor.

I am in the same project as duncan and we've implemented something similar to your example with markdown (http://hallojs.org/demo/markdown/) and ran into XSS problems. Easily reproduced by putting i.e. "><script>alert(document.cookie);</script> in the source area (can also be done within your demo).

This is because the markdown to html conversion from showdown.js doesn't do any sanitizing as they explain here. In our project the code would be htmlentities after save "&gt;&lt;script&gt;alert(document.cookie);&lt;/script&gt;. However shodown would convert them back to proper html, which we don't want. We've fixed it now by sanitizing the output of showdown.js which is the method the shodown maintainers recommend.

Maybe you could do the same on the demo page for having a better copy paste demo for future use.

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

2 participants