mindmup / bootstrap-wysiwyg

Tiny bootstrap-compatible WISWYG rich text editor
MIT License
5.55k stars 840 forks source link

XSS in bootstrap-wysiwyg #142

Open soaj1664 opened 10 years ago

soaj1664 commented 10 years ago

Hi,

The editor is vulnerable to an XSS. The editor allows users to insert link and if instead of normal link, I input JavaScript URI

javascript:alert%28location%29

then it works. The attacker can execute arbitrary code of his choice. Please fix this issue. Thanks!

mikepmtl commented 10 years ago

Why don't you simply filter out what you don't want upon submission server side? Some of us use it in the back end to add/edit content.

On any form it should be your responsibility to clean up the input to your needs.

On Apr 13, 2014, at 8:32 AM, Ashar Javed notifications@github.com wrote:

Hi,

The editor is vulnerable to an XSS. The editor allows users to insert link and if instead of normal link, I input JavaScript URI

javascript:alert%28location%29

then it works. The attacker can execute arbitrary code of his choice. Please fix this issue. Thanks!

— Reply to this email directly or view it on GitHub.

soaj1664 commented 10 years ago

Hi @mikepmtl

In case of TinyMCE and Jive, they filter JS URI and do not allow user to even input it. I still believe it is should be available as a part of WYSIWYG editor. It would be safe by default. Isn't it?

mikepmtl commented 10 years ago

Since it's all running client side, a user can submit anything they want really, no matter what the editor does. They can edit any value upon submission pretty easily these days.

If you are leaving security up to a client side script, it's not very secure.

Personally I will allow anything in, but I filter everything on output, rendering it useless. This way at least I know what people are doing.

On Apr 13, 2014, at 9:33 AM, Ashar Javed notifications@github.com wrote:

Hi @mikepmtl

In case of TinyMCE and Jive, they filter JS URI and do not allow user to even input it. I still believe it is should be available as a part of WYSIWYG editor. It would be safe by default. Isn't it?

— Reply to this email directly or view it on GitHub.

soaj1664 commented 10 years ago

I know client side scripting can be easily bypassable but at least what you can do is to force user to submit a legitimate value e.g., in this case it can be easily done on client-side by appending http or https and it will make JS URI affect null n void.

If you want to leave this as such then its OK . :)

mikepmtl commented 10 years ago

I am not the author. I am simply another user of this script.

My point is that what you consider a "legitimate value" is not necessarily what others need as a legitimate value.

We all have different needs.

Regards, Mike

steveathon commented 10 years ago

I think it's important to note here, that because you cannot pass a bookmark value populating the editor with the html with scripting, that it's not really exposed as an XSS.

If on the server side, no filtering is happening and the input is being blindly output, really, you are exposing some weaknesses - but that's server side, and outside of the scope of the client.

As @mikepmtl has pointed out, there's nothing stopping someone submitting arbitrary code into any form , what matters is what happens server side.

That said, if you enable / extend the editor to allow external passing of editor text via a URI or GET param, then you are exposing the user - but that's not something you should be doing.