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

Sanitize all the html content to solve security concerns #3082

Open
ananzh opened this issue Dec 14, 2022 · 1 comment
Open

Sanitize all the html content to solve security concerns #3082

ananzh opened this issue Dec 14, 2022 · 1 comment
Assignees
Labels
refactor Tech debt related tasks that need refactoring security feature technical debt If not paid, jeapardizes long-term success and maintainability of the repository.

Comments

@ananzh
Copy link
Member

ananzh commented Dec 14, 2022

From #2905 discussion, we found that we currently use dangerouslySetInnerHTML to let react know this is html content. There are multiple places in our code base using dangerouslySetInnerHTML

dangerouslySetInnerHTML={{ __html: output }} // eslint-disable-line react/no-danger

If XSS is really a concern and we think customer data is not trustful enough for security purpose, another way is to add dompurify. It is a DOM-only, super-fast, uber-tolerant XSS sanitizer for HTML, MathML and SVG.

@joshuarrrr
Copy link
Member

This can be done in addition to, or alternatively to #2932. Most of the examples here would go away if we ditch the HTML formatter.

@joshuarrrr joshuarrrr added security feature refactor Tech debt related tasks that need refactoring technical debt If not paid, jeapardizes long-term success and maintainability of the repository. labels Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Tech debt related tasks that need refactoring security feature technical debt If not paid, jeapardizes long-term success and maintainability of the repository.
Projects
None yet
Development

No branches or pull requests

2 participants