Harden file parsing

pyproject.toml

@@ -42,7 +42,7 @@ dependencies = [
   "winrt-Windows.Graphics.Imaging >=3.2.1; sys_platform == 'win32'",
   #
   # Linux-only dependencies
-  "pillow >=12.3.0; sys_platform == 'linux'", # Python 3.15 support # Necessary for ImageGrab.
+  "pillow >=13.0.0; sys_platform == 'linux'",
   "python-xlib >=0.33; sys_platform == 'linux'",
 ]
 [dependency-groups]
@@ -66,7 +66,7 @@ dev = [
   "types-pywin32 >=306.0.0.20240130; sys_platform == 'win32'",
   #
   # OS-specific, but installable on other OSes for convenient typing references
-  "pillow",
+  "pillow >=13.0.0",
   "pygrabber",
   "python-xlib",
   "typed-D3DShot[numpy]",
@@ -102,6 +102,7 @@ override-dependencies = [
 exclude-newer = "1 week"
 [tool.uv.exclude-newer-package]
 typed-D3DShot = false # I own it
+pillow = "6 day"
 
 ###
 # Development channels

res/design.ui

@@ -492,6 +492,9 @@
           <property name="text">
            <string/>
           </property>
+          <property name="textFormat">
+           <enum>Qt::TextFormat::PlainText</enum>
+          </property>
           <property name="alignment">
            <set>Qt::AlignmentFlag::AlignCenter</set>
           </property>
@@ -865,6 +868,9 @@
              <property name="text">
               <string>-</string>
              </property>
+             <property name="textFormat">
+              <enum>Qt::TextFormat::PlainText</enum>
+             </property>
              <property name="alignment">
               <set>Qt::AlignmentFlag::AlignCenter</set>
              </property>
@@ -921,6 +927,9 @@
           <property name="text">
            <string/>
           </property>
+          <property name="textFormat">
+           <enum>Qt::TextFormat::PlainText</enum>
+          </property>
           <property name="alignment">
            <set>Qt::AlignmentFlag::AlignCenter</set>
           </property>

src/AutoSplitImage.py

@@ -11,7 +11,14 @@
 
 import error_messages
 from compare import extract_and_compare_text, get_comparison_method_by_index
-from utils import MAXBYTE, TESSERACT_PATH, ColorChannel, imread, is_valid_image
+from utils import (
+    MAXBYTE,
+    TESSERACT_PATH,
+    ColorChannel,
+    imread,
+    is_supported_image_file,
+    is_valid_image,
+)
 
 if TYPE_CHECKING:
     from cv2.typing import MatLike
@@ -147,6 +154,9 @@ def __validate_ocr(self):
         )
 
     def __read_image_bytes(self, path: str):
+        if not is_supported_image_file(path):
+            error_messages.image_type(path)
+            return
         image = imread(path, cv2.IMREAD_UNCHANGED)
         if not is_valid_image(image):
             self.byte_array = None

src/capture_method/XcbCaptureMethod.py

@@ -63,6 +63,7 @@ def get_frame(self):
                 x + selection["width"],
                 y + selection["height"],
             ),
+            # Passing a non-None xdisplay also prevents pillow falling back to a screenshot tool
             xdisplay=self._xdisplay,
         )
         image = np.array(image)

src/error_messages.py

@@ -2,10 +2,12 @@
 
 from __future__ import annotations
 
+import html
 import os
 import signal
 import sys
 import traceback
+from collections.abc import Mapping
 from types import TracebackType
 from typing import TYPE_CHECKING, NoReturn
 
@@ -40,17 +42,28 @@ def _set_text_message(
     details: str = "",
     kill_button: str = "",
     accept_button: str = "",
+    *,
+    rich_text: bool = False,
 ):
+    """
+    Plain text by default, so dynamic values (paths, filenames, hotkeys, etc.)
+    can't inject HTML. When `rich_text` is needed (for links),
+    every dynamic value must be escaped with `html.escape`.
+    """
     # Also surface the error message in the logs
-    plain_message = QtGui.QTextDocumentFragment.fromHtml(message).toPlainText()
+    plain_message = (
+        QtGui.QTextDocumentFragment.fromHtml(message).toPlainText() if rich_text else message
+    )
     print(
         f"{plain_message}\n{details}\n" if details else f"{plain_message}",
         file=sys.stderr if details else None,
     )
 
     message_box = QtWidgets.QMessageBox()
     message_box.setWindowTitle("Error")
