fix: re-sanitize after scrubbing style blocks to block markup reintroduction

scrubStyleBlocks runs after sanitizeHtml and splices un-escaped CSS back
as raw markup, so an escaped sequence like \3c/style\3e\3cimage/\3e
(inert text during the first pass) became a real </style><image href>
element after unescaping, past the allowlist. Re-run the allowlist over
the scrubbed result whenever a <style> block was rewritten, stripping any
element the un-escaping surfaced while leaving legit local rules intact.

The client sanitizer is unaffected: it sets the scrubbed CSS as a DOM
text node, which serializes with < and > escaped, so no element is
reintroduced. Added a regression test pinning that behavior too.
This commit is contained in:
Marco Beretta 2026-07-04 01:57:41 +02:00
parent 3a94d5d9ff
commit 2e54d0d171
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
3 changed files with 33 additions and 1 deletions

View file

@ -271,6 +271,16 @@ describe('sanitizeSvg', () => {
expect(clean).not.toContain('alert(1)');
});
it('does not let escaped markup in a stylesheet reintroduce elements', () => {
// Scrubbed CSS is set as a text node, so `\3c/style\3e\3cimage\3e` stays
// inert escaped text rather than becoming a real element.
const dirty =
'<svg><style>\\3c/style\\3e\\3cimage href="https://evil.example/x.png"/\\3e</style></svg>';
const clean = sanitizeSvg(dirty);
expect(clean.toLowerCase()).not.toContain('<image');
expect(clean).toContain('&lt;');
});
it('strips CSS-escaped external url() from style attributes and stylesheets', () => {
const attrEsc = sanitizeSvg(
'<svg><rect style="fill:u\\72l(https://evil.example/x)" width="10" height="10" /></svg>',

View file

@ -131,6 +131,22 @@ describe('sanitizeMcpIconPath', () => {
expect(clean).not.toContain('alert(1)');
});
it('does not let escaped markup in a stylesheet reintroduce elements past the allowlist', () => {
// `\3c` = "<", `\3e` = ">": harmless text during the first pass, real markup
// once the CSS is un-escaped and spliced back — must be re-sanitized away.
const cases = [
'<svg><style>\\3c/style\\3e\\3cimage href="https://evil.example/x.png"/\\3e</style></svg>',
'<svg><style>\\3c/style\\3e\\3cscript\\3ealert(1)\\3c/script\\3e</style></svg>',
'<svg><style>\\3c/style\\3e\\3cstyle\\3e@import "https://evil.example/x.css";\\3c/style\\3e</style></svg>',
];
for (const raw of cases) {
const clean = decode(sanitizeMcpIconPath(`data:image/svg+xml,${encodeURIComponent(raw)}`));
expect(clean).not.toContain('evil.example');
expect(clean.toLowerCase()).not.toContain('<image');
expect(clean.toLowerCase()).not.toContain('<script');
}
});
it('strips external url() references from presentation and style attributes', () => {
for (const attr of [
'filter="url(https://evil.example/f.svg#f)"',

View file

@ -362,7 +362,13 @@ export function sanitizeMcpIconPath(iconPath: string): string {
if (svg == null) {
return '';
}
const clean = scrubStyleBlocks(sanitizeHtml(svg, SVG_SANITIZE_OPTIONS));
const sanitized = sanitizeHtml(svg, SVG_SANITIZE_OPTIONS);
const scrubbed = scrubStyleBlocks(sanitized);
/* `scrubStyleBlocks` splices un-escaped CSS back as raw markup, so an escaped
* sequence like `\3c/style\3e\3cimage/\3e` can reintroduce a real element past
* the allowlist. When a `<style>` block was actually rewritten, re-run the
* allowlist over the result to strip anything the un-escaping surfaced. */
const clean = scrubbed === sanitized ? sanitized : sanitizeHtml(scrubbed, SVG_SANITIZE_OPTIONS);
const encoded = `data:image/svg+xml;base64,${Buffer.from(clean, 'utf-8').toString('base64')}`;
return encoded.length > MAX_MCP_ICON_PATH_LENGTH ? '' : encoded;
}