Skip to content

fix(trace-viewer): sanitize snapshot renderer against XSS from crafted traces#40655

Open
SebTardif wants to merge 1 commit intomicrosoft:mainfrom
SebTardif:fix/snapshot-renderer-xss-v2
Open

fix(trace-viewer): sanitize snapshot renderer against XSS from crafted traces#40655
SebTardif wants to merge 1 commit intomicrosoft:mainfrom
SebTardif:fix/snapshot-renderer-xss-v2

Conversation

@SebTardif
Copy link
Copy Markdown

@SebTardif SebTardif commented May 6, 2026

Summary

  • The snapshot renderer did not filter SCRIPT elements or sanitize the doctype field. A crafted trace file could inject ["SCRIPT", {}, "alert(1)"] into snapshot JSON or use a malicious doctype value to break out of the DOCTYPE declaration and execute arbitrary JavaScript in the trace viewer context.
  • The capture side (snapshotterInjected.ts) already strips SCRIPT elements and sanitizes attributes, but trace files are untrusted input and the renderer must not rely on the capture side for safety.
  • Trace files are routinely shared between team members for debugging and can be opened on trace.playwright.dev, a public website. The XSS executes automatically when a snapshot renders, with no user interaction beyond opening the trace.
  • Skip SCRIPT elements in the renderer (mirrors snapshotterInjected.ts:364)
  • Sanitize doctype to alphanumeric only (valid doctype names are always alphanumeric)

Follows the same defense-in-depth pattern as #40115 and #14325.

…d traces

The snapshot renderer did not filter SCRIPT elements or sanitize the
doctype field. A crafted trace file could inject a SCRIPT node into
snapshot JSON or use a malicious doctype value to break out of the
DOCTYPE declaration and execute arbitrary JavaScript in the trace
viewer context.

The capture side (snapshotterInjected.ts) already strips SCRIPT
elements, but trace files are untrusted input and the renderer must
not rely on the capture side for safety.

- Skip SCRIPT elements in the renderer (mirrors snapshotterInjected.ts:364)
- Sanitize doctype to alphanumeric only (valid doctype names are always
  alphanumeric)

Follows the same defense-in-depth pattern as microsoft#40115 and microsoft#14325.
@pavelfeldman
Copy link
Copy Markdown
Member

I'll accept the patch, but this vector of attack has been previously discovered and considered to not be a threat. trace.playwright.dev is designed to not manage user data. Looking at the vulnerabilities you report, you are going over the results of the same model we have access to.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Test results for "MCP"

10 failed
❌ [chromium] › mcp/dashboard.spec.ts:185 › should start dashboard and annotate when no dashboard is running @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:207 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:231 › should annotate via direct browser_annotate MCP call @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:264 › should annotate when context has no fixed viewport @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:301 › should cancel browser_annotate when the MCP request is aborted @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:332 › should cancel browser_annotate when the MCP client disconnects @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:361 › should switch screencast to -s session on show --annotate @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:410 › should disengage annotate mode when --annotate client disconnects @mcp-windows-latest-chromium
❌ [chromium] › mcp/dashboard.spec.ts:499 › save recording streams WebM bytes to the chosen file @mcp-windows-latest-chromium
❌ [webkit] › mcp/config.ini.spec.ts:57 › ini config sets browser launch options @mcp-windows-latest-webkit

6933 passed, 1052 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Test results for "tests 1"

1 failed
❌ [firefox-page] › page/page-emulate-media.spec.ts:196 › should report hover and fine pointer for desktop @firefox-ubuntu-22.04-node20

2 flaky ⚠️ [chromium-library] › library/browsertype-connect.spec.ts:891 › run-server › socks proxy › should proxy ipv6 localhost requests @smoke `@chromium-ubuntu-22.04-node22`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:812 › should update state on subsequent run `@windows-latest-node20`

41652 passed, 851 skipped


Merge workflow run.

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.

2 participants