Sanitize extractor HTML#326
Merged
Merged
Conversation
Site extractors build their output from template-literal strings, interpolating attacker-controlled DOM attribute values (image alt/src, og:image, video description) without escaping. That output is returned through buildExtractorResponse() which previously only resolved URLs, bypassing the DOM-based sanitization the main pipeline applies — so an injected event handler or javascript: URL could reach consumers that render contentHtml. Fix, defense in depth: - Route all extractor output through _sanitizeExtractorHtml(), which parses into a DOM and runs the same _stripUnsafeElements() pass used elsewhere (and resolves relative URLs in the same pass). This covers every extractor, present and future. - escapeHtml() the interpolated values at the confirmed sinks in x-article, substack, and youtube extractors. This is complementary, not redundant: it also blocks injection of attributes the central pass does not strip (style/class/etc.) and preserves data integrity. Add a regression test that re-parses extractor output and asserts no event-handler attributes or javascript: URLs survive.
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.
Fixes a stored XSS in site extractors. Several extractors build their output by interpolating attacker-controlled DOM attribute values into template-literal HTML strings without escaping:
An
alt(orsrc,og:image, video description) containing a"closes the attribute and the remainder becomes new attributes — e.g.alt='x" onerror="alert(1)'yields a liveonerrorhandler. This output is returned throughbuildExtractorResponse(), which previously only resolved URLs and bypassed the DOM-based sanitization the main pipeline applies.Fix
_sanitizeExtractorHtml(), which parses into a DOM and runs the same_stripUnsafeElements()pass used elsewhere (and resolves relative URLs in the same parse, so output is parsed/serialized once). This covers every extractor, present and future.escapeHtml()wraps the interpolated values in the three confirmed extractors. This is complementary, not redundant: the central pass stripson*/dangerous URLs but not injectedstyle/classattributes, and escaping preserves the original text's integrity.