From 65eccc6ee48643872650bcce7fd3f68700515065 Mon Sep 17 00:00:00 2001 From: Steph Ango <10565871+kepano@users.noreply.github.com> Date: Tue, 23 Jun 2026 18:56:47 +0000 Subject: [PATCH 1/2] Sanitize site-extractor HTML output (GHSA-jg4p-g6xj-4qmf) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/defuddle.ts | 27 ++++++++++++++-- src/extractors/substack.ts | 6 ++-- src/extractors/x-article.ts | 4 +-- src/extractors/youtube.ts | 2 +- tests/extractor-xss.test.ts | 63 +++++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 tests/extractor-xss.test.ts diff --git a/src/defuddle.ts b/src/defuddle.ts index b26ec4bc2..122245ebb 100644 --- a/src/defuddle.ts +++ b/src/defuddle.ts @@ -1678,7 +1678,7 @@ export class Defuddle { extractor: BaseExtractor, pageMetaTags: MetaTagItem[] ): DefuddleResponse { - const contentHtml = this.resolveContentUrls(extracted.contentHtml); + const contentHtml = this._sanitizeExtractorHtml(extracted.contentHtml); const variables = this.getExtractorVariables(extracted.variables); return { content: contentHtml, @@ -1692,7 +1692,7 @@ export class Defuddle { author: extracted.variables?.author || metadata.author, site: extracted.variables?.site || metadata.site, schemaOrgData: metadata.schemaOrgData, - wordCount: this.countHtmlWords(extracted.contentHtml), + wordCount: this.countHtmlWords(contentHtml), parseTime: Math.round(Date.now() - startTime), extractorType: extractor.constructor.name.replace('Extractor', '').toLowerCase(), metaTags: pageMetaTags, @@ -1700,6 +1700,29 @@ export class Defuddle { }; } + /** + * Sanitize and finalize HTML produced by site extractors. + * + * Extractors build their output from template-literal strings, so unlike the + * main pipeline their output never passes through the DOM-based attribute + * sanitizer. Attacker-controlled attribute values (e.g. an image `alt` or + * `src` read straight off the page) could otherwise close an attribute and + * inject an event handler or a `javascript:` URL. Parsing the output into a + * DOM and running the same `_stripUnsafeElements` pass used elsewhere + * neutralizes any such injection regardless of which extractor produced it. + * + * Relative-URL resolution runs on the same parsed DOM so extractor output is + * parsed and serialized only once (resolveRelativeUrls no-ops without a URL). + */ + private _sanitizeExtractorHtml(html: string): string { + if (!html) return html; + const container = this.doc.createElement('div'); + container.appendChild(parseHTML(this.doc, html)); + this._stripUnsafeElements(container); + this.resolveRelativeUrls(container); + return serializeHTML(container); + } + /** * Filter extractor variables to only include custom ones * (exclude standard fields that are already mapped to top-level properties) diff --git a/src/extractors/substack.ts b/src/extractors/substack.ts index 8d4f82be0..0353df070 100644 --- a/src/extractors/substack.ts +++ b/src/extractors/substack.ts @@ -1,6 +1,6 @@ import { BaseExtractor } from './_base'; import { ExtractorResult } from '../types/extractors'; -import { parseHTML } from '../utils/dom'; +import { parseHTML, escapeHtml } from '../utils/dom'; const INJECTED_ATTR = 'data-defuddle-substack-post'; @@ -178,12 +178,12 @@ export class SubstackExtractor extends BaseExtractor { if (!this.noteImage) return ''; const ogImage = this.document.querySelector('meta[property="og:image"]')?.getAttribute('content'); - if (ogImage) return ``; + if (ogImage) return ``; const img = this.noteImage.querySelector('img'); if (!img) return ''; const src = this.getLargestSrc(img); - return src ? `` : ''; + return src ? `` : ''; } private getLargestSrc(img: Element): string { diff --git a/src/extractors/x-article.ts b/src/extractors/x-article.ts index 0bb0ad879..f0d836345 100644 --- a/src/extractors/x-article.ts +++ b/src/extractors/x-article.ts @@ -1,6 +1,6 @@ import { BaseExtractor } from './_base'; import { ExtractorResult } from '../types/extractors'; -import { serializeHTML } from '../utils/dom'; +import { serializeHTML, escapeHtml } from '../utils/dom'; const SELECTORS = { ARTICLE_READ_VIEW: '[data-testid="twitterArticleReadView"]', @@ -111,7 +111,7 @@ export class XArticleExtractor extends BaseExtractor { const alt = headerPhoto.getAttribute('alt')?.replace(/\s+/g, ' ').trim() || 'Image'; - return `${alt}`; + return `${escapeHtml(alt)}`; } private cleanContent(container: HTMLElement): void { diff --git a/src/extractors/youtube.ts b/src/extractors/youtube.ts index 16206c84c..b0c73d341 100644 --- a/src/extractors/youtube.ts +++ b/src/extractors/youtube.ts @@ -366,7 +366,7 @@ export class YoutubeExtractor extends BaseExtractor { } private formatDescription(description: string): string { - return `

${description.replace(/\n/g, '
')}

`; + return `

${escapeHtml(description).replace(/\n/g, '
')}

`; } private getVideoData(): any { diff --git a/tests/extractor-xss.test.ts b/tests/extractor-xss.test.ts new file mode 100644 index 000000000..7177b9902 --- /dev/null +++ b/tests/extractor-xss.test.ts @@ -0,0 +1,63 @@ +import { describe, test, expect } from 'vitest'; +import { Defuddle } from '../src/node'; +import { parseLinkedomHTML } from '../src/utils/linkedom-compat'; + +// Regression test for GHSA-jg4p-g6xj-4qmf: XSS via unescaped attribute +// interpolation in site extractors. Extractor output is built from template +// strings and previously bypassed DOM-based sanitization, so attacker- +// controlled attribute values (e.g. an image alt read off the page) could +// close the attribute and inject an event handler or a javascript: URL. + +const X_URL = 'https://x.com/testuser/article/123456789'; + +// Header image lives in the read view but OUTSIDE the article container, so +// extractHeaderImage() emits it via a template string. +function makeXArticleHTML(headerImgAttrs: string): string { + return ` + Test Article + +
+
+
+

Test Article

+
Body text
+
+
+ + `; +} + +// Re-parse the extractor output and assert no element carries an executable +// attribute. This is the true security property: an escaped "onerror" living +// inside an alt value is harmless; a real onerror attribute is not. +function assertNoExecutableAttributes(html: string) { + const doc = parseLinkedomHTML(`${html}`, X_URL); + for (const el of Array.from(doc.querySelectorAll('*'))) { + for (const attr of Array.from((el as Element).attributes)) { + expect(attr.name.toLowerCase().startsWith('on')).toBe(false); + if (['src', 'href'].includes(attr.name.toLowerCase())) { + expect(attr.value.toLowerCase().replace(/\s+/g, '')).not.toContain('javascript:'); + } + } + } +} + +describe('Extractor output XSS sanitization (GHSA-jg4p-g6xj-4qmf)', () => { + test('does not emit an event handler attribute from X header image alt', async () => { + // alt contains a double-quote that, unescaped, would close the attribute + // and turn the rest into a real onerror handler. + const html = makeXArticleHTML('src="https://example.com/img.jpg" alt=\'x" onerror="alert(1)\''); + const doc = parseLinkedomHTML(html, X_URL); + const response = await Defuddle(doc, X_URL); + + assertNoExecutableAttributes(response.content); + }); + + test('strips javascript: URL injected via X header image src', async () => { + const html = makeXArticleHTML('src="javascript:alert(1)" alt="ok"'); + const doc = parseLinkedomHTML(html, X_URL); + const response = await Defuddle(doc, X_URL); + + assertNoExecutableAttributes(response.content); + }); +}); From 31a7e6f411ef34dee02e5e84b83fbf016af4904f Mon Sep 17 00:00:00 2001 From: Steph Ango <10565871+kepano@users.noreply.github.com> Date: Tue, 23 Jun 2026 22:34:33 +0000 Subject: [PATCH 2/2] Update CLAUDE.md --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index 6e6bfba10..723e14f77 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -95,6 +95,7 @@ Debug mode preserves class/id/data-* attributes and skips div flattening. Use `c - **Never use `innerHTML` directly.** Always use `parseHTML()` from `src/utils/dom.ts` which parses via `