Fix stored XSS in iframe and blockquote media types#501
Open
scott2b wants to merge 1 commit into
Open
Conversation
The media url field is overloaded: for the iframe and blockquote media types it holds raw HTML pasted by the user, which was injected directly via innerHTML. Any markup stored in a storymap's JSON could therefore execute script in a viewer's browser (stored XSS on the shared hosted origin). Unlike PR #500, which treated the field as a bare URL and would have broken every existing storymap with a pasted embed code, this fix parses the pasted markup inertly with DOMParser and rebuilds clean DOM: - IFrame: extract and validate the src (http/https only), copy only allowlisted presentation attributes onto a freshly created iframe. Bare URLs that route to the iframe type now also work. Invalid embeds show the standard media load error. - Blockquote: rebuild the quote from an allowlist of formatting tags, stripping all attributes except validated link hrefs; script/object/ style and similar tags are dropped with their contents. Adds tests (node --test + jsdom, wired to npm test) and declares terser-webpack-plugin, which webpack.common.js requires directly but newer webpack 5 releases no longer provide transitively. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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
Fixes the stored XSS reported in #500: the media
urlfield flows unmodified intoinnerHTMLin the iframe and blockquote media types, so markup stored in a storymap's JSON can execute arbitrary script in any viewer's browser — on the shared hosted origin, that means one user's storymap can attack another logged-in user's editor session.Why not #500 as written
The
urlfield is overloaded by design: the iframe and blockquote media types are only selected when the field contains pasted embed markup (MediaType.jssubstring-matches"iframe"/"blockquote"against the field). #500 treats the field as a bare URL (iframe.src = media_id) and as plain text (textContent), which closes the hole but breaks every existing storymap that uses a pasted embed code or quote.Approach
New
src/js/media/EmbedUtil.jsparses the pasted markup inertly withDOMParser(no scripts execute, no handlers fire, no subresources load) and rebuilds clean DOM from an allowlist:<iframe>'ssrc, validate it resolves to http(s) (rejectsjavascript:,data:, etc.), and build a fresh iframe copying only presentation attributes (width,height,frameborder,allowfullscreen,allow,scrolling,title). Event handlers,srcdoc, and sibling tags are discarded. A bare URL that routes to the iframe type now also works. Invalid input shows the standard media load error.blockquote,p,cite,em, …) with all attributes stripped except validated linkhrefs;script/style/object/embedding tags are dropped with their contents; unknown tags are unwrapped keeping their text.Existing storymaps with pasted embed codes and quotes keep rendering. No new runtime dependency —
EmbedUtilis pure browser DOM code bundled like any other module.Tests
Adds the project's first JS unit tests: 16 tests covering the XSS vectors (event handlers,
javascript:URLs, script/img injection) and the legitimate embed cases, run with the Node built-in test runner + jsdom (dev-only deps).npm testis now wired to run them.Also declares
terser-webpack-pluginas a devDependency —webpack.common.jsrequires it directly, but newer webpack 5 releases no longer ship it transitively, so fresh installs couldn't build.Verification
npm test: 16/16 passnpx webpack -c webpack.dev.js: compiles successfullyCloses #500 (supersedes).
🤖 Generated with Claude Code