Resolves HtmlSanitizer.js XSS Security Vulnerability #19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
XSS vulnerability when the sanitizer is used with a `contentEditable` element to set the elements `innerHTML` to a sanitized string produced by the package. If the code is particularly crafted to abuse the code beautifier, that runs AFTER sanitation. The key change I made is to eliminate the post-sanitization string manipulation that was causing the vulnerability. Instead of using a simple regex replacement for beautification, I've modified the code to: 1. Get the sanitized HTML as a string 2. Parse it again using DOMParser (which is already used in the code) 3. Return the innerHTML of the body element This approach ensures that any output formatting is done through the DOM parser, which will properly handle the HTML structure and prevent XSS attacks that could exploit the regex-based beautification. The vulnerability existed because the regex replacement was happening after the sanitization process was complete. If an attacker crafted specific HTML that looked harmless after sanitization but would become malicious after the regex replacement, they could potentially execute arbitrary JavaScript when the sanitized content was inserted into a contentEditable element.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
XSS vulnerability when the sanitizer is used with a
contentEditableelement to set the elementsinnerHTMLto a sanitized string produced by the package. If the code is particularly crafted to abuse the code beautifier, that runs AFTER sanitation.The key change I made is to eliminate the post-sanitization string manipulation that was causing the vulnerability. Instead of using a simple regex replacement for beautification, I've modified the code to:
This approach ensures that any output formatting is done through the DOM parser, which will properly handle the HTML structure and prevent XSS attacks that could exploit the regex-based beautification.
The vulnerability existed because the regex replacement was happening after the sanitization process was complete. If an attacker crafted specific HTML that looked harmless after sanitization but would become malicious after the regex replacement, they could potentially execute arbitrary JavaScript when the sanitized content was inserted into a contentEditable element.