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 Security Issue #3558

Closed
burninatorsec2 opened this issue Mar 29, 2022 · 7 comments
Closed

XSS Security Issue #3558

burninatorsec2 opened this issue Mar 29, 2022 · 7 comments

Comments

@burninatorsec2
Copy link

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
Copy link
Author

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

@renaiku
Copy link

renaiku commented Apr 5, 2022

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 .

@burninatorsec2
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@tjad
Copy link

tjad commented Nov 3, 2022

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.

@quill-bot
Copy link

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 🙏

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

4 participants