From bfc87c0427daca0d2fc2490e08ccdfc2f0e48a7b 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. (cherry picked from commit ddb8573e8daf0fcfda4422b6738a044197d7cfd6) --- 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 3a4d6ad27..d3c771927 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 7ed652576..3acfe96c3 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 @@ -740,8 +743,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 d4ebcbdda..bf959e266 100644 --- a/ts/editor/NoteEditor.svelte +++ b/ts/editor/NoteEditor.svelte @@ -132,7 +132,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; @@ -413,7 +413,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 886ad262d..4fa454795 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1648,13 +1648,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" @@ -2011,7 +2004,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.19.10" esbuild-sass-plugin: "npm:^2" @@ -3162,18 +3154,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"