Skip to content

fix(security): sandbox voice panel HTML in iframe (CRITICAL XSS) - #818

Closed
AndriiPasternak31 wants to merge 1 commit into
devfrom
fix/voice-panel-xss-iframe
Closed

fix(security): sandbox voice panel HTML in iframe (CRITICAL XSS)#818
AndriiPasternak31 wants to merge 1 commit into
devfrom
fix/voice-panel-xss-iframe

Conversation

@AndriiPasternak31

Copy link
Copy Markdown
Contributor

Summary

Agent-controlled HTML from update_panel was DOMPurify-sanitized with ADD_TAGS:['script'], then _execScripts() cloned surviving <script> tags into live DOM nodes that executed in the parent origin. Since the agent's html argument is influenceable by any external input (chat, files, MCP tool results, webhooks, channel messages, agent-website visitors), this was a generic indirect-prompt-injection XSS sink in the admin UI with JWT-theft / cross-agent-pivot blast radius.

What changed

Render the panel inside an <iframe sandbox="allow-scripts"> instead. Without allow-same-origin / allow-forms / allow-popups / allow-top-navigation / allow-modals, the iframe has an opaque origin: scripts run (Chart.js still works) but cannot read parent localStorage / cookies / JWT, submit forms, navigate the parent, or open popups.

  • Drop DOMPurify ADD_TAGS:['script'] and the _execScripts() shim
  • Drop the parent-side window.Chart global injection
  • Load Chart.js inside the iframe via Vite ?url asset import (relative path; chart.js 4 doesn't expose the UMD bundle via package exports)
  • s_open / s_close concat avoids the Vue SFC parser tripping on the inner <script> tag inside the srcdoc template

Why this shape

The previous DOMPurify-then-execScripts pattern conflates content sanitization with code execution: any element that survives sanitization gets cloned into a live script tag. Even tightening the DOMPurify rules wouldn't eliminate the sink — the agent-controlled html is the input boundary, and the parent origin is the wrong place for it to evaluate.

The sandbox iframe inverts this: the agent's script runs (preserving the panel's intended Chart.js functionality) but in an origin that has no access to anything that matters. The fix is structural, not a filter tweak.

Test plan

  • Voice panel still renders Chart.js panels correctly
  • Voice panel scripts cannot read parent.localStorage (verified via test panel attempting to access)
  • Voice panel scripts cannot post to backend endpoints with parent's JWT
  • Vue SFC parser doesn't trip on the srcdoc template

Out of scope

This PR was previously bundled into #798 (circuit-breaker fix) — split out for isolated security review.

🤖 Generated with Claude Code

Agent-controlled HTML from update_panel was DOMPurify-sanitized with
ADD_TAGS:['script'], then _execScripts() cloned surviving <script> tags
into live DOM nodes that executed in the parent origin. Since the agent's
html argument is influenceable by any external input (chat, files, MCP
tool results, webhooks, channel messages, agent-website visitors), this
was a generic indirect-prompt-injection XSS sink in the admin UI with
JWT-theft / cross-agent-pivot blast radius.

Render the panel inside an <iframe sandbox="allow-scripts"> instead.
Without allow-same-origin / allow-forms / allow-popups / allow-top-
navigation / allow-modals, the iframe has an opaque origin: scripts run
(Chart.js still works) but cannot read parent localStorage / cookies /
JWT, submit forms, navigate the parent, or open popups.

- Drop DOMPurify ADD_TAGS:['script'] and the _execScripts() shim
- Drop the parent-side window.Chart global injection
- Load Chart.js inside the iframe via Vite ?url asset import (relative
  path; chart.js 4 doesn't expose the UMD bundle via package exports)
- s_open/s_close concat avoids the Vue SFC parser tripping on the inner
  <script> tag inside the srcdoc template
@AndriiPasternak31

Copy link
Copy Markdown
Contributor Author

Closed by branch rename. Continuation: #824 (same commits, same branch under its new name).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant