Harden access to internal API (#3925)

* Sanitize field content in editor

The editor already strips script tags from fields, but was allowing
through Javascript in things like onclick handlers. We block this now,
as the editor context has access to internal APIs that we don't want to
expose to untrusted third-party code.

* Require an auth token for API access

We were previously inspecting the referrer, but that is spoofable,
and doesn't guard against other processes on the machine.

To accomplish this, we use a request interceptor to automatically
add an auth token to webviews with the right context. Some related
changes were required:

- We avoid storing _page, which was leading to leaks & warning on exit
- At webview creation (or set_kind() invocation), we assign either
an authenticated or unauthenticated web profile.
- Some of our screens initialize the AnkiWebView when calling, e.g.,
aqt.forms.stats.Ui_Dialog(). They then immediately call .set_kind().
This reveals a race condition in our DOM handling code: the webview
initialization creates an empty page with the injected script, which
causes a domDone signal to be sent back. This signal arrives after
we've created another page with .set_kind(), causing our code to think
the DOM is ready when it's not. Then when we try to inject the dynamic
styling, we get an error, as the DOM is not ready yet. In the absence
of better solutions, I've added a hack to set_kind() to deal with this
for now.

* Provide AnkiWebPage init defaults for existing add-on callers

* Inject bridge script when profile set-up skipped

Some add-ons fully override AnkiWebPage.__init__ and thus depend on _setupBridge injecting the JS bridge script.

With this change we account for these cases, while giving add-ons the opportunity to look for solutions that do not require overriding AnkiWebPage.__init__ completely.

* Add some missed pages/endpoints (thanks to iamllama)

* Avoid sending API key for remote resources

Thanks to Abdo for the report

---------

Co-authored-by: Aristotelis P <201596065+aps-amboss@users.noreply.github.com>

package.json

@@ -69,6 +69,7 @@
         "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",

qt/aqt/mediasrv.py

@@ -7,7 +7,9 @@ import enum
 import logging
 import mimetypes
 import os
+import random
 import re
+import string
 import sys
 import threading
 import traceback
@@ -700,7 +702,6 @@ def _extract_collection_post_request(path: str) -> DynamicRequest | NotFound:
 def _check_dynamic_request_permissions():
     if request.method == "GET":
         return
-    context = _extract_page_context()
 
     def warn() -> None:
         show_warning(
@@ -712,24 +713,17 @@ def _check_dynamic_request_permissions():
         aqt.mw.taskman.run_on_main(warn)
         abort(403)
 
-    if context in [
+    # does page have access to entire API?
-        PageContext.NON_LEGACY_PAGE,
+    if _have_api_access():
-        PageContext.EDITOR,
+        return
-        PageContext.ADDON_PAGE,
+
-        PageContext.DECK_OPTIONS,
+    # whitelisted API endpoints for reviewer/previewer
-    ]:
+    if request.path in (
-        pass
-    elif context == PageContext.REVIEWER and request.path in (
         "/_anki/getSchedulingStatesWithContext",
         "/_anki/setSchedulingStates",
         "/_anki/i18nResources",
+        "/_anki/congratsInfo",
     ):
-        # reviewer is only allowed to access custom study methods
-        pass
-    elif (
-        context == PageContext.PREVIEWER or context == PageContext.CARD_LAYOUT
-    ) and request.path == "/_anki/i18nResources":
-        # previewers are only allowed to access i18n resources
         pass
     else:
         # other legacy pages may contain third-party JS, so we do not
@@ -754,23 +748,11 @@ def legacy_page_data() -> Response:
         return _text_response(HTTPStatus.NOT_FOUND, "page not found")
 
 
-def _extract_page_context() -> PageContext:
+_APIKEY = "".join(random.choices(string.ascii_letters + string.digits, k=32))
-    "Get context based on referer header."
-    from urllib.parse import parse_qs, urlparse
 
-    referer = urlparse(request.headers.get("Referer", ""))
+
-    if referer.path.startswith("/_anki/pages/") or is_sveltekit_page(referer.path[1:]):
+def _have_api_access() -> bool:
-        return PageContext.NON_LEGACY_PAGE
+    return request.headers.get("Authorization") == f"Bearer {_APIKEY}"
-    elif referer.path == "/_anki/legacyPageData":
-        query_params = parse_qs(referer.query)
-        query_id = query_params.get("id")
-        if not query_id:
-            return PageContext.UNKNOWN
-        id = int(query_id[0])
-        page_context = aqt.mw.mediaServer.get_page_context(id)
-        return page_context if page_context else PageContext.UNKNOWN
-    else:
-        return PageContext.UNKNOWN
 
 
 # this currently only handles a single method; in the future, idempotent

qt/aqt/sound.py

@@ -141,7 +141,7 @@ class AVPlayer:
     # audio be stopped?
     interrupt_current_audio = True
     # caller key for the current playback (optional)
-    current_caller = None
+    current_caller: Any = None
     # whether the last call to play_file_with_caller interrupted another
     current_caller_interrupted = False
 
@@ -167,7 +167,7 @@ class AVPlayer:
         self._enqueued = []
         self._stop_if_playing()
 
-    def stop_and_clear_queue_if_caller(self, caller) -> None:
+    def stop_and_clear_queue_if_caller(self, caller: Any) -> None:
         if caller == self.current_caller:
             self.stop_and_clear_queue()
 
@@ -179,7 +179,7 @@ class AVPlayer:
     def play_file(self, filename: str) -> None:
         self.play_tags([SoundOrVideoTag(filename=filename)])
 
-    def play_file_with_caller(self, filename: str, caller) -> None:
+    def play_file_with_caller(self, filename: str, caller: Any) -> None:
         if self.current_caller:
             self.current_caller_interrupted = True
         self.current_caller = caller

qt/aqt/webview.py

@@ -28,21 +28,154 @@ serverbaseurl = re.compile(r"^.+:\/\/[^\/]+")
 if TYPE_CHECKING:
     from aqt.mediasrv import PageContext
 
-
-# Page for debug messages
-##########################################################################
-
 BridgeCommandHandler = Callable[[str], Any]
 
 
+class AnkiWebViewKind(Enum):
+    """Enum registry of all web views managed by Anki
+
+    The value of each entry corresponds to the web view's title.
+
+    When introducing a new web view, please add it to the registry below.
+    """
+
+    DEFAULT = "default"
+    MAIN = "main webview"
+    TOP_TOOLBAR = "top toolbar"
+    BOTTOM_TOOLBAR = "bottom toolbar"
+    DECK_OPTIONS = "deck options"
+    EDITOR = "editor"
+    LEGACY_DECK_STATS = "legacy deck stats"
+    DECK_STATS = "deck stats"
+    PREVIEWER = "previewer"
+    CHANGE_NOTETYPE = "change notetype"
+    CARD_LAYOUT = "card layout"
+    BROWSER_CARD_INFO = "browser card info"
+    IMPORT_CSV = "csv import"
+    EMPTY_CARDS = "empty cards"
+    FIND_DUPLICATES = "find duplicates"
+    FIELDS = "fields"
+    IMPORT_LOG = "import log"
+    IMPORT_ANKI_PACKAGE = "anki package import"
+
+
+class AuthInterceptor(QWebEngineUrlRequestInterceptor):
+    _api_enabled = False
+
+    def __init__(self, parent: QObject | None = None, api_enabled: bool = False):
+        super().__init__(parent)
+        self._api_enabled = api_enabled
+
+    def interceptRequest(self, info):
+        from aqt.mediasrv import _APIKEY
+
+        if self._api_enabled and info.requestUrl().host() == "127.0.0.1":
+            info.setHttpHeader(b"Authorization", f"Bearer {_APIKEY}".encode("utf-8"))
+
+
+def _create_bridge_script() -> QWebEngineScript:
+    qwebchannel = ":/qtwebchannel/qwebchannel.js"
+    jsfile = QFile(qwebchannel)
+    if not jsfile.open(QIODevice.OpenModeFlag.ReadOnly):
+        print(f"Error opening '{qwebchannel}': {jsfile.error()}", file=sys.stderr)
+    jstext = bytes(cast(bytes, jsfile.readAll())).decode("utf-8")
+    jsfile.close()
+
+    script = QWebEngineScript()
+    script.setSourceCode(
+        jstext
+        + """
+        var pycmd, bridgeCommand;
+        new QWebChannel(qt.webChannelTransport, function(channel) {
+            bridgeCommand = pycmd = function (arg, cb) {
+                var resultCB = function (res) {
+                    // pass result back to user-provided callback
+                    if (cb) {
+                        cb(JSON.parse(res));
+                    }
+                }
+            
+                channel.objects.py.cmd(arg, resultCB);
+                return false;                   
+            }
+            pycmd("domDone");
+        });
+    """
+    )
+    script.setWorldId(QWebEngineScript.ScriptWorldId.MainWorld)
+    script.setInjectionPoint(QWebEngineScript.InjectionPoint.DocumentReady)
+    script.setRunsOnSubFrames(False)
+
+    return script
+
+
+_bridge_script = _create_bridge_script()
+
+_profile_with_api_access: QWebEngineProfile | None = None
+_profile_without_api_access: QWebEngineProfile | None = None
+
+
 class AnkiWebPage(QWebEnginePage):
-    def __init__(self, onBridgeCmd: BridgeCommandHandler) -> None:
+    def __init__(
-        QWebEnginePage.__init__(self)
+        self,
+        onBridgeCmd: BridgeCommandHandler,
+        kind: AnkiWebViewKind = AnkiWebViewKind.DEFAULT,
+        parent: QObject | None = None,
+    ) -> None:
+        profile = self._profileForPage(kind)
+        self._inject_user_script(profile, _bridge_script)
+        QWebEnginePage.__init__(self, profile, parent)
         self._onBridgeCmd = onBridgeCmd
+        self._kind = kind
         self._setupBridge()
         self.open_links_externally = True
 
+    def _profileForPage(self, kind: AnkiWebViewKind) -> QWebEngineProfile:
+        have_api_access = kind in (
+            AnkiWebViewKind.DECK_OPTIONS,
+            AnkiWebViewKind.EDITOR,
+            AnkiWebViewKind.DECK_STATS,
+            AnkiWebViewKind.CHANGE_NOTETYPE,
+            AnkiWebViewKind.BROWSER_CARD_INFO,
+            AnkiWebViewKind.IMPORT_ANKI_PACKAGE,
+            AnkiWebViewKind.IMPORT_CSV,
+            AnkiWebViewKind.IMPORT_LOG,
+        )
+
+        global _profile_with_api_access, _profile_without_api_access
+
+        # Use cached profile if available
+        if have_api_access and _profile_with_api_access is not None:
+            return _profile_with_api_access
+        elif not have_api_access and _profile_without_api_access is not None:
+            return _profile_without_api_access
+
+        # Create a new profile if not cached
+        profile = QWebEngineProfile()
+
+        interceptor = AuthInterceptor(profile, api_enabled=have_api_access)
+        profile.setUrlRequestInterceptor(interceptor)
+        if have_api_access:
+            _profile_with_api_access = profile
+        else:
+            _profile_without_api_access = profile
+
+        return profile
+
     def _setupBridge(self) -> None:
+        # Add-on compatibility: For existing add-on callers that override the init
+        # and invoke _setupBridge directly (e.g. in order to use a custom web profile),
+        # we need to ensure that the bridge script is injected into the profile scripts,
+        # if it has yet to be injected.
+        profile = self.profile()
+        assert profile is not None
+        scripts = profile.scripts()
+        assert scripts is not None
+
+        if not scripts.contains(_bridge_script):
+            print("add-on callers should not call _setupBridge directly")
+            self._inject_user_script(profile, _bridge_script)
+
         class Bridge(QObject):
             def __init__(self, bridge_handler: Callable[[str], Any]) -> None:
                 super().__init__()
@@ -58,40 +191,9 @@ class AnkiWebPage(QWebEnginePage):
         self._channel.registerObject("py", self._bridge)
         self.setWebChannel(self._channel)
 
-        qwebchannel = ":/qtwebchannel/qwebchannel.js"
+    def _inject_user_script(
-        jsfile = QFile(qwebchannel)
+        self, profile: QWebEngineProfile, script: QWebEngineScript
-        if not jsfile.open(QIODevice.OpenModeFlag.ReadOnly):
+    ) -> None:
-            print(f"Error opening '{qwebchannel}': {jsfile.error()}", file=sys.stderr)
-        jstext = bytes(cast(bytes, jsfile.readAll())).decode("utf-8")
-        jsfile.close()
-
-        script = QWebEngineScript()
-        script.setSourceCode(
-            jstext
-            + """
-            var pycmd, bridgeCommand;
-            new QWebChannel(qt.webChannelTransport, function(channel) {
-                bridgeCommand = pycmd = function (arg, cb) {
-                    var resultCB = function (res) {
-                        // pass result back to user-provided callback
-                        if (cb) {
-                            cb(JSON.parse(res));
-                        }
-                    }
-                
-                    channel.objects.py.cmd(arg, resultCB);
-                    return false;                   
-                }
-                pycmd("domDone");
-            });
-        """
-        )
-        script.setWorldId(QWebEngineScript.ScriptWorldId.MainWorld)
-        script.setInjectionPoint(QWebEngineScript.InjectionPoint.DocumentReady)
-        script.setRunsOnSubFrames(False)
-
-        profile = self.profile()
-        assert profile is not None
         scripts = profile.scripts()
         assert scripts is not None
         scripts.insert(script)
@@ -247,34 +349,6 @@ class WebContent:
 ##########################################################################
 
 
-class AnkiWebViewKind(Enum):
-    """Enum registry of all web views managed by Anki
-
-    The value of each entry corresponds to the web view's title.
-
-    When introducing a new web view, please add it to the registry below.
-    """
-
-    DEFAULT = "default"
-    MAIN = "main webview"
-    TOP_TOOLBAR = "top toolbar"
-    BOTTOM_TOOLBAR = "bottom toolbar"
-    DECK_OPTIONS = "deck options"
-    EDITOR = "editor"
-    LEGACY_DECK_STATS = "legacy deck stats"
-    DECK_STATS = "deck stats"
-    PREVIEWER = "previewer"
-    CHANGE_NOTETYPE = "change notetype"
-    CARD_LAYOUT = "card layout"
-    BROWSER_CARD_INFO = "browser card info"
-    IMPORT_CSV = "csv import"
-    EMPTY_CARDS = "empty cards"
-    FIND_DUPLICATES = "find duplicates"
-    FIELDS = "fields"
-    IMPORT_LOG = "import log"
-    IMPORT_ANKI_PACKAGE = "anki package import"
-
-
 class AnkiWebView(QWebEngineView):
     allow_drops = False
     _kind: AnkiWebViewKind
@@ -287,11 +361,8 @@ class AnkiWebView(QWebEngineView):
     ) -> None:
         QWebEngineView.__init__(self, parent=parent)
         self.set_kind(kind)
-        if title:
-            self.set_title(title)
-        self._page = AnkiWebPage(self._onBridgeCmd)
         # reduce flicker
-        self._page.setBackgroundColor(theme_manager.qcolor(colors.CANVAS))
+        self.page().setBackgroundColor(theme_manager.qcolor(colors.CANVAS))
 
         # in new code, use .set_bridge_command() instead of setting this directly
         self.onBridgeCmd: Callable[[str], Any] = self.defaultOnBridgeCmd
@@ -299,7 +370,6 @@ class AnkiWebView(QWebEngineView):
         self._domDone = True
         self._pendingActions: list[tuple[str, Sequence[Any]]] = []
         self.requiresCol = True
-        self.setPage(self._page)
         self._disable_zoom = False
 
         self.resetHandlers()
@@ -323,6 +393,15 @@ class AnkiWebView(QWebEngineView):
     def set_kind(self, kind: AnkiWebViewKind) -> None:
         self._kind = kind
         self.set_title(kind.value)
+        # this is an ugly hack to avoid breakages caused by
+        # creating a default webview then immediately calling set_kind, which results
+        # in the creation of two pages, and the second fails as the domDone
+        # signal from the first one is received
+        if kind != AnkiWebViewKind.DEFAULT:
+            self.setPage(AnkiWebPage(self._onBridgeCmd, kind, self))
+
+    def page(self) -> AnkiWebPage:
+        return cast(AnkiWebPage, super().page())
 
     @property
     def kind(self) -> AnkiWebViewKind:
@@ -360,7 +439,7 @@ class AnkiWebView(QWebEngineView):
         return False
 
     def set_open_links_externally(self, enable: bool) -> None:
-        self._page.open_links_externally = enable
+        self.page().open_links_externally = enable
 
     def onEsc(self) -> None:
         w = self.parent()
@@ -825,7 +904,7 @@ html {{ {font} }}
         Must be done on Windows prior to changing current working directory."""
         self.requiresCol = False
         self._domReady = False
-        self._page.setContent(cast(QByteArray, bytes("", "ascii")))
+        self.page().setContent(cast(QByteArray, bytes("", "ascii")))
 
     def cleanup(self) -> None:
         try:
@@ -839,14 +918,14 @@ html {{ {font} }}
         # defer page cleanup so that in-flight requests have a chance to complete first
         # https://forums.ankiweb.net/t/error-when-exiting-browsing-when-the-software-is-installed-in-the-path-c-program-files-anki/38363
         mw.progress.single_shot(5000, lambda: mw.mediaServer.clear_page_html(id(self)))
-        self._page.deleteLater()
+        self.page().deleteLater()
 
     def on_theme_did_change(self) -> None:
         # avoid flashes if page reloaded
-        self._page.setBackgroundColor(theme_manager.qcolor(colors.CANVAS))
+        self.page().setBackgroundColor(theme_manager.qcolor(colors.CANVAS))
         if hasattr(QWebEngineSettings.WebAttribute, "ForceDarkMode"):
             force_dark_mode = getattr(QWebEngineSettings.WebAttribute, "ForceDarkMode")
-            page_settings = self._page.settings()
+            page_settings = self.page().settings()
             if page_settings is not None:
                 page_settings.setAttribute(
                     force_dark_mode,

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(fieldContent);
+            fieldStores[index].set(sanitize(fieldContent));
         }
 
         fieldNames = newFieldNames;
@@ -424,6 +424,7 @@ 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);

ts/lib/domlib/index.ts

@@ -5,4 +5,5 @@ export * from "./content-editable";
 export * from "./location";
 export * from "./move-nodes";
 export * from "./place-caret";
+export * from "./sanitize";
 export * from "./surround";

ts/lib/domlib/sanitize.ts

@@ -0,0 +1,10 @@
+// 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 });
+}

ts/licenses.json

@@ -57,6 +57,12 @@
         "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",
@@ -436,6 +442,14 @@
         "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"

yarn.lock

@@ -1661,6 +1661,13 @@ __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"
@@ -2017,6 +2024,7 @@ __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"
@@ -3168,6 +3176,18 @@ __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"