From c6f3d23d8eb03c61de29ce4c819a920349ec9929 Mon Sep 17 00:00:00 2001 From: scott2b Date: Thu, 16 Jul 2026 17:04:47 -0500 Subject: [PATCH 1/2] Fix stored XSS in iframe and blockquote media types 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) --- package.json | 4 +- src/js/media/EmbedUtil.js | 118 ++++++++++++++++++++++++ src/js/media/types/Blockquote.js | 21 +++-- src/js/media/types/IFrame.js | 28 +++--- tests/js/embed_util.test.mjs | 148 +++++++++++++++++++++++++++++++ 5 files changed, 299 insertions(+), 20 deletions(-) create mode 100644 src/js/media/EmbedUtil.js create mode 100644 tests/js/embed_util.test.mjs diff --git a/package.json b/package.json index 0a1df880..6d8826d8 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "file-loader": "^6.2.0", "fs-extra": "^9.0.1", "html-webpack-plugin": "^4.5.0", + "jsdom": "^29.1.1", "json-loader": "^0.5.7", "jstrace": "^0.3.0", "less": "^3.13.1", @@ -30,6 +31,7 @@ "run-all": "^1.0.1", "run-s": "0.0.0", "style-loader": "^2.0.0", + "terser-webpack-plugin": "^5.6.1", "trash": "^7.1.0", "webpack": "^5.11.0", "webpack-cli": "^4.7.0", @@ -40,7 +42,7 @@ "start": "webpack serve --config webpack.dev.js --open", "build": "webpack --config webpack.prd.js && node tasks/compile_less.js", "clean": "trash dist", - "test": "echo \"Error: no test specified\" && exit 1", + "test": "node --test \"tests/js/**/*.test.mjs\"", "dist": "npm-run-all -s clean build && cp dist/js/storymap.js dist/js/storymap-min.js", "stage": "npm run dist && node tasks/stage.js", "stage_latest": "npm run dist && node tasks/stage.js latest", diff --git a/src/js/media/EmbedUtil.js b/src/js/media/EmbedUtil.js new file mode 100644 index 00000000..2c7ffba0 --- /dev/null +++ b/src/js/media/EmbedUtil.js @@ -0,0 +1,118 @@ +/* EmbedUtil + Utilities for safely rendering user-pasted embed snippets. + + The media "url" field is overloaded: for the iframe and blockquote + media types it holds raw HTML pasted by the user (that markup is + what routes the media to those types in MediaType.js). These helpers + parse that HTML inertly -- DOMParser never executes scripts or event + handlers -- and rebuild clean DOM from an allowlist, so markup stored + in a storymap's JSON can never execute code in a viewer's browser. +================================================== */ + +// Attributes copied from a pasted ' + ); + assert.ok(iframe); + assert.equal(iframe.tagName, "IFRAME"); + assert.equal(iframe.getAttribute("src"), "https://www.youtube.com/embed/abc123"); + assert.equal(iframe.getAttribute("width"), "560"); + assert.equal(iframe.getAttribute("height"), "315"); + assert.equal(iframe.getAttribute("frameborder"), "0"); + assert.ok(iframe.hasAttribute("allowfullscreen")); +}); + +test("buildIframe strips event handlers and unknown attributes", () => { + const iframe = buildIframe( + '' + ); + assert.ok(iframe); + assert.equal(iframe.getAttribute("onload"), null); + assert.equal(iframe.getAttribute("name"), null); + assert.equal(iframe.getAttribute("srcdoc"), null); +}); + +test("buildIframe discards markup outside the iframe", () => { + const iframe = buildIframe( + '' + ); + assert.ok(iframe); + assert.equal(iframe.tagName, "IFRAME"); + assert.equal(iframe.getAttribute("src"), "https://example.com/"); +}); + +test("buildIframe rejects javascript: src", () => { + assert.equal(buildIframe(''), null); +}); + +test("buildIframe rejects an iframe with no src", () => { + assert.equal(buildIframe(""), null); +}); + +test("buildIframe accepts a bare URL that routed to the iframe type", () => { + const iframe = buildIframe("https://example.com/iframe-demo"); + assert.ok(iframe); + assert.equal(iframe.getAttribute("src"), "https://example.com/iframe-demo"); + assert.equal(iframe.getAttribute("width"), "100%"); +}); + +test("buildIframe returns null for text that is neither embed nor URL", () => { + assert.equal(buildIframe("this mentions iframe but embeds nothing"), null); +}); + +/* sanitizeBlockquote +================================================== */ + +function renderedHTML(fragment) { + const div = document.createElement("div"); + div.appendChild(fragment); + return div.innerHTML; +} + +test("sanitizeBlockquote keeps quote structure and text", () => { + const html = renderedHTML( + sanitizeBlockquote("

