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

quill is not safe #2

Open
sijad opened this issue Oct 15, 2017 · 8 comments
Open

quill is not safe #2

sijad opened this issue Oct 15, 2017 · 8 comments

Comments

@sijad
Copy link

sijad commented Oct 15, 2017

see slab/quill#981 for more info.

for reproducing xss go to http://flarum.courierhost.com/d/15-xss and edit the last post.

https://youtu.be/w1uPaI0Vvds

@esledov
Copy link
Owner

esledov commented Oct 15, 2017

sijad,

Thank you for your interest in this. Please, correct me if I am wrong.

I think when you insert specially constructed html into quill that affects only your own view before the post is saved.

After post it is saved and shown again (and this is when other users can see it) only limited list of html tags/attributes should be allowed to pass through the flarum's s9e text formatter machinery.

@sijad
Copy link
Author

sijad commented Oct 15, 2017

s9e text only affects on output, in

this.quill.clipboard.dangerouslyPasteHTML(0, this.value());
you're feeding quill with original user input which can contains malicious codes. this can also be triggered by admins, if they try to edit attacker post.

a proper fix would be use HTML parser like htmlpurifier to cleanup user input before it goes through s9e. but it is possible to introduce other problems for example user might see different content when he try to edit his posts or even lost some of his contents due htmlpurifier.

@esledov
Copy link
Owner

esledov commented Oct 15, 2017

you're feeding quill with original user input which can contains malicious codes.

User himself enters html. He can enter the same html into notepad, save it as myfile.html and open it in browser. Does it make notepad unsafe?

this can also be triggered by admins, if they try to edit attacker post

There is no way. When somebody else opens the post, offending html will be sanitized.

There is html filtering on server side. I understand your point, but I think implementing html filtering on client side as well is kind of excessive.

@sijad
Copy link
Author

sijad commented Oct 15, 2017

User himself enters html. He can enter the same html into notepad, save it as myfile.html and open it in browser. Does it make notepad unsafe?

it's not a self xss, as I said this can be triggered by admins when they try to edit attacker post.

There is no way. When somebody else opens the post, offending html will be sanitized.

you can login as an admin and try to edit this post: http://flarum.courierhost.com/d/16-xxs-again

@esledov
Copy link
Owner

esledov commented Oct 15, 2017

I am logged in as admin now.

When I am looking at the source of this post

  <div class="Post-body">
      &lt;svg onload=alert(document.location)&gt;
  </div>

I see that svg tag is sanitized.

When I open post for editing, I don't see any content. I assume it was filtered out.

@sijad
Copy link
Author

sijad commented Oct 15, 2017

didn't you see an alert when you click on edit link?

@esledov
Copy link
Owner

esledov commented Oct 15, 2017

Nope

@sijad
Copy link
Author

sijad commented Oct 15, 2017

https://youtu.be/skdBnMt4fh4

I just want to make sure we're in same page, as I said before quill is not safe (unlike TinyMCE and CKEditor). I just wanted to warn you, as this impact all Flarum users.

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

No branches or pull requests

2 participants