From ddb8573e8daf0fcfda4422b6738a044197d7cfd6 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 23 Apr 2025 16:21:48 +1000 Subject: [PATCH] Use CSP to block inline JS content in editor (#3939) * Revert "Sanitize field content in editor" This reverts commit 1c156905f8f282397fc4514dff59efdf41ae645e. * Use CSP to block inline JS content in editor This blocks inline scripts, scripts in the media folder, and handlers like onclick in the editor. This is nicer than the previous solution - it doesn't make any permanent changes, and leaves other content like SVGs alone. Thanks to Nil Admirari for the suggestion. --- package.json | 1 - qt/aqt/mediasrv.py | 20 ++++++++++++++++---- ts/editor/NoteEditor.svelte | 3 +-- ts/lib/domlib/index.ts | 1 - ts/lib/domlib/sanitize.ts | 10 ---------- ts/licenses.json | 34 ++++++---------------------------- yarn.lock | 20 -------------------- 7 files changed, 23 insertions(+), 66 deletions(-) delete mode 100644 ts/lib/domlib/sanitize.ts diff --git a/package.json b/package.json index 4b1b184d1..b0795c918 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,6 @@ "bootstrap-icons": "^1.10.5", "codemirror": "^5.63.1", "d3": "^7.0.0", - "dompurify": "^3.2.5", "fabric": "^5.3.0", "hammerjs": "^2.0.8", "intl-pluralrules": "^2.0.0", diff --git a/qt/aqt/mediasrv.py b/qt/aqt/mediasrv.py index 67f3b4e94..f160b84a9 100644 --- a/qt/aqt/mediasrv.py +++ b/qt/aqt/mediasrv.py @@ -141,14 +141,17 @@ class MediaServer(threading.Thread): ) -> None: self._legacy_pages[id] = LegacyPage(html, context) + def get_page(self, id: int) -> LegacyPage | None: + return self._legacy_pages.get(id) + def get_page_html(self, id: int) -> str | None: - if page := self._legacy_pages.get(id): + if page := self.get_page(id): return page.html else: return None def get_page_context(self, id: int) -> PageContext | None: - if page := self._legacy_pages.get(id): + if page := self.get_page(id): return page.context else: return None @@ -742,8 +745,17 @@ def _handle_dynamic_request(req: DynamicRequest) -> Response: def legacy_page_data() -> Response: id = int(request.args["id"]) - if html := aqt.mw.mediaServer.get_page_html(id): - return Response(html, mimetype="text/html") + page = aqt.mw.mediaServer.get_page(id) + if page: + response = Response(page.html, mimetype="text/html") + # Prevent JS in field content from being executed in the editor, as it would + # have access to our internal API, and is a security risk. + if page.context == PageContext.EDITOR: + port = aqt.mw.mediaServer.getPort() + response.headers["Content-Security-Policy"] = ( + f"script-src http://127.0.0.1:{port}/_anki/" + ) + return response else: return _text_response(HTTPStatus.NOT_FOUND, "page not found") diff --git a/ts/editor/NoteEditor.svelte b/ts/editor/NoteEditor.svelte index a51ef4fe9..9bf66d858 100644 --- a/ts/editor/NoteEditor.svelte +++ b/ts/editor/NoteEditor.svelte @@ -133,7 +133,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html } for (const [index, [, fieldContent]] of fs.entries()) { - fieldStores[index].set(sanitize(fieldContent)); + fieldStores[index].set(fieldContent); } fieldNames = newFieldNames; @@ -424,7 +424,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html } from "../routes/image-occlusion/store"; import CollapseLabel from "./CollapseLabel.svelte"; import * as oldEditorAdapter from "./old-editor-adapter"; - import { sanitize } from "$lib/domlib"; $: isIOImageLoaded = false; $: ioImageLoadedStore.set(isIOImageLoaded); diff --git a/ts/lib/domlib/index.ts b/ts/lib/domlib/index.ts index 0834e0e77..27b21016d 100644 --- a/ts/lib/domlib/index.ts +++ b/ts/lib/domlib/index.ts @@ -5,5 +5,4 @@ export * from "./content-editable"; export * from "./location"; export * from "./move-nodes"; export * from "./place-caret"; -export * from "./sanitize"; export * from "./surround"; diff --git a/ts/lib/domlib/sanitize.ts b/ts/lib/domlib/sanitize.ts deleted file mode 100644 index d6ae4f049..000000000 --- a/ts/lib/domlib/sanitize.ts +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright: Ankitects Pty Ltd and contributors -// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html - -import DOMPurify from "dompurify"; - -export function sanitize(html: string): string { - // We need to treat the text as a document fragment, or a style tag - // at the start of input will be discarded. - return DOMPurify.sanitize(html, { FORCE_BODY: true }); -} diff --git a/ts/licenses.json b/ts/licenses.json index 58815d397..9d6f7d2c1 100644 --- a/ts/licenses.json +++ b/ts/licenses.json @@ -57,12 +57,6 @@ "path": "node_modules/@tootallnate/once", "licenseFile": "node_modules/@tootallnate/once/LICENSE" }, - "@types/trusted-types@2.0.7": { - "licenses": "MIT", - "repository": "https://github.com/DefinitelyTyped/DefinitelyTyped", - "path": "node_modules/@types/trusted-types", - "licenseFile": "node_modules/@types/trusted-types/LICENSE" - }, "abab@2.0.6": { "licenses": "BSD-3-Clause", "repository": "https://github.com/jsdom/abab", @@ -101,8 +95,8 @@ "repository": "https://github.com/TooTallNate/node-agent-base", "publisher": "Nathan Rajlich", "email": "nathan@tootallnate.net", - "path": "node_modules/http-proxy-agent/node_modules/agent-base", - "licenseFile": "node_modules/http-proxy-agent/node_modules/agent-base/README.md" + "path": "node_modules/agent-base", + "licenseFile": "node_modules/agent-base/README.md" }, "asynckit@0.4.0": { "licenses": "MIT", @@ -442,14 +436,6 @@ "path": "node_modules/domexception", "licenseFile": "node_modules/domexception/LICENSE.txt" }, - "dompurify@3.2.5": { - "licenses": "(MPL-2.0 OR Apache-2.0)", - "repository": "https://github.com/cure53/DOMPurify", - "publisher": "Dr.-Ing. Mario Heiderich, Cure53", - "email": "mario@cure53.de", - "path": "node_modules/dompurify", - "licenseFile": "node_modules/dompurify/LICENSE" - }, "empty-npm-package@1.0.0": { "licenses": "ISC", "path": "node_modules/canvas" @@ -586,14 +572,6 @@ "path": "node_modules/lodash-es", "licenseFile": "node_modules/lodash-es/LICENSE" }, - "lru-cache@10.4.3": { - "licenses": "ISC", - "repository": "https://github.com/isaacs/node-lru-cache", - "publisher": "Isaac Z. Schlueter", - "email": "i@izs.me", - "path": "node_modules/lru-cache", - "licenseFile": "node_modules/lru-cache/LICENSE" - }, "marked@5.1.2": { "licenses": "MIT", "repository": "https://github.com/markedjs/marked", @@ -790,16 +768,16 @@ "repository": "https://github.com/jsdom/whatwg-url", "publisher": "Sebastian Mayr", "email": "github@smayr.name", - "path": "node_modules/jsdom/node_modules/whatwg-url", - "licenseFile": "node_modules/jsdom/node_modules/whatwg-url/LICENSE.txt" + "path": "node_modules/whatwg-url", + "licenseFile": "node_modules/whatwg-url/LICENSE.txt" }, "whatwg-url@11.0.0": { "licenses": "MIT", "repository": "https://github.com/jsdom/whatwg-url", "publisher": "Sebastian Mayr", "email": "github@smayr.name", - "path": "node_modules/whatwg-url", - "licenseFile": "node_modules/whatwg-url/LICENSE.txt" + "path": "node_modules/data-urls/node_modules/whatwg-url", + "licenseFile": "node_modules/data-urls/node_modules/whatwg-url/LICENSE.txt" }, "ws@8.18.0": { "licenses": "MIT", diff --git a/yarn.lock b/yarn.lock index 76201b219..ebb11b5eb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1661,13 +1661,6 @@ __metadata: languageName: node linkType: hard -"@types/trusted-types@npm:^2.0.7": - version: 2.0.7 - resolution: "@types/trusted-types@npm:2.0.7" - checksum: 10c0/4c4855f10de7c6c135e0d32ce462419d8abbbc33713b31d294596c0cc34ae1fa6112a2f9da729c8f7a20707782b0d69da3b1f8df6645b0366d08825ca1522e0c - languageName: node - linkType: hard - "@typescript-eslint/eslint-plugin@npm:^5.60.1": version: 5.62.0 resolution: "@typescript-eslint/eslint-plugin@npm:5.62.0" @@ -2024,7 +2017,6 @@ __metadata: cross-env: "npm:^7.0.2" d3: "npm:^7.0.0" diff: "npm:^5.0.0" - dompurify: "npm:^3.2.5" dprint: "npm:^0.47.2" esbuild: "npm:^0.25.0" esbuild-sass-plugin: "npm:^2" @@ -3176,18 +3168,6 @@ __metadata: languageName: node linkType: hard -"dompurify@npm:^3.2.5": - version: 3.2.5 - resolution: "dompurify@npm:3.2.5" - dependencies: - "@types/trusted-types": "npm:^2.0.7" - dependenciesMeta: - "@types/trusted-types": - optional: true - checksum: 10c0/b564167cc588933ad2d25c185296716bdd7124e9d2a75dac76efea831bb22d1230ce5205a1ab6ce4c1010bb32ac35f7a5cb2dd16c78cbf382111f1228362aa59 - languageName: node - linkType: hard - "domutils@npm:^3.0.1": version: 3.1.0 resolution: "domutils@npm:3.1.0"