Truth is rarely pure.

Oscar Wilde
") + ); + assert.equal(html, "

Truth is rarely pure.

Oscar Wilde
"); +}); + +test("sanitizeBlockquote drops script tags and their contents", () => { + const html = renderedHTML( + sanitizeBlockquote("
quote
") + ); + assert.equal(html, "
quote
"); +}); + +test("sanitizeBlockquote strips event handler and style attributes", () => { + const html = renderedHTML( + sanitizeBlockquote('
quote
') + ); + assert.equal(html, "
quote
"); +}); + +test("sanitizeBlockquote unwraps unknown tags but keeps their text", () => { + const html = renderedHTML( + sanitizeBlockquote("
quote

heading

") + ); + assert.equal(html, "
quote
heading"); +}); + +test("sanitizeBlockquote drops img-based XSS entirely", () => { + const html = renderedHTML( + sanitizeBlockquote('
q
') + ); + assert.equal(html, "
q
"); +}); + +test("sanitizeBlockquote keeps links with valid href, drops javascript: href", () => { + const good = renderedHTML( + sanitizeBlockquote('
link
') + ); + assert.equal(good, '
link
'); + + const bad = renderedHTML( + sanitizeBlockquote('
link
') + ); + assert.equal(bad, "
link
"); +}); From 88d76eac56d168bba8085f04a6e485af4fd94a9c Mon Sep 17 00:00:00 2001 From: scott2b Date: Fri, 17 Jul 2026 11:16:42 -0500 Subject: [PATCH 2/2] Add STORYMAP_LIB_URL dev-mode override for the viewer library The editor loaded the StoryMap viewer library (js + css) from CDN_URL, which forced a choice between pointing CDN_URL at a deployed CDN (can't test local library changes) or at the local build (loses the deployed convenience). Decouple the two: - settings.STORYMAP_LIB_URL, defaulting to CDN_URL (backward compatible) - context processor injects LIB_URL/lib_url alongside CDN_URL - edit.html and base.html load the viewer from LIB_URL - set STORYMAP_LIB_URL=/compiled/ to load the local npm run build output (served from dist/ via the existing /compiled/ route) Documents the dev modes and a console snippet to verify which bundle a page is running, which is how you confirm the XSS fix is being exercised locally rather than against the deployed CDN. Co-Authored-By: Claude Opus 4.8 (1M context) --- DEVELOPMENT.md | 64 +++++++++++++++++++++++++++++++++--- dotenv.example | 6 ++++ storymap/api.py | 7 +++- storymap/core/settings.py | 8 +++++ storymap/templates/base.html | 4 +-- storymap/templates/edit.html | 10 ++---- 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 5157bd13..c4e60a1a 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -16,17 +16,71 @@ Install the dependencies and build the javascript: ## Questions not yet completely addressed with the new localstack based setup: -**Note:** +## Which StoryMap viewer library the editor loads -Built static is served directly (via the flask app) from the `compiled` -directory, but the current development setup does not sort this out very well. -Unless you have a need to host the static code locally, the easiest thing to do -is probably to point to a deployed cdn. ie., set this env variable: +The editor renders slide media — and the map preview — using the compiled +StoryMap viewer library (`storymap.js` + `storymap.css`). Two environment +variables control where those assets come from: + +- `CDN_URL` — base URL for shared assets (fonts, embed pages, etc.). +- `STORYMAP_LIB_URL` — base URL for the **viewer library** specifically. + Defaults to `CDN_URL` when unset. + +Splitting these lets you keep `CDN_URL` pointed at a deployed CDN for +convenience while independently loading the viewer library from a local build +when you are working on the library itself. + +### Development modes + +| `STORYMAP_LIB_URL` | Editor loads the viewer from | Use when | +| --- | --- | --- | +| *(unset)* | whatever `CDN_URL` is (e.g. the deployed CDN) | working on the editor/server, not the library | +| `/compiled/` | your local `npm run build` output in `dist/`, served by the `/compiled/` route | testing library changes before they are deployed | + +Pointing `CDN_URL` at a deployed CDN remains the easiest default when you have +no need to host the library locally: ``` CDN_URL=https://cdn.knightlab.com/libs/storymapjs/latest/ ``` +To load the **local** build instead, set the library override in `.env`: + +``` + STORYMAP_LIB_URL=/compiled/ +``` + +Then build the library and recreate the app container so it picks up the env +change (a plain `docker compose restart` does NOT reload env-file values): + +``` + $ npm run build # writes dist/js/storymap.js + dist/css/storymap.css + $ docker compose up -d --force-recreate app +``` + +`dist/` is bind-mounted into the container, so for subsequent library changes +you only need to re-run `npm run build` and hard-reload the browser — no +container recreate required. + +### Verifying which library version is loaded + +Browser tabs cache the library, so after switching modes or rebuilding you must +hard-reload the editor (Cmd/Ctrl+Shift+R). A tab opened *before* you changed +`STORYMAP_LIB_URL` (or before a rebuild) keeps running the previously loaded +bundle in memory — a common source of "my change isn't showing up" confusion. + +To confirm which bundle a page is actually running, open the browser console +and run: + +```js +document.querySelector('script[src*="storymap.js"]').src +``` + +- `.../compiled/js/storymap.js` → the local build (your uncommitted changes). +- `https://cdn.knightlab.com/libs/storymapjs/latest/...` → the deployed CDN build. + +If the src is not what you expect, hard-reload the tab. + ## Overview / tl;dr To get started you will need to do the following steps which are described in diff --git a/dotenv.example b/dotenv.example index 7e7eaca6..18504e4d 100644 --- a/dotenv.example +++ b/dotenv.example @@ -5,6 +5,12 @@ AWS_STORAGE_BUCKET_KEY=storymapjs AWS_STORAGE_BUCKET_NAME=uploads.knilab.com AWS_STORAGE_BUCKET_URL=http://localhost:4566/uploads.knilab.com/ CDN_URL=https://localhost/compiled/ +# Where the editor loads the StoryMap viewer library (js + css). Defaults to +# CDN_URL when unset. Development modes: +# (unset) -> follow CDN_URL (e.g. a deployed CDN for convenience) +# /compiled/ -> load the local `npm run build` output from dist/, to test +# viewer changes in the editor before deploying to the CDN +# STORYMAP_LIB_URL=/compiled/ DB_ENGINE__DEFAULT=mongo DB_HOST__DEFAULT=mongo DB_NAME__DEFAULT=storymapjs diff --git a/storymap/api.py b/storymap/api.py index 7867c298..0d680651 100644 --- a/storymap/api.py +++ b/storymap/api.py @@ -116,10 +116,15 @@ def inject_urls(): if not cdn_url.endswith('/'): cdn_url += '/' + lib_url = settings.STORYMAP_LIB_URL + if not lib_url.endswith('/'): + lib_url += '/' + return dict( STATIC_URL=static_url, static_url=static_url, STORAGE_URL=storage_url, storage_url=storage_url, - CDN_URL=cdn_url, cdn_url=cdn_url) + CDN_URL=cdn_url, cdn_url=cdn_url, + LIB_URL=lib_url, lib_url=lib_url) @app.context_processor diff --git a/storymap/core/settings.py b/storymap/core/settings.py index c6c880aa..3a1b36fc 100644 --- a/storymap/core/settings.py +++ b/storymap/core/settings.py @@ -13,6 +13,14 @@ STATIC_URL = env['STATIC_URL'] CDN_URL = env['CDN_URL'] +# Base URL for the StoryMap viewer library (js + css) that the editor loads. +# Defaults to CDN_URL, so existing setups are unchanged. Set STORYMAP_LIB_URL +# in the environment to load the viewer from somewhere other than CDN_URL -- +# most usefully '/compiled/', which serves the local `npm run build` output +# from dist/ (via the /compiled/ route) so viewer changes can be tested in the +# editor before they are deployed to the CDN. +STORYMAP_LIB_URL = env.get('STORYMAP_LIB_URL') or CDN_URL + # Default database `pg` is setup for local execution via docker-compose where the # postgresql service host is set to `pg`. To run migrations, set the `PG_HOST` diff --git a/storymap/templates/base.html b/storymap/templates/base.html index 5e9e1a04..68296ded 100644 --- a/storymap/templates/base.html +++ b/storymap/templates/base.html @@ -7,8 +7,8 @@ {% include "_analytics.html" %} {% endblock head %} - - + + diff --git a/storymap/templates/edit.html b/storymap/templates/edit.html index 3dc3f3bf..cdb08e1d 100644 --- a/storymap/templates/edit.html +++ b/storymap/templates/edit.html @@ -15,13 +15,9 @@ - - - - - + + +