fix: decode XML entities and parse quoted URLs in SVG CSS scrubbing

Two more bypasses of the CSS reference scrubber, both live once the
stored image/svg+xml is parsed by a viewer:

- A <style> block reaches the scrubber as raw text, so an entity-encoded
  reference like &#64;import (or &#x40;import) was never seen as @import;
  the browser decodes it at render time. Decode XML character references
  (numeric + the five predefined named) before the matchers run.
- The url() regex excluded ')' even inside quotes, so a valid quoted URL
  like url("https://x/a)b") slipped through. Parse quoted and unquoted
  URL tokens separately so a ')' inside quotes stays part of the target.

Applied to both the server (sanitizeMcpIconPath) and client (sanitizeSvg)
scrubbers. Entity decoding also catches an entity-encoded </style> markup
breakout, which the re-sanitize pass then strips.
This commit is contained in:
Marco Beretta 2026-07-05 05:06:43 +02:00
parent 127bcbd1c9
commit 9c7718e854
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
4 changed files with 170 additions and 25 deletions

View file

@ -308,6 +308,27 @@ describe('sanitizeSvg', () => {
expect(clean).toContain('stroke:#000');
});
it('strips XML-entity-encoded @import from internal stylesheets', () => {
for (const enc of ['&#64;import', '&#x40;import', '&#x40;IMPORT']) {
const clean = sanitizeSvg(
`<svg><style>${enc} "https://evil.example/x.css";</style><rect /></svg>`,
);
expect(clean).not.toContain('evil.example');
}
});
it('strips a quoted CSS url() whose path contains a right parenthesis', () => {
const block = sanitizeSvg(
'<svg><style>.a{fill:url("https://evil.example/a)b")}.b{fill:url(#g)}</style><rect class="a" /></svg>',
);
expect(block).not.toContain('evil.example');
expect(block).toContain('url(#g)');
const attr = sanitizeSvg(
'<svg><rect style="fill:url(&quot;https://evil.example/a)b&quot;)" width="10" height="10" /></svg>',
);
expect(attr).not.toContain('evil.example');
});
it('drops href-smuggling animation elements', () => {
const dirty = '<svg><a><animate attributeName="href" values="javascript:alert(1)" /></a></svg>';
const clean = sanitizeSvg(dirty);

View file

@ -113,9 +113,45 @@ export function detectMonochrome(src: string): Promise<boolean> {
});
}
/** Matches every `url(...)` reference in a CSS/presentation value, capturing the
* optional quote and the target so non-fragment references can be rejected. */
const CSS_URL_REFERENCE = /url\(\s*(['"]?)([^'")]*)\1\s*\)/gi;
/** Matches every `url(...)` reference in a CSS/presentation value. A quoted
* target may contain `)` (capture groups 1/2), an unquoted one may not (group 3),
* so the target is `match[1] ?? match[2] ?? match[3]`. */
const CSS_URL_REFERENCE = /url\(\s*(?:"([^"]*)"|'([^']*)'|([^'")]*))\s*\)/gi;
/** XML predefined entities the only named references a `data:image/svg+xml`
* document (parsed as XML) decodes; unknown named entities make it fail to parse. */
const XML_NAMED_ENTITIES: Record<string, string> = {
amp: '&',
lt: '<',
gt: '>',
quot: '"',
apos: "'",
};
/**
* Decodes the XML character references a browser resolves when it parses the
* stored `image/svg+xml` document, so `&#64;import` / `&#x40;import` are seen as
* `@import` before the CSS matchers run (SVG `<style>` text is otherwise raw and
* reaches the scrubber still entity-encoded). Single pass, matching the parser.
*/
export function decodeXmlEntities(text: string): string {
if (text.indexOf('&') === -1) {
return text;
}
return text.replace(
/&#(\d+);|&#[xX]([0-9a-fA-F]+);|&(amp|lt|gt|quot|apos);/g,
(match, dec?: string, hex?: string, named?: string) => {
if (named !== undefined) {
return XML_NAMED_ENTITIES[named];
}
const code = dec !== undefined ? Number.parseInt(dec, 10) : Number.parseInt(hex ?? '', 16);
if (code === 0 || code > 0x10ffff || (code >= 0xd800 && code <= 0xdfff)) {
return match;
}
return String.fromCodePoint(code);
},
);
}
/** Matches a single CSS escape: `\` + up to six hex digits (with an optional
* trailing whitespace the browser consumes), or `\` + any other character. */
@ -143,14 +179,21 @@ export function unescapeCss(value: string): string {
});
}
/** Resolves the character references and CSS escapes a browser would, in that
* order, so an obfuscated reference is seen as the token it becomes. */
function resolveCssRefs(value: string): string {
return unescapeCss(decodeXmlEntities(value));
}
/** True when a value carries a `url(...)` reference that is not a same-document
* fragment, e.g. `url(https://…)`, `url(//…)`, `url(data:…)`, or `url(x.svg#id)`. */
export function hasExternalUrlReference(value: string): boolean {
CSS_URL_REFERENCE.lastIndex = 0;
let match: RegExpExecArray | null;
const decoded = unescapeCss(value);
const decoded = resolveCssRefs(value);
while ((match = CSS_URL_REFERENCE.exec(decoded)) !== null) {
if (!match[2].trim().startsWith('#')) {
const target = (match[1] ?? match[2] ?? match[3] ?? '').trim();
if (!target.startsWith('#')) {
return true;
}
}
@ -160,17 +203,18 @@ export function hasExternalUrlReference(value: string): boolean {
/**
* Neutralizes external references inside a `<style>` block or inline `style`
* (used by exporter SVGs that store multi-color paint in class rules) while
* keeping local rules intact. Resolves CSS escapes first so an obfuscated
* reference cannot hide, then strips comments and `@import` at-rules and
* rewrites any non-fragment `url(...)` to `none`.
* keeping local rules intact. Resolves entities and CSS escapes first so an
* obfuscated reference cannot hide, then strips comments and `@import` at-rules
* and rewrites any non-fragment `url(...)` to `none`.
*/
export function sanitizeCssText(css: string): string {
return unescapeCss(css)
return resolveCssRefs(css)
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/@import[^;]*;?/gi, '')
.replace(CSS_URL_REFERENCE, (full, _quote, target) =>
target.trim().startsWith('#') ? full : 'none',
);
.replace(CSS_URL_REFERENCE, (full, q1, q2, q3) => {
const target = (q1 ?? q2 ?? q3 ?? '').trim();
return target.startsWith('#') ? full : 'none';
});
}
let svgPurifier: ReturnType<typeof DOMPurify> | null = null;

View file

@ -198,6 +198,38 @@ describe('sanitizeMcpIconPath', () => {
expect(clean).toContain('stroke:#000');
});
it('strips XML-entity-encoded @import from internal stylesheets', () => {
for (const enc of ['&#64;import', '&#x40;import', '&#x40;IMPORT']) {
const raw = `<svg><style>${enc} "https://evil.example/x.css";</style><rect/></svg>`;
expect(
decode(sanitizeMcpIconPath(`data:image/svg+xml,${encodeURIComponent(raw)}`)),
).not.toContain('evil.example');
}
});
it('strips markup smuggled through XML-entity-encoded </style>', () => {
const raw =
'<svg><style>&#60;/style&#62;&#60;image href="https://evil.example/x.png"/&#62;</style></svg>';
const clean = decode(sanitizeMcpIconPath(`data:image/svg+xml,${encodeURIComponent(raw)}`));
expect(clean).not.toContain('evil.example');
expect(clean.toLowerCase()).not.toContain('<image');
});
it('strips a quoted CSS url() whose path contains a right parenthesis', () => {
const block =
'<svg><style>.a{fill:url("https://evil.example/a)b")}.b{fill:url(#g)}</style><rect class="a"/></svg>';
const cleanBlock = decode(
sanitizeMcpIconPath(`data:image/svg+xml,${encodeURIComponent(block)}`),
);
expect(cleanBlock).not.toContain('evil.example');
expect(cleanBlock).toContain('url(#g)');
const attr =
'<svg><rect style="fill:url(&quot;https://evil.example/a)b&quot;)" width="10" height="10"/></svg>';
expect(
decode(sanitizeMcpIconPath(`data:image/svg+xml,${encodeURIComponent(attr)}`)),
).not.toContain('evil.example');
});
it('preserves case-sensitive SVG names and multi-color paint', () => {
const raw =
'<svg viewBox="0 0 24 24"><linearGradient id="g"><stop offset="0" stop-color="#f00"/></linearGradient><path d="M0 0h24v24H0z" fill="url(#g)"/></svg>';

View file

@ -181,9 +181,50 @@ const ALLOWED_SVG_ATTRS = [
'tableValues',
];
/** Matches every `url(...)` reference in a presentation/style value, capturing
* the optional quote and the target so non-fragment references can be rejected. */
const CSS_URL_REFERENCE = /url\(\s*(['"]?)([^'")]*)\1\s*\)/gi;
/** Matches every `url(...)` reference in a presentation/style value. A quoted
* target may contain `)` (capture groups 1/2), an unquoted one may not (group 3),
* so the target is `match[1] ?? match[2] ?? match[3]`. */
const CSS_URL_REFERENCE = /url\(\s*(?:"([^"]*)"|'([^']*)'|([^'")]*))\s*\)/gi;
/** The target of a `CSS_URL_REFERENCE` match, from whichever quoting group hit. */
function urlTarget(match: RegExpExecArray | RegExpMatchArray): string {
return (match[1] ?? match[2] ?? match[3] ?? '').trim();
}
/** XML predefined entities the only named references a `data:image/svg+xml`
* document (parsed as XML) decodes; unknown named entities make it fail to parse. */
const XML_NAMED_ENTITIES: Record<string, string> = {
amp: '&',
lt: '<',
gt: '>',
quot: '"',
apos: "'",
};
/**
* Decodes the XML character references a browser resolves when it parses the
* stored `image/svg+xml` document, so `&#64;import` / `&#x40;import` are seen as
* `@import` before the CSS matchers run (SVG `<style>` text is otherwise raw and
* reaches this scrubber still entity-encoded). Single pass, matching the parser.
*/
function decodeXmlEntities(text: string): string {
if (text.indexOf('&') === -1) {
return text;
}
return text.replace(
/&#(\d+);|&#[xX]([0-9a-fA-F]+);|&(amp|lt|gt|quot|apos);/g,
(match, dec: string | undefined, hex: string | undefined, named: string | undefined) => {
if (named !== undefined) {
return XML_NAMED_ENTITIES[named];
}
const code = dec !== undefined ? Number.parseInt(dec, 10) : Number.parseInt(hex ?? '', 16);
if (code === 0 || code > 0x10ffff || (code >= 0xd800 && code <= 0xdfff)) {
return match;
}
return String.fromCodePoint(code);
},
);
}
/** Matches a single CSS escape: `\` + up to six hex digits (with an optional
* trailing whitespace the browser consumes), or `\` + any other character. */
@ -211,31 +252,38 @@ function unescapeCss(value: string): string {
});
}
/** Resolves the character references and CSS escapes a browser would, in that
* order, so an obfuscated reference is seen as the token it becomes. */
function resolveCssRefs(value: string): string {
return unescapeCss(decodeXmlEntities(value));
}
/** True when a value carries a `url(...)` reference that is not a same-document
* fragment, e.g. `url(https://…)`, `url(//…)`, `url(data:…)`, or `url(x.svg#id)`. */
function hasExternalUrlReference(value: string): boolean {
CSS_URL_REFERENCE.lastIndex = 0;
let match: RegExpExecArray | null;
const decoded = unescapeCss(value);
const decoded = resolveCssRefs(value);
while ((match = CSS_URL_REFERENCE.exec(decoded)) !== null) {
if (!match[2].trim().startsWith('#')) {
if (!urlTarget(match).startsWith('#')) {
return true;
}
}
return false;
}
/** Resolves CSS escapes, then strips comments and `@import` at-rules and rewrites
* external `url(...)` to `none`, keeping only local paint rules from a stylesheet
* or inline `style`. Escapes are resolved first so an obfuscated reference cannot
* hide from the matchers. */
/** Resolves entities/escapes, then strips comments and `@import` at-rules and
* rewrites external `url(...)` to `none`, keeping only local paint rules from a
* stylesheet or inline `style`. References are resolved first so an obfuscated
* one cannot hide from the matchers. */
function sanitizeCssText(css: string): string {
return unescapeCss(css)
return resolveCssRefs(css)
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/@import[^;]*;?/gi, '')
.replace(CSS_URL_REFERENCE, (full, _quote, target) =>
target.trim().startsWith('#') ? full : 'none',
);
.replace(CSS_URL_REFERENCE, (full, q1, q2, q3) => {
const target = (q1 ?? q2 ?? q3 ?? '').trim();
return target.startsWith('#') ? full : 'none';
});
}
/** Scrubs the CSS inside every `<style>` block of an already-tag-sanitized SVG so