fix: prevent D3 export action XSS via untrusted dataUrl#1344
Merged
Conversation
The D3 chart runs in a null-origin sandboxed iframe and returns its exported image over postMessage, so the dataUrl is attacker-controlled. The print action wrote it unescaped into a same-origin window.open() popup via document.write, letting hostile chart code inject active markup into the site origin (stored XSS reachable by a Contributor who can edit their own draft chart, no unfiltered_html needed). Validate the value as a base64 image data URI and build the <img> via the DOM API instead of string concatenation. Adds a jsdom regression test driving the real module (red against the old code, green now). Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
💂♂️ PR Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Contributor
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pirate-bot
added
the
pr-checklist-complete
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The D3 renderer's print action wrote the
dataUrlreceived overpostMessagefrom the null-origin export iframe into a popup viadocument.write(), letting a hostile value smuggle markup/script into a same-origin document. The value is now validated as a base64data:image/(png|jpeg|webp)URI and the<img>is built via the DOM API so it is only ever an attribute value.Follow-up hardening for the same value on the PHP side:
sanitizeSettings()only storeschart-imgwhen it is a base64 image data URI, andsave_chart_image()verifies the PNG magic bytes before writing to uploads. The bogus baselined@return attachmenttype onsave_chart_image()was corrected tointand its three stale PHPStan baseline entries removed.Will affect visual aspect of the product
NO
Test instructions
npx jest classes/Visualizer/D3Renderer/tests/print-action-xss.test.js— hostiledataUrlpayloads open no popup and inject no markup; a legitimate PNG export renders as a single<img>.vendor/bin/phpunit --filter Test_Visualizer_Sanitize_Settings— hostilechart-imgvalues are dropped;save_chart_image()rejects non-PNG payloads.Check before Pull Request is ready:
🤖 Generated with Claude Code