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

XSS Vulnrability #1627

Closed
adameska opened this Issue Jan 18, 2016 · 16 comments

Comments

Projects
None yet
@adameska
Copy link

commented Jan 18, 2016

You can edit the text in code view mode and right scripts that get executed. This can then be sent to the server.

@stdex

This comment has been minimized.

Copy link

commented Jan 20, 2016

And how you want to use this "vulnrability"? Right way filter text on server side.

@adameska

This comment has been minimized.

Copy link
Author

commented Jan 20, 2016

We do filter text serverside. Just thought i would bring it to your attention in case you didn't already know about it. One way would be to have an optional way to remove code view (not sure if that exists or not).

@TBarina

This comment has been minimized.

Copy link

commented Jan 21, 2016

I also would like to have an optional way to remove code view. In fact I can enter scripts in code view that get executed switching back to normal view.
Is there a way to prevent this?

@nwpan

This comment has been minimized.

Copy link

commented Jan 26, 2016

I'm running into this issue as well -- my team's QA caught this. We ended up injecting a simple script tag (within the code view) like this:
<script>alert(1);</script>

And an alert appears.

@ghost

This comment has been minimized.

Copy link

commented Jan 27, 2016

As @stdex mentioned sanitation should be done server side before storing data, and before output.

@irandamay

This comment has been minimized.

Copy link

commented Mar 23, 2016

You should sanitize for sure, but the scripts being executed have the ability to trap the user in the codeview if they mess something up, see #1462

@byanjiong

This comment has been minimized.

Copy link

commented Apr 30, 2016

please dont remove the code-view, the code view is very very important to do fine-tuning for the html, i.e can add class, style, etc. Server side should handle all the filtering task. However, may consider to fix the bug that execute code view's script... just a suggestion... (code-view is very important...)

@SG5

This comment has been minimized.

Copy link

commented Jun 21, 2016

There is an example of xss - https://jsfiddle.net/9sdhwdzw/
script tag was escaped, but summernote executes it

@jplew

This comment has been minimized.

Copy link

commented Sep 28, 2017

According to @balping, <script> execution in the browser is nothing to worry about:
thekordy/ticketit#192 (comment)

I know little about XSS injections, so can someone else please confirm? Are we secure as long as we intercept the malicious code before it enters the database? What if the code is sent as a POST parameter, for pre-processing before being saved to the database? Is that dangerous?

@bangnokia

This comment has been minimized.

Copy link

commented Feb 23, 2018

it's happen to me

@kevzlou7979

This comment has been minimized.

Copy link

commented May 29, 2018

Are there any updates on this?

@DiemenDesign

This comment has been minimized.

Copy link
Member

commented May 30, 2018

As previously mentioned, it is up to the developer to sanitise data sent to the server for storage. There are plenty of online resources on how to do this in any backend language your using.
It also wouldn't hurt to sanitise output data as well.

@minuz

This comment has been minimized.

Copy link

commented Jun 15, 2018

I believe it would be useful to have sanitization on the client side as well. There's an obvious implementation to prevent malicious code to be persisted (our team thought that was enough)
However, there's an undergoing application penetration test and whole security check and it has been pointed out to us that back-end sanitization is good, but we also should do on the front-end.

I'm doing it on the Angular Side, but there are DOM manipulation from summernote that happens before I have the chance to sanitize it. Is there a way or an event that I can hook and sanitize before switching the view?

@DiemenDesign

This comment has been minimized.

Copy link
Member

commented Jun 15, 2018

The problem of adding in front end sanitation is that there will be people that need or want to edit javascript in the editor, sanitation would prevent this.
I think the best way to implement something to sanitise would be via a plugin that can monitor changes, or intercept the data before being sent to the server. If you have a look at the summernote-save-button plugin, you'll see how it can be done when submitting the editor when it's in a form.

@minuz

This comment has been minimized.

Copy link

commented Jun 18, 2018

Thanks @DiemenDesign, I'll have a look on the plugin you mentioned.
Nevertheless, I still think it would be nice to provide an event so one could simply subscribe to it before the default action happens, thus providing the chance to sanitize and this choice would still be to whoever implements the library. Does that make sense?

@DiemenDesign

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

It does, though, a plugin still make more sense. Implementing a sanitiser within the Summernote API would create a lot of work, not only to implement but to also keep up to date. I also think it's outside of the scope of what Summernote is here to achieve in an editor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.