fix: escape HTML in heading ID and code info attributes to prevent XSS - #723
Open
Yanhu007 wants to merge 1 commit into
Open
fix: escape HTML in heading ID and code info attributes to prevent XSS#723Yanhu007 wants to merge 1 commit into
Yanhu007 wants to merge 1 commit into
Conversation
Heading IDs from {#id} syntax and fenced code block info strings
were interpolated into HTML attributes without escaping. This allows
stored XSS that fires automatically on page load:
# Test {#"><img src=x onerror=alert(1)>}
→ <h1 id=""><img src=x onerror=alert(1)>">Test</h1>
```lang"><img src=x onerror=alert(1)>
→ <code class="language-lang"><img src=x ...>
Both are enabled by default via CommonExtensions and bypass SkipHTML
since the injection targets are generated by the renderer, not
preserved from the input.
Fix: apply html.EscapeString() to heading IDs and code info strings
before interpolation into attribute values.
Ref russross#722
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
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.
Security Fix
Addresses #722 — Critical XSS via unescaped attribute injection
Vulnerabilities
Finding 1: Heading ID injection (Critical)
# Test {#"><img src=x onerror=alert(1)>}Renders as:
Finding 2: Fenced code info string injection (Critical)
Renders as:
Both fire automatically on page load, bypass
SkipHTML, and are enabled by default viaCommonExtensions.Fix
Apply
html.EscapeString()to:id=""attribute (line 679)class=""attribute (line 330)After Fix
All existing tests pass.