diff --git a/actions/setup/js/sanitize_content.test.cjs b/actions/setup/js/sanitize_content.test.cjs index 2e2116c0fe..f9ecdcd97f 100644 --- a/actions/setup/js/sanitize_content.test.cjs +++ b/actions/setup/js/sanitize_content.test.cjs @@ -484,6 +484,88 @@ describe("sanitize_content.cjs", () => { }); }); + describe("XML/HTML tag conversion: dangerous attribute stripping", () => { + it("should strip ontoggle event handler from details tag", () => { + const input = '
content
'; + const result = sanitizeContent(input); + expect(result).toBe("
content
"); + }); + + it("should strip style attribute enabling CSS overlay attacks from span tag", () => { + const input = 'overlay'; + const result = sanitizeContent(input); + expect(result).toBe("overlay"); + }); + + it("should strip onclick event handler from allowed tags", () => { + const result = sanitizeContent('

text

'); + expect(result).toBe("

text

"); + }); + + it("should strip onerror event handler from allowed tags", () => { + const result = sanitizeContent('text'); + expect(result).toBe("text"); + }); + + it("should strip style attribute with single-quoted value", () => { + const result = sanitizeContent("text"); + expect(result).toBe("text"); + }); + + it("should strip style attribute with unquoted value", () => { + const result = sanitizeContent("text"); + expect(result).toBe("text"); + }); + + it("should strip style attribute with unquoted value (simple, no special chars)", () => { + const result = sanitizeContent("text"); + expect(result).toBe("text"); + }); + + it("should strip on* attributes case-insensitively (uppercase)", () => { + const result = sanitizeContent('text'); + expect(result).toBe("text"); + }); + + it("should strip multiple dangerous attributes from a single tag", () => { + const result = sanitizeContent('text'); + expect(result).toBe('text'); + }); + + it("should preserve safe attributes (title, class, open) while stripping dangerous ones", () => { + const result = sanitizeContent('
content
'); + expect(result).toBe("
content
"); + }); + + it("should preserve span title attribute after stripping style", () => { + const result = sanitizeContent('text'); + expect(result).toBe('text'); + }); + + it("should preserve closing tags of allowed elements unchanged", () => { + // Closing tags do not carry attributes in HTML; verify they pass through unmodified + const result = sanitizeContent("text"); + expect(result).toBe("text"); + }); + + it("should strip on* attribute with extra whitespace around equals sign", () => { + const result = sanitizeContent('text'); + expect(result).toBe("text"); + }); + + it("should treat concatenated tag+attribute name as a single unknown tag (not strip attributes)", () => { + // is not a valid tag — the tag name is "spanonclick" + // which is not in the allowlist, so it gets converted to parentheses entirely + const result = sanitizeContent('text'); + expect(result).toBe('(spanonclick="bad()")text(/spanonclick)'); + }); + + it("should not affect disallowed tags (still converted to parentheses with attributes)", () => { + const result = sanitizeContent('
content
'); + expect(result).toBe('(div onclick="bad()")content(/div)'); + }); + }); + describe("XML/HTML tag conversion: code-region awareness", () => { it("should preserve angle brackets inside fenced code blocks (backticks)", () => { const input = "Before\n```\nVBuffer x;\n```\nAfter"; diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index 4e69b11f27..2081431330 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -568,17 +568,41 @@ function convertXmlTags(s) { return `(![CDATA[${convertedContent}]])`; }); + /** + * Strips dangerous HTML attributes from an allowed tag's content string. + * Removes on* event handler attributes (e.g. onclick, ontoggle) and style + * attributes in all quoting forms (double-quoted, single-quoted, unquoted, bare). + * Safe attributes such as title, class, open, lang, id, etc. are preserved. + * + * Note: `\s+` (requiring at least one whitespace before the attribute name) is + * intentional — HTML attributes are always separated from the tag name and from + * each other by at least one whitespace character. Using `\s*` would risk false + * matches inside tag names (e.g. matching "ong" inside "strong"). + * + * @param {string} tagContent - Tag content without surrounding angle brackets + * @returns {string} Tag content with dangerous attributes removed + */ + function stripDangerousAttributes(tagContent) { + // Match: one-or-more whitespace + (on* | style) + optional =value + // Value forms: "...", '...', or unquoted (no whitespace / > / quote chars), or bare (no =) + // The unquoted form excludes >, whitespace, and all quote characters (', ", `) so it + // cannot consume the closing > of the tag or straddle other attribute values. + return tagContent.replace(/\s+(?:on\w+|style)(?:\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>"'`]*))?/gi, ""); + } + // Convert opening tags: or to (tag) or (tag attr="value") // Convert closing tags: to (/tag) // Convert self-closing tags: or to (tag/) or (tag /) - // But preserve allowed safe tags + // But preserve allowed safe tags (with dangerous attributes stripped) return s.replace(/<(\/?[A-Za-z!][^>]*?)>/g, (match, tagContent) => { // Extract tag name from the content (handle closing tags and attributes) const tagNameMatch = tagContent.match(/^\/?\s*([A-Za-z][A-Za-z0-9]*)/); if (tagNameMatch) { const tagName = tagNameMatch[1].toLowerCase(); if (allowedTags.includes(tagName)) { - return match; // Preserve allowed tags + // Strip dangerous attributes (on* event handlers and style) before preserving + const sanitizedContent = stripDangerousAttributes(tagContent); + return `<${sanitizedContent}>`; } } return `(${tagContent})`; // Convert other tags to parentheses