Skip to content

Security: Unsanitized SVG content injected via dangerouslySetInnerHTML (XSS risk) #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tuanaiseo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Security: Unsanitized SVG content injected via dangerouslySetInnerHTML (XSS risk) #647

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions src/components/plugins/view/PluginIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,43 @@ type Props = {
// Module-level cache so repeated renders don't re-fetch
const svgCache = new Map<string, string>();

function sanitizeSvg(svgText: string): string | null {
try {
const doc = new DOMParser().parseFromString(svgText, 'image/svg+xml');
const root = doc.documentElement;
if (!root || root.nodeName.toLowerCase() !== 'svg') return null;

doc
.querySelectorAll('script,foreignObject,iframe,object,embed,link,meta,style')
.forEach((el) => el.remove());
Comment on lines +19 to +21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

SVG animate/set elements can bypass event handler sanitization.

The removal list is missing animate, set, and animateTransform elements. These SVG animation elements can dynamically set attribute values, including event handlers:

<svg xmlns="http://www.w3.org/2000/svg">
  <rect width="100" height="100">
    <set attributeName="onclick" to="alert('XSS')" begin="0s" fill="freeze"/>
  </rect>
</svg>

When the user clicks the rect, the injected handler executes.

🛡️ Proposed fix to include animation elements 
   doc
-    .querySelectorAll('script,foreignObject,iframe,object,embed,link,meta,style')
+    .querySelectorAll('script,foreignObject,iframe,object,embed,link,meta,style,animate,set,animateTransform,animateMotion')
     .forEach((el) => el.remove());
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/plugins/view/PluginIcon.tsx` around lines 19 - 21, The SVG
sanitizer in PluginIcon.tsx currently removes elements via
doc.querySelectorAll('script,foreignObject,iframe,object,embed,link,meta,style')
but omits SVG animation elements which can inject event handlers; update the
selector used on the parsed document (the call where doc.querySelectorAll(...)
is invoked inside the PluginIcon component/cleanup logic) to also remove
'animate', 'set', and 'animateTransform' nodes so those SVG animation elements
are stripped before insertion.


const walker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT);
const elements: Element[] = [root];
while (walker.nextNode()) {
elements.push(walker.currentNode as Element);
}

elements.forEach((el) => {
Array.from(el.attributes).forEach((attr) => {
const name = attr.name.toLowerCase();
const value = attr.value.trim().toLowerCase();
if (
name.startsWith('on') ||
name === 'href' ||
name === 'xlink:href' ||
value.startsWith('javascript:')
) {
el.removeAttribute(attr.name);
}
});
});

return new XMLSerializer().serializeToString(root);
} catch {
return null;
}
}

export default function PluginIcon({ pluginName, iconFile, className }: Props) {
const url = iconFile
? `/api/plugins/${encodeURIComponent(pluginName)}/assets/${encodeURIComponent(iconFile)}`
Expand All @@ -24,9 +61,11 @@ export default function PluginIcon({ pluginName, iconFile, className }: Props) {
return r.text();
})
.then((text) => {
if (text && text.trimStart().startsWith('<svg')) {
svgCache.set(url, text);
setSvg(text);
if (!text) return;
const sanitized = sanitizeSvg(text);
if (sanitized) {
svgCache.set(url, sanitized);
setSvg(sanitized);
}
})
.catch(() => {});
Expand All @@ -35,10 +74,6 @@ export default function PluginIcon({ pluginName, iconFile, className }: Props) {
if (!svg) return <span className={className} />;

return (
<span
className={className}
// SVG is fetched from the user's own installed plugin — same trust level as the plugin code itself
dangerouslySetInnerHTML={{ __html: svg }}
/>
<span className={className} dangerouslySetInnerHTML={{ __html: svg }} />
);
}