From aeb96018d7187d1bd475c93f5c8ae6e79146d515 Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:24:14 +0300 Subject: [PATCH] Allowlist markdown tags --- app/components/Markdown.test.tsx | 123 +++++++++++++++++ app/components/Markdown.tsx | 43 +++--- app/utils/markdown-html.test.ts | 156 +++++++++++++++++++++ app/utils/markdown-html.ts | 224 +++++++++++++++++++++++++++++++ 4 files changed, 531 insertions(+), 15 deletions(-) create mode 100644 app/components/Markdown.test.tsx create mode 100644 app/utils/markdown-html.test.ts create mode 100644 app/utils/markdown-html.ts diff --git a/app/components/Markdown.test.tsx b/app/components/Markdown.test.tsx new file mode 100644 index 000000000..49756c52e --- /dev/null +++ b/app/components/Markdown.test.tsx @@ -0,0 +1,123 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, test } from "vitest"; +import { Markdown } from "./Markdown"; + +function render(markdown: string) { + return renderToStaticMarkup({markdown}); +} + +describe("Markdown", () => { + test.each([ + { + why: "meta refresh redirect", + markdown: `hi there`, + forbidden: "[x](/relative)`, + forbidden: "`, + forbidden: "`, + forbidden: "`, + forbidden: "`, + forbidden: "`, + forbidden: "x`, + forbidden: "c`, + forbidden: "onclick", + }, + ])("does not render $why", ({ markdown, forbidden }) => { + expect(render(markdown)).not.toContain(forbidden); + }); + + test("renders inline svg gradients authored by users", () => { + const html = render( + ``, + ); + + expect(html).toContain(''); + expect(html).toContain( + ' { + const html = render( + `a`, + ); + + expect(html).toContain( + `a`, + ); + expect(html).not.toContain("unsafe-url"); + }); + + test.each([ + { + why: "the text content of an unknown element", + markdown: `hi`, + expected: `hi`, + }, + { + why: "a prototype-named element as text", + markdown: `text`, + expected: "text", + }, + { + why: "video with sources", + markdown: ``, + expected: ``, + }, + { + why: "task lists", + markdown: "- [ ] todo\n- [x] done", + expected: `
  • todo
  • done
`, + }, + { + why: "standard markdown", + markdown: + "## Rules\n\n**bold** [link](https://x)\n\n```js\nconst a = 1;\n```", + expected: `

Rules

bold link

const a = 1;
`, + }, + { + why: "new-tab links with a forced rel", + markdown: `x`, + expected: `x`, + }, + { + why: "inline styles without url()", + markdown: `a`, + expected: `a`, + }, + ])("renders $why", ({ markdown, expected }) => { + expect(render(markdown)).toBe(expected); + }); +}); diff --git a/app/components/Markdown.tsx b/app/components/Markdown.tsx index 8965b261d..1b12887a1 100644 --- a/app/components/Markdown.tsx +++ b/app/components/Markdown.tsx @@ -1,5 +1,6 @@ import MarkdownToJsx from "markdown-to-jsx"; import * as React from "react"; +import * as MarkdownHtml from "~/utils/markdown-html"; // note: markdown-to-jsx also handles these, this is just to prevent them from appearing as plain text const DANGEROUS_HTML_TAGS_REGEX = @@ -20,24 +21,36 @@ export function Markdown({ children }: { children: string }) {
}, - hr: { component: () =>
}, - img: { - component: ({ - children: _, - ...props - }: React.ComponentProps<"img"> & { - children?: React.ReactNode; - }) => ( - // biome-ignore lint/a11y/useAltText: parsed markdown, so we can't guarantee alt text is present - - ), - }, - }, + createElement: createAllowlistedElement, }} > {sanitized}
); } + +function createAllowlistedElement( + tag: Parameters[0], + props: React.JSX.IntrinsicAttributes, + ...children: React.ReactNode[] +) { + if (typeof tag !== "string") { + return React.createElement(tag, props, ...children); + } + + const element = MarkdownHtml.sanitizeElement( + tag, + (props ?? {}) as Record, + ); + if (!element) { + return React.createElement( + React.Fragment, + { key: props?.key }, + ...children, + ); + } + + return element.isVoid + ? React.createElement(element.tag, element.props) + : React.createElement(element.tag, element.props, ...children); +} diff --git a/app/utils/markdown-html.test.ts b/app/utils/markdown-html.test.ts new file mode 100644 index 000000000..c2a1eb6ec --- /dev/null +++ b/app/utils/markdown-html.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, test } from "vitest"; +import * as MarkdownHtml from "./markdown-html"; + +describe("MarkdownHtml.sanitizeElement", () => { + test.each([ + { why: "meta", tag: "meta", props: { httpEquiv: "refresh" } }, + { why: "base", tag: "base", props: { href: "https://evil.example/" } }, + { why: "link", tag: "link", props: { rel: "stylesheet", href: "x.css" } }, + { why: "iframe", tag: "iframe", props: { src: "https://evil.example" } }, + { why: "object", tag: "object", props: { data: "https://evil.example" } }, + { why: "embed", tag: "embed", props: { src: "https://evil.example" } }, + { why: "form", tag: "form", props: { action: "https://evil.example" } }, + { why: "script", tag: "script", props: {} }, + { why: "style", tag: "style", props: {} }, + { why: "textarea", tag: "textarea", props: {} }, + { why: "title", tag: "title", props: {} }, + { why: "xmp", tag: "xmp", props: {} }, + { why: "noembed", tag: "noembed", props: {} }, + { why: "noframes", tag: "noframes", props: {} }, + { why: "plaintext", tag: "plaintext", props: {} }, + { why: "head", tag: "head", props: {} }, + { why: "svg use", tag: "use", props: { href: "#x" } }, + { why: "svg set", tag: "set", props: { attributeName: "href" } }, + { why: "svg animate", tag: "animate", props: { attributeName: "href" } }, + { why: "svg image", tag: "image", props: { href: "https://x/a.png" } }, + { why: "svg foreignObject", tag: "foreignObject", props: {} }, + { why: "unknown custom tag", tag: "TextType", props: { align: "center" } }, + { why: "constructor", tag: "constructor", props: { href: "x" } }, + { why: "toString", tag: "toString", props: { href: "x" } }, + { why: "hasOwnProperty", tag: "hasOwnProperty", props: { href: "x" } }, + { why: "__proto__", tag: "__proto__", props: { href: "x" } }, + { why: "non-checkbox input", tag: "input", props: { type: "text" } }, + ])("refuses $why", ({ tag, props }) => { + expect(MarkdownHtml.sanitizeElement(tag, props)).toBeNull(); + }); + + test.each([ + { + why: "event handlers", + tag: "div", + props: { onClick: "alert(1)", onerror: "alert(1)", style: "color:red" }, + expected: { style: "color:red" }, + }, + { + why: "data attributes", + tag: "div", + props: { "data-testid": "x", className: "box" }, + expected: { className: "box" }, + }, + { + why: "id outside headings and svg", + tag: "div", + props: { id: "clobber" }, + expected: {}, + }, + { + why: "user-supplied referrer policy on images", + tag: "img", + props: { src: "a.png", referrerpolicy: "unsafe-url", alt: "" }, + expected: { src: "a.png", alt: "", referrerPolicy: "no-referrer" }, + }, + { + why: "authored rel on new-tab links", + tag: "a", + props: { href: "https://x", target: "_blank", rel: "opener" }, + expected: { + href: "https://x", + target: "_blank", + rel: "noopener noreferrer", + }, + }, + { + why: "empty class name", + tag: "code", + props: { className: "" }, + expected: {}, + }, + { + why: "unlisted anchor attributes", + tag: "a", + props: { href: "https://x", download: "x", ping: "y" }, + expected: { href: "https://x" }, + }, + { + why: "authored rel on same-tab links", + tag: "a", + props: { href: "https://x", rel: "opener" }, + expected: { href: "https://x" }, + }, + ])("drops $why", ({ tag, props, expected }) => { + expect(MarkdownHtml.sanitizeElement(tag, props)?.props).toEqual(expected); + }); + + test.each([ + { + why: "heading ids generated for anchors", + tag: "h2", + props: { id: "rules", key: 3 }, + expected: { id: "rules", key: 3 }, + }, + { + why: "hyphenated svg attributes", + tag: "stop", + props: { offset: "0", "stop-color": "#fff" }, + expected: { offset: "0", "stop-color": "#fff" }, + }, + { + why: "camel-cased svg tags and attributes", + tag: "linearGradient", + props: { + id: "g", + gradientUnits: "userSpaceOnUse", + spreadMethod: "repeat", + }, + expected: { + id: "g", + gradientUnits: "userSpaceOnUse", + spreadMethod: "repeat", + }, + }, + { + why: "task list checkboxes", + tag: "input", + props: { type: "checkbox", checked: true, readOnly: true }, + expected: { type: "checkbox", checked: true, readOnly: true }, + }, + { + why: "legacy font colors", + tag: "font", + props: { color: "#B8860B" }, + expected: { color: "#B8860B" }, + }, + { + why: "video sources", + tag: "source", + props: { src: "https://x/a.mp4", type: "video/mp4" }, + expected: { src: "https://x/a.mp4", type: "video/mp4" }, + }, + ])("keeps $why", ({ tag, props, expected }) => { + expect(MarkdownHtml.sanitizeElement(tag, props)?.props).toEqual(expected); + }); + + test("preserves the original tag casing so React can render svg elements", () => { + expect(MarkdownHtml.sanitizeElement("linearGradient", {})?.tag).toBe( + "linearGradient", + ); + }); + + test.each([ + { tag: "br", isVoid: true }, + { tag: "img", isVoid: true }, + { tag: "div", isVoid: false }, + ])("marks $tag void: $isVoid", ({ tag, isVoid }) => { + expect(MarkdownHtml.sanitizeElement(tag, {})?.isVoid).toBe(isVoid); + }); +}); diff --git a/app/utils/markdown-html.ts b/app/utils/markdown-html.ts new file mode 100644 index 000000000..799b58c74 --- /dev/null +++ b/app/utils/markdown-html.ts @@ -0,0 +1,224 @@ +const GLOBAL_ATTRIBUTES = attributes( + "align", + "classname", + "dir", + "height", + "style", + "title", + "width", +); + +const HEADING_ATTRIBUTES = attributes("id"); + +const TABLE_CELL_ATTRIBUTES = attributes("colspan", "rowspan", "valign"); + +const SVG_PRESENTATION_ATTRIBUTES = attributes( + "fill", + "fillopacity", + "fillrule", + "opacity", + "stroke", + "strokedasharray", + "strokelinecap", + "strokelinejoin", + "strokeopacity", + "strokewidth", + "transform", +); + +const SVG_GRADIENT_ATTRIBUTES = attributes( + "id", + "gradientunits", + "gradienttransform", + "spreadmethod", + ...SVG_PRESENTATION_ATTRIBUTES, +); + +/** Elements user markdown may render, mapped to the attributes allowed on them (lower-cased, hyphens removed) in addition to {@link GLOBAL_ATTRIBUTES}. Anything else is unwrapped so only its text content survives. */ +const ALLOWED_ELEMENTS: Record> = { + a: attributes("href", "target"), + abbr: attributes(), + b: attributes(), + blockquote: attributes(), + br: attributes(), + center: attributes(), + code: attributes(), + dd: attributes(), + del: attributes(), + details: attributes("open"), + div: attributes(), + dl: attributes(), + dt: attributes(), + em: attributes(), + figcaption: attributes(), + figure: attributes(), + font: attributes("color", "face", "size"), + h1: HEADING_ATTRIBUTES, + h2: HEADING_ATTRIBUTES, + h3: HEADING_ATTRIBUTES, + h4: HEADING_ATTRIBUTES, + h5: HEADING_ATTRIBUTES, + h6: HEADING_ATTRIBUTES, + hr: attributes(), + i: attributes(), + img: attributes("src", "alt", "loading", "border"), + input: attributes("type", "checked", "readonly", "disabled"), + ins: attributes(), + kbd: attributes(), + li: attributes("value"), + mark: attributes(), + ol: attributes("start", "type", "reversed"), + p: attributes(), + picture: attributes(), + pre: attributes(), + s: attributes(), + small: attributes(), + source: attributes("src", "srcset", "type", "media"), + span: attributes(), + strike: attributes(), + strong: attributes(), + sub: attributes(), + summary: attributes(), + sup: attributes(), + table: attributes("border", "cellpadding", "cellspacing"), + tbody: attributes(), + td: TABLE_CELL_ATTRIBUTES, + tfoot: attributes(), + th: attributes("scope", ...TABLE_CELL_ATTRIBUTES), + thead: attributes(), + tr: attributes("valign"), + u: attributes(), + ul: attributes("start"), + video: attributes( + "src", + "poster", + "controls", + "autoplay", + "loop", + "muted", + "playsinline", + "preload", + ), + svg: attributes( + "xmlns", + "viewbox", + "preserveaspectratio", + ...SVG_PRESENTATION_ATTRIBUTES, + ), + g: SVG_PRESENTATION_ATTRIBUTES, + defs: attributes(), + path: attributes("d", ...SVG_PRESENTATION_ATTRIBUTES), + rect: attributes("x", "y", "rx", "ry", ...SVG_PRESENTATION_ATTRIBUTES), + circle: attributes("cx", "cy", "r", ...SVG_PRESENTATION_ATTRIBUTES), + ellipse: attributes("cx", "cy", "rx", "ry", ...SVG_PRESENTATION_ATTRIBUTES), + line: attributes("x1", "y1", "x2", "y2", ...SVG_PRESENTATION_ATTRIBUTES), + polyline: attributes("points", ...SVG_PRESENTATION_ATTRIBUTES), + polygon: attributes("points", ...SVG_PRESENTATION_ATTRIBUTES), + text: attributes( + "x", + "y", + "dx", + "dy", + "textanchor", + "fontsize", + ...SVG_PRESENTATION_ATTRIBUTES, + ), + lineargradient: attributes( + "x1", + "y1", + "x2", + "y2", + ...SVG_GRADIENT_ATTRIBUTES, + ), + radialgradient: attributes( + "cx", + "cy", + "r", + "fx", + "fy", + ...SVG_GRADIENT_ATTRIBUTES, + ), + stop: attributes("offset", "stopcolor", "stopopacity"), + animatetransform: attributes( + "attributename", + "type", + "from", + "to", + "by", + "values", + "dur", + "begin", + "repeatcount", + "additive", + "accumulate", + "fill", + ), +}; + +const VOID_ELEMENTS = new Set(["br", "hr", "img", "input", "source"]); + +const NEW_TAB_LINK_REL = "noopener noreferrer"; + +export interface SanitizedElement { + tag: string; + props: Record; + /** Void elements must be created without children or React warns */ + isVoid: boolean; +} + +/** + * Filters an HTML element authored inside user markdown down to the allowlist. + * Returns `null` when the element itself may not render; the caller should then render only its children. + * + * @example + * MarkdownHtml.sanitizeElement("img", { src: "a.png", onerror: "x()" }) + * // -> { tag: "img", props: { src: "a.png", referrerPolicy: "no-referrer" }, isVoid: true } + * MarkdownHtml.sanitizeElement("a", { href: "https://x", target: "_blank", rel: "opener" }) + * // -> { tag: "a", props: { href: "https://x", target: "_blank", rel: "noopener noreferrer" }, isVoid: false } + * MarkdownHtml.sanitizeElement("meta", { httpEquiv: "refresh" }) + * // -> null + */ +export function sanitizeElement( + tag: string, + props: Record, +): SanitizedElement | null { + const tagName = tag.toLowerCase(); + if (!Object.hasOwn(ALLOWED_ELEMENTS, tagName)) return null; + const allowedAttributes = ALLOWED_ELEMENTS[tagName]; + if (tagName === "input" && props.type !== "checkbox") return null; + + const sanitizedProps: Record = {}; + for (const [key, value] of Object.entries(props)) { + if (key === "key") { + sanitizedProps.key = value; + continue; + } + + const normalizedKey = normalizeAttributeName(key); + if (normalizedKey === "classname" && !value) continue; + if ( + GLOBAL_ATTRIBUTES.has(normalizedKey) || + allowedAttributes.has(normalizedKey) + ) { + sanitizedProps[key] = value; + } + } + + if (tagName === "img") { + sanitizedProps.referrerPolicy = "no-referrer"; + } + if (tagName === "a" && sanitizedProps.target) { + sanitizedProps.rel = NEW_TAB_LINK_REL; + } + + return { tag, props: sanitizedProps, isVoid: VOID_ELEMENTS.has(tagName) }; +} + +function attributes(...names: string[]) { + return new Set(names); +} + +/** `className`, `class-name` and `CLASSNAME` all become `classname` so the allowlist is spelled once */ +function normalizeAttributeName(name: string) { + return name.toLowerCase().replaceAll("-", ""); +}