Skip to content

fix(trace-viewer): neutralize object and embed tags in snapshot renderer#40761

Merged
pavelfeldman merged 2 commits intomicrosoft:mainfrom
SebTardif:fix-trace-viewer-object-xss
May 10, 2026
Merged

fix(trace-viewer): neutralize object and embed tags in snapshot renderer#40761
pavelfeldman merged 2 commits intomicrosoft:mainfrom
SebTardif:fix-trace-viewer-object-xss

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

Summary

Attack scenario

A crafted trace can include a resource containing malicious HTML/JS with content type text/html, and a snapshot referencing it via <object data="/sha1/<sha1>" type="text/html">. The service worker serves the resource at /sha1/ with the attacker-controlled content type. The <object> tag renders it in a browsing context within the trace viewer's origin, giving the attacker full JS execution. This allows exfiltrating sensitive data from the trace (API keys, cookies, auth tokens from the tested application).

<embed src> has the same issue. Although src is processed by rewriteURLForCustomProtocol, relative URLs (like /sha1/...) pass through unchanged because new URL(relativePath) throws and the catch block returns the URL as-is.

Fix

Rename data on <object> and src on <embed> to __playwright_data__ / __playwright_src__, following the same pattern used for <iframe src> and <iframe srcdoc> since #40676.

Origin

The iframe src neutralization was introduced by Simon Knott (2024-09-04). It was expanded in #40676 to cover srcdoc and sandbox, but <object> and <embed> were not covered.

Neutralize <object data> and <embed src> attributes in the snapshot
renderer. Both can load arbitrary HTML in the trace viewer's origin
via /sha1/ resources, bypassing the script and iframe sanitization
added in microsoft#40655 and microsoft#40676.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
attrName = '__playwright_' + attr.toLowerCase() + '__';
}
if (upperName === 'OBJECT' && attr.toLowerCase() === 'data') {
// Neutralize <object data> - it can load arbitrary HTML in the trace
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's drop all thees "neutralize" comments, they are noise.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, dropped all the comments.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chromium] › mcp/annotate.spec.ts:273 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-chromium
❌ [chromium] › mcp/annotate.spec.ts:297 › should annotate via direct browser_annotate MCP call @mcp-windows-latest-chromium

7055 passed, 1068 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

3 flaky ⚠️ [chromium-library] › library/video.spec.ts:647 › screencast › should capture full viewport `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [firefox-page] › page/page-request-gc.spec.ts:19 › should work `@firefox-ubuntu-22.04-node20`

41731 passed, 850 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