mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-11-15 13:58:20 +00:00
fix(media-use): replace regex SVG sanitizer with a DOM allowlist
Two rounds of regex/tokenizer patches to sanitizeSvg found new bypasses in the same shape each time (unquoted href, style= url(), aliased xlink prefix; then presentation-attribute url(), unquoted '=' handling, SMIL animate/set href rewrite, self-closing-slash tag corruption, data:image/ navigation via <a>). Denylisting attribute names against a hand-rolled tokenizer was the wrong approach regardless of how careful the regex. Rewrote sanitizeSvg (packages/core/src/figma/sanitizeSvg.ts) as a real DOM walk (linkedom, already a dependency of core and cli) with an allowlist: known-safe elements and attributes only, everything else dropped by construction rather than pattern-matched as dangerous. href/xlink:href (any namespace prefix) kept only for a same-document fragment, or data:image/ off non-<a> elements. Any attribute value or <style> text containing url() kept only when every url() argument is a fragment; @import/expression(/javascript: dropped from style content. Serializing from a real DOM means output is always well-formed, so the self-closing-slash corruption class of bug is gone by construction. media-use's own copy (packages/cli/src/media-use/lib/svg-sanitize.mjs) can't import linkedom directly: its CI test lane runs bare `node --test` with no install step, so even an external dependency fails to resolve, not just a workspace import. It's now a generated file instead of a hand-copy: scripts/generate-media-use-svg-sanitize.mjs bundles the core source (esbuild, minified, zero remaining imports) and scripts/check-media-use-svg-sanitize-generated.test.mjs fails the build on drift. Verified the generated file runs with plain node in an empty directory with no node_modules on the path. Every payload from both independent-review reports is now a table-driven test in sanitizeSvg.test.ts (46 cases) and the same corpus in check-svg-sanitize-parity.test.mjs (39 cases) asserts core and the generated media-use copy agree and each independently rejects every non-local url(), javascript:, data: href on <a>, and on* handler. Mutation-proved against the real pre-fix blob, not a synthetic case. Sampled 56 real theSVG icons across all 7 collections to check the allowlist doesn't over-strip legitimate content: found and fixed three missing attributes (fx/fy, enable-background, a blanket data-* allowance) and one real behavioral bug - <style> elements were being dropped outright, silently unstyling any Illustrator-exported logo that colors paths via CSS classes. Fixed by keeping <style> on the allowlist and content-checking its text the same way as an inline style= attribute, with a regression test using that exact shape. Full suite after the fix: sanitizeSvg.test.ts 46/46, parity+drift tests 39/39, media-use suite 298/298, tsc/oxlint/oxfmt clean.
This commit is contained in:
@@ -0,0 +1,24 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { resolve } from "node:path";
|
||||
import { describe, it } from "node:test";
|
||||
import { generate } from "./generate-media-use-svg-sanitize.mjs";
|
||||
|
||||
// The single source of truth for SVG sanitization is packages/core/src/figma/sanitizeSvg.ts;
|
||||
// media-use's copy is a generated bundle (its test lane runs bare `node --test`, no install, so
|
||||
// it can't import linkedom directly). This is the trip wire against hand-editing the bundle or
|
||||
// forgetting to regenerate it after touching the source.
|
||||
describe("media-use svg-sanitize.mjs matches a fresh regeneration", () => {
|
||||
it("is byte-identical to `node scripts/generate-media-use-svg-sanitize.mjs`", async () => {
|
||||
const committed = readFileSync(
|
||||
resolve(import.meta.dirname, "../packages/cli/src/media-use/lib/svg-sanitize.mjs"),
|
||||
"utf8",
|
||||
);
|
||||
const fresh = await generate();
|
||||
assert.equal(
|
||||
committed,
|
||||
fresh,
|
||||
"packages/cli/src/media-use/lib/svg-sanitize.mjs is stale — run: node scripts/generate-media-use-svg-sanitize.mjs",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -3,10 +3,9 @@ import { describe, it } from "node:test";
|
||||
import { sanitizeSvg as coreSanitizeSvg } from "../packages/core/src/figma/sanitizeSvg.ts";
|
||||
import { sanitizeSvg as mediaUseSanitizeSvg } from "../packages/cli/src/media-use/lib/svg-sanitize.mjs";
|
||||
|
||||
// media-use/lib/svg-sanitize.mjs is a hand-kept port of
|
||||
// packages/core/src/figma/sanitizeSvg.ts (media-use's test lane forbids
|
||||
// workspace imports). This corpus is the trip wire: if either changes
|
||||
// without the other, one of these cases stops matching.
|
||||
// svg-sanitize.mjs is generated from sanitizeSvg.ts (see check-media-use-svg-sanitize-generated
|
||||
// for the drift check). This is the behavioral trip wire: both copies must independently
|
||||
// neutralize and agree on every payload, so a bad regeneration can't silently reopen a bypass.
|
||||
const HOSTILE_CORPUS = [
|
||||
["<svg><script>alert(1)</script><rect/></svg>", "script element"],
|
||||
['<svg><script src="//evil"/></svg>', "self-closing script"],
|
||||
@@ -18,7 +17,11 @@ const HOSTILE_CORPUS = [
|
||||
["<svg><rect onload=alert(1)/></svg>", "unquoted on* handler"],
|
||||
['<svg><a href="javascript:alert(1)"><rect/></a></svg>', "javascript: href"],
|
||||
['<svg><a xlink:href="javascript:alert(1)"><rect/></a></svg>', "javascript: xlink:href"],
|
||||
['<svg><a href="data:image/png;base64,AA=="><rect/></a></svg>', "allowed data:image/ href"],
|
||||
[
|
||||
'<svg><a href="data:image/png;base64,AA=="><rect/></a></svg>',
|
||||
"data:image/ href on <a> (still navigable, dropped)",
|
||||
],
|
||||
['<svg><use href="data:image/png;base64,AA=="/></svg>', "allowed data:image/ href on non-<a>"],
|
||||
['<svg><a href="#clip"><rect/></a></svg>', "allowed local fragment href"],
|
||||
['<svg><a href="data:text/html,<script>1</script>"><rect/></a></svg>', "data:text/html href"],
|
||||
[
|
||||
@@ -35,7 +38,6 @@ const HOSTILE_CORPUS = [
|
||||
],
|
||||
["<svg><rect onMouseOver=\"alert('x')\" ONCLICK='y'/></svg>", "mixed-case on* handler names"],
|
||||
['<svg><a HREF="javascript:alert(1)"><rect/></a></svg>', "mixed-case href attribute name"],
|
||||
// Reported against b7ace99b4 — see hyperframes-4291-b7ace99b.md BLOCKER 1-3.
|
||||
["<svg><a href=javascript:alert(1)><rect/></a></svg>", "unquoted javascript: href"],
|
||||
["<svg><use href=https://evil.example/x.svg#p/></svg>", "unquoted external use href"],
|
||||
['<svg><rect style="fill:url(https://evil.example/x)"/></svg>', "style url() exfil"],
|
||||
@@ -51,22 +53,62 @@ const HOSTILE_CORPUS = [
|
||||
'<svg xmlns:x="http://www.w3.org/1999/xlink"><use x:href="https://evil.example/x.svg#p"/></svg>',
|
||||
"href via a renamed xlink namespace prefix",
|
||||
],
|
||||
['<svg><rect fill="url(https://evil.example/x.svg#p)"/></svg>', "fill url() exfil"],
|
||||
['<svg><rect filter="url(https://evil.example/x.svg#f)"/></svg>', "filter url() exfil"],
|
||||
['<svg><rect mask="url(https://evil.example/x.svg#m)"/></svg>', "mask url() exfil"],
|
||||
['<svg><rect clip-path="url(https://evil.example/x.svg#c)"/></svg>', "clip-path url() exfil"],
|
||||
[
|
||||
'<svg><path marker-start="url(https://evil.example/x.svg#m)"/></svg>',
|
||||
"marker-start url() exfil",
|
||||
],
|
||||
[
|
||||
"<svg><rect style=x=y;fill:url(https://evil.example/beacon)>r</rect></svg>",
|
||||
"unquoted attribute value containing '='",
|
||||
],
|
||||
[
|
||||
'<svg><a href="#a"><animate attributeName="href" to="javascript:alert(1)"/></a></svg>',
|
||||
"SMIL <animate> rewrites href to javascript:",
|
||||
],
|
||||
[
|
||||
'<svg><a href="#a"><set attributeName="href" to="javascript:alert(1)"/></a></svg>',
|
||||
"SMIL <set> rewrites href to javascript:",
|
||||
],
|
||||
[
|
||||
'<svg><a href="data:image/svg+xml;base64,PHN2Zz48c2NyaXB0PmFsZXJ0KDEpPC9zY3JpcHQ+PC9zdmc+">click</a></svg>',
|
||||
"data:image/svg+xml navigation on <a>",
|
||||
],
|
||||
];
|
||||
|
||||
describe("svg-sanitize parity: core vs media-use port", () => {
|
||||
describe("svg-sanitize parity: core vs media-use (generated) port", () => {
|
||||
for (const [input, label] of HOSTILE_CORPUS) {
|
||||
it(`matches core output for: ${label}`, () => {
|
||||
assert.equal(mediaUseSanitizeSvg(input), coreSanitizeSvg(input));
|
||||
});
|
||||
}
|
||||
|
||||
it("both strip every hostile marker from the full corpus", () => {
|
||||
for (const [input] of HOSTILE_CORPUS) {
|
||||
const out = mediaUseSanitizeSvg(input);
|
||||
assert.ok(!/<script\b/i.test(out), `<script survived: ${input}`);
|
||||
assert.ok(!/<foreignObject\b/i.test(out), `<foreignObject survived: ${input}`);
|
||||
assert.ok(!/\son\w+\s*=/i.test(out), `on* handler survived: ${input}`);
|
||||
assert.ok(!/javascript:/i.test(out), `javascript: href survived: ${input}`);
|
||||
it("both copies independently reject every unsafe marker in the corpus", () => {
|
||||
const failures = [];
|
||||
for (const [input, label] of HOSTILE_CORPUS) {
|
||||
for (const [name, sanitize] of [
|
||||
["core", coreSanitizeSvg],
|
||||
["media-use", mediaUseSanitizeSvg],
|
||||
]) {
|
||||
const out = sanitize(input);
|
||||
if (/<script\b/i.test(out)) failures.push(`${name}/${label}: <script survived`);
|
||||
if (/<foreignobject\b/i.test(out))
|
||||
failures.push(`${name}/${label}: <foreignObject survived`);
|
||||
if (/\son\w+\s*=/i.test(out)) failures.push(`${name}/${label}: on* handler survived`);
|
||||
if (/javascript:/i.test(out)) failures.push(`${name}/${label}: javascript: survived`);
|
||||
if (/data:image/i.test(out) && /<a[\s>]/i.test(out)) {
|
||||
failures.push(`${name}/${label}: data:image href survived on <a>`);
|
||||
}
|
||||
const urlMatches = [...out.matchAll(/url\(\s*(['"]?)([^)]*)\1\s*\)/gi)];
|
||||
for (const m of urlMatches) {
|
||||
if (!m[2].trim().startsWith("#"))
|
||||
failures.push(`${name}/${label}: non-local url(${m[2]}) survived`);
|
||||
}
|
||||
}
|
||||
}
|
||||
assert.deepEqual(failures, []);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
#!/usr/bin/env node
|
||||
// Bundles packages/core/src/figma/sanitizeSvg.ts (linkedom and all) into a single
|
||||
// import-free .mjs so media-use's zero-install test lane can still run it under plain `node`.
|
||||
// Regenerate after editing the source: node scripts/generate-media-use-svg-sanitize.mjs
|
||||
import { writeFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, resolve } from "node:path";
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
const ENTRY = resolve(here, "../packages/core/src/figma/sanitizeSvg.ts");
|
||||
const OUTFILE = resolve(here, "../packages/cli/src/media-use/lib/svg-sanitize.mjs");
|
||||
// scripts/ has no node_modules of its own (bun's per-consumer linking); reach into the one
|
||||
// package that already depends on esbuild rather than adding a root-level dependency.
|
||||
const { build } = await import(resolve(here, "../packages/core/node_modules/esbuild/lib/main.js"));
|
||||
|
||||
const HEADER = `// GENERATED — do not hand-edit. Source: packages/core/src/figma/sanitizeSvg.ts, bundled by
|
||||
// scripts/generate-media-use-svg-sanitize.mjs so media-use's zero-install \`node --test\` lane can
|
||||
// still run the real sanitizer. Regenerate: node scripts/generate-media-use-svg-sanitize.mjs
|
||||
`;
|
||||
|
||||
export async function generate() {
|
||||
const result = await build({
|
||||
entryPoints: [ENTRY],
|
||||
bundle: true,
|
||||
format: "esm",
|
||||
platform: "node",
|
||||
write: false,
|
||||
minify: true,
|
||||
legalComments: "none",
|
||||
});
|
||||
return HEADER + result.outputFiles[0].text;
|
||||
}
|
||||
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
const code = await generate();
|
||||
writeFileSync(OUTFILE, code);
|
||||
console.log(`wrote ${OUTFILE} (${code.length} bytes)`);
|
||||
}
|
||||
Reference in New Issue
Block a user