From 2e54d0d1713d486eec17b9c25c7ef8d8f209b6af Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Sat, 4 Jul 2026 01:57:41 +0200
Subject: [PATCH] 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.
---
 client/src/utils/__tests__/svg.test.ts | 10 ++++++++++
 packages/api/src/mcp/icons.spec.ts     | 16 ++++++++++++++++
 packages/api/src/mcp/icons.ts          |  8 +++++++-
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/client/src/utils/__tests__/svg.test.ts b/client/src/utils/__tests__/svg.test.ts
index 1f2054656a..439eb4dcce 100644
--- a/client/src/utils/__tests__/svg.test.ts
+++ b/client/src/utils/__tests__/svg.test.ts
@@ -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>',
diff --git a/packages/api/src/mcp/icons.spec.ts b/packages/api/src/mcp/icons.spec.ts
index 65091e181e..87bebe453f 100644
--- a/packages/api/src/mcp/icons.spec.ts
+++ b/packages/api/src/mcp/icons.spec.ts
@@ -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)"',
diff --git a/packages/api/src/mcp/icons.ts b/packages/api/src/mcp/icons.ts
index e495679a5c..b7eebc6c8c 100644
--- a/packages/api/src/mcp/icons.ts
+++ b/packages/api/src/mcp/icons.ts
@@ -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;
 }