-    message_box.setTextFormat(QtCore.Qt.TextFormat.RichText)
+    message_box.setTextFormat(
+        QtCore.Qt.TextFormat.RichText if rich_text else QtCore.Qt.TextFormat.PlainText
+    )
     message_box.setText(message)
     # Button order is important for default focus
     if accept_button:
@@ -156,6 +169,26 @@ def invalid_hotkey(hotkey_name: str):
     _set_text_message(f"Invalid hotkey {hotkey_name!r}")
 
 
+def invalid_settings_hotkeys(hotkeys: Mapping[str, object]):
+    _set_text_message(
+        "The following hotkeys from the settings file are invalid and have been unset:\n"
+        + "\n".join(f"{name} = {value!r}" for name, value in hotkeys.items())
+        + "\nOnly a single key, optionally combined with modifiers (excluding Windows/Super), "
+        + "is allowed."
+    )
+
+
+def invalid_settings_network_directories(directories: Mapping[str, str]):
+    _set_text_message(
+        "The following folders from the settings file are network locations "
+        + "and have been unset:\n"
+        + "\n".join(f"{name} = {value!r}" for name, value in directories.items())
+        + "\nOn Windows, accessing a network location can automatically send "
+        + "your Windows credentials to that server. "
+        + "If you trust this location, map it to a drive letter instead."
+    )
+
+
 def no_settings_file_on_open():
     print(
         "No settings file found. "
@@ -189,7 +222,7 @@ def stdin_lost():
 def already_open():
     _set_text_message(
         "An instance of AutoSplit is already running."
-        + "<br/>Are you sure you want to open a another one?",
+        + "\nAre you sure you want to open a another one?",
         "",
         "Don't open",
         "Ignore",
@@ -222,7 +255,7 @@ def linux_uinput():
 
 
 def linux_wayland():
-    _set_text_message(WAYLAND_WARNING)
+    _set_text_message(WAYLAND_WARNING, rich_text=True)
 
 
 def exception_traceback(exception: BaseException, message: str = ""):
@@ -236,15 +269,17 @@ def exception_traceback(exception: BaseException, message: str = ""):
         message,
         "\n".join(traceback.format_exception(None, exception, exception.__traceback__)),
         "Close AutoSplit",
+        rich_text=True,
     )
 
 
 def tesseract_missing(ocr_split_file_path: str):
     _set_text_message(
-        f"{ocr_split_file_path!r} is an Optical Character Recognition split file "
+        f"{html.escape(repr(ocr_split_file_path))} is an Optical Character Recognition split file "
         + "but tesseract couldn't be found."
-        + f'\nPlease read <a href="https://github.com/{GITHUB_REPOSITORY}#install-tesseract">'
-        + f"github.com/{GITHUB_REPOSITORY}#install-tesseract</a> for installation instructions."
+        + f'<br/>Please read <a href="https://github.com/{GITHUB_REPOSITORY}#install-tesseract">'
+        + f"github.com/{GITHUB_REPOSITORY}#install-tesseract</a> for installation instructions.",
+        rich_text=True,
     )
 
 
@@ -255,7 +290,7 @@ def ocr_missing_key(ocr_split_file_path: str, missing_key: str):
 def wrong_ocr_values(ocr_split_file_path: str):
     _set_text_message(
         f"{ocr_split_file_path!r} has invalid values."
-        + "\nPlease make sure that `left &lt; right` and `top &lt; bottom`. "
+        + "\nPlease make sure that `left < right` and `top < bottom`. "
         + "Also check for negative values in the 'methods' or 'fps_limit' settings"
     )
 

src/hotkeys.py

@@ -2,14 +2,15 @@
 
 import sys
 from collections.abc import Callable
-from typing import TYPE_CHECKING, Literal, cast
+from typing import TYPE_CHECKING, Literal, TypeGuard, cast
 
 import keyboard
 import pyautogui
+from keyboard._canonical_names import normalize_name  # noqa: PLC2701
 from PySide6.QtTest import QTest
 
 import error_messages
-from utils import fire_and_forget, is_digit, try_input_device_access
+from utils import fire_and_forget, flatten, is_digit, try_input_device_access
 
 if sys.platform == "linux":
     import grp
@@ -282,10 +283,40 @@ def toggle_auto_reset_image():
     return getattr(autosplit, f"{hotkey}_signal").emit
 
 
-def is_valid_hotkey_name(hotkey_name: str):
-    return any(
-        key and not keyboard.is_modifier(keyboard.key_to_scan_codes(key)[0])
-        for key in hotkey_name.split("+")
+def is_valid_hotkey_name(hotkey_name: object) -> TypeGuard[str]:
+    """
+    A valid hotkey is a single regular key, optionally combined with modifiers.
+    This is the only kind of hotkey `__read_hotkey` can produce.
+
+    Hotkeys are also sent as keystrokes, so anything else must be rejected to prevent
+    a malicious profile from injecting arbitrary keystrokes (see `_send_hotkey`).
+    This includes key sequences ("a, b"), multiple regular keys and the Windows key.
+    """
+    if not isinstance(hotkey_name, str):
+        return False
+    keys = hotkey_name.split("+")
+    if any(
+        not key
+        # keyboard.send treats commas as a separator for a sequence of hotkeys
+        or ("," in key and key != ",")
+        # The Windows key triggers OS shortcuts (ie: Win+R opens the Run dialog).
+        # keyboard normalizes aliases ("win", "cmd", "command", ...) to "[left |right ]windows"
+        or normalize_name(key).endswith("windows")
+        for key in keys
+    ):
+        return False
+
+    if KEYBOARD_GROUPS_ISSUE or KEYBOARD_UINPUT_ISSUE:
+        # Can't map to scan codes without keyboard access, fallback to canonical names
+        return sum(not keyboard.is_modifier(normalize_name(key)) for key in keys) == 1
+
+    # Scan codes also support localized key names
+    keys_scan_codes = [keyboard.key_to_scan_codes(key, error_if_missing=False) for key in keys]
+    windows_scan_codes = set(keyboard.key_to_scan_codes("windows", error_if_missing=False))
+    return (
+        all(keys_scan_codes)  # Unknown keys have no scan codes
+        and windows_scan_codes.isdisjoint(flatten(keys_scan_codes))
+        and sum(not keyboard.is_modifier(scan_codes[0]) for scan_codes in keys_scan_codes) == 1
     )
 
 

src/region_selection.py

@@ -15,10 +15,12 @@
 from utils import (
     ALPHA_CHANNEL_COUNT,
     MAXBYTE,
+    SUPPORTED_IMREAD_FORMATS,
     ImageShape,
     auto_split_directory,
     get_window_bounds,
     imread,
+    is_supported_image_file,
     is_valid_hwnd,
     is_valid_image,
 )
@@ -37,27 +39,13 @@
 GNOME_DESKTOP_ICONS_EXTENSION = "@!0,0;BDHF"
 ALIGN_REGION_THRESHOLD = 0.9
 BORDER_WIDTH = 2
-SUPPORTED_IMREAD_FORMATS = (
-    ("Windows bitmaps", "*.bmp *.dib"),
-    ("JPEG files", "*.jpeg *.jpg *.jpe"),
-    ("JPEG 2000 files", "*.jp2"),
-    ("Portable Network Graphics", "*.png"),
-    ("WebP", "*.webp"),
-    ("AVIF", "*.avif"),
-    ("Portable image format", "*.pbm *.pgm *.ppm *.pxm *.pnm"),
-    ("PFM files", "*.pfm"),
-    ("Sun rasters", "*.sr *.ras"),
-    ("TIFF files", "*.tiff *.tif"),
-    ("OpenEXR Image files", "*.exr"),
-    ("Radiance HDR", "*.hdr *.pic"),
-)
-"""https://docs.opencv.org/4.8.0/d4/da8/group__imgcodecs.html#imread"""
 IMREAD_EXT_FILTER = (
     "All Files ("
-    + " ".join(f"{extensions}" for _, extensions in SUPPORTED_IMREAD_FORMATS)
+    + " ".join(f"{extensions}" for _, extensions, _ in SUPPORTED_IMREAD_FORMATS)
     + ");;"
     + ";;".join(
-        f"{imread_format} ({extensions})" for imread_format, extensions in SUPPORTED_IMREAD_FORMATS
+        f"{imread_format} ({extensions})"
+        for imread_format, extensions, _ in SUPPORTED_IMREAD_FORMATS
     )
 )
 
@@ -218,6 +206,9 @@ def align_region(autosplit: AutoSplit):
     if not template_filename:
         return
 
+    if not is_supported_image_file(template_filename):
+        error_messages.image_validity()
+        return
     template = imread(template_filename, cv2.IMREAD_UNCHANGED)
     # Captures are standardized to BGR, so drop the template's alpha channel if present
     if template.shape[ImageShape.Channels] == ALPHA_CHANNEL_COUNT: