From 9c7718e854169f049d2b1f021c052dcdfad015f5 Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Sun, 5 Jul 2026 05:06:43 +0200
Subject: [PATCH] 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.
---
 client/src/utils/__tests__/svg.test.ts | 21 ++++++++
 client/src/utils/svg.ts                | 68 ++++++++++++++++++-----
 packages/api/src/mcp/icons.spec.ts     | 32 +++++++++++
 packages/api/src/mcp/icons.ts          | 74 +++++++++++++++++++++-----
 4 files changed, 170 insertions(+), 25 deletions(-)

diff --git a/client/src/utils/__tests__/svg.test.ts b/client/src/utils/__tests__/svg.test.ts
index 439eb4dcce..00c9bc525b 100644
--- a/client/src/utils/__tests__/svg.test.ts
+++ b/client/src/utils/__tests__/svg.test.ts
@@ -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);
diff --git a/client/src/utils/svg.ts b/client/src/utils/svg.ts
index f7f6a01b4c..fae614ef0d 100644
--- a/client/src/utils/svg.ts
+++ b/client/src/utils/svg.ts
@@ -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;
diff --git a/packages/api/src/mcp/icons.spec.ts b/packages/api/src/mcp/icons.spec.ts
index 44f57877c7..93062c28ed 100644
--- a/packages/api/src/mcp/icons.spec.ts
+++ b/packages/api/src/mcp/icons.spec.ts
@@ -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>';
diff --git a/packages/api/src/mcp/icons.ts b/packages/api/src/mcp/icons.ts
index eeb66aa02f..4078892430 100644
--- a/packages/api/src/mcp/icons.ts
+++ b/packages/api/src/mcp/icons.ts
@@ -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