quill icon indicating copy to clipboard operation
quill copied to clipboard

XSS Security Issue

Open burninatorsec2 opened this issue 2 years ago • 5 comments

Please describe the a concise description and fill out the details below. It will help others efficiently understand your request and get to an answer instead of repeated back and forth. Providing a minimal, complete and verifiable example will further increase your chances that someone can help.

Steps for Reproduction

  1. Visit a site using Quill
  2. As an attacker, you enter nonsense text into the QL editor text box
  3. You intercept your own traffic to edit the nonsense text in the HTTP request to be the payload listed under CVE-2021-3136 (https://burninatorsec.blogspot.com/2021/04/cve-2021-3163-xss-slab-quill-js.html)
  4. Now the XSS payload is stored on the server
  5. When a victim visits the same page, the XSS launches, and can execute key logging, session stealing, among other malicious actions

Expected behavior:

  1. I expect Quill to filter out malicious payloads server-side like other major wysiwyg editors and JavaScript libraries. Client-side-only remediation won't be enough since the user can control traffic moving to the server.

Actual behavior:

  1. The XSS payload launches for other users

Platforms:

This works on all browsers and operating systems

Version:

1.3.7

burninatorsec2 avatar Mar 29 '22 15:03 burninatorsec2

This was previously disclosed under https://github.com/quilljs/quill/issues/3273, but unfortunately that appears to have been deleted or inaccessible at the moment

burninatorsec2 avatar Mar 29 '22 15:03 burninatorsec2

I'm not sure that this is an issue. I've always been taught in backend coding to never trust any clients. The payload HAS to be sanitized by the backend anyway.

The only case I can imagine, is that you have no control on the backend, and you are asking text from an external API that might contain an exploit, and even then, only a client that could remove the check from the frontend code would be affected. Meaning the hacker would hack themself ?

I'm maybe wrong, but I don't see the problem here that can't be fixed by respecting some basic standard security coding behaviors .

renaiku avatar Apr 05 '22 09:04 renaiku

Thank you for your response.

The way this works is not the hacker hacking themselves: since this is stored XSS, not reflected XSS, this is an issue that affects other users without needing any action on behalf of users and without intercepting their traffic. It is more severe than reflected XSS. The interception proxy is used by the attacker locally in order to bypass clientside protections against storing the XSS in the first place. It is important to have layers of security, in case validation is bypassed on the backend OR the frontend. In general, validation works best went it is done in both client and server side, and using a library that introduces a vulnerability while relying on a backend filter only is only a bandaid fix. We would never rely only on the security of a 3rd party application, but it is best to report exploits directly caused by them and fix them at the source.

There must already be some XSS filtering in Quill because certain types of XSS payloads don't work in Quill. Perhaps the XSS filtering behavior that would fix this one can be added there?

Because of the nature of what WYSIWYG editors do (processing HTML tags), other WYSIWYG editors have had this same issue that they've released security patches for. For example, consider this very similar stored XSS issue in CKEditor, which has a great explanation here: https://checkmarx.com/blog/cve-2021-33829-stored-xss-vulnerability-discovered-in-ckeditor4-affects-widely-used-cms/ Additionally, CKEditor provided a user setting to disallow scripts, which also solved the issue.

If you'd like I can try to find more open source examples of JavaScript libraries (in particular, WYSIWYG editors) fixing XSS issues, to use as a guide.

I hope this information helps, please feel free to reach out.

burninatorsec2 avatar Apr 05 '22 12:04 burninatorsec2

Additionally, for an overview of this type of security issue, here are some OWASP resources about Injection which includes XSS: https://owasp.org/Top10/A03_2021-Injection/

From a more technical standpoint: input from the user should be validated using a white-listing approach so only characters expected to be found are included in values. User input should be HTML-encoded at any point where it is copied into application responses. All HTML metacharacters, including <>” ‘ and =, should be replaced with the corresponding HTML entities (< >..etc).

burninatorsec2 avatar Apr 05 '22 12:04 burninatorsec2

Hello - Any updates on this? Has there been a security patch released for it to include the validation code mentioned above? Thanks

burninatorsec2 avatar Sep 06 '22 13:09 burninatorsec2

If you are not using Quill Deltas with quill.getContent() and quill.setContent(), you have a 10T error (your usage is incorrect). If you paste malicious content directly as innerHTML into any element, you have problems.

You clearly do not understand and have misused quill editor.

tjad avatar Nov 03 '22 01:11 tjad

Quill 2.0 has been released (announcement post) with many changes and fixes. If this is still an issue please create a new issue after reviewing our updated Contributing guide :pray:

quill-bot avatar Apr 17 '24 10:04 quill-bot