From 39778a7359c8d112a02c3860da82f7286119e41d Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:23:41 +0100 Subject: [PATCH] fix: refuse drive segments in manifest paths A drive is refused only at the start of a value, so a later segment such as ../C:x joins as a drive-relative path on Windows. Every manifest path now refuses a segment Windows reads as a drive and one ending with a colon. roms_directory, emulators_directory and the dests below them refuse any colon. Elsewhere a colon inside a name is kept where the host is not Windows: RetroDECK's ares reads bios/snes:st018.data.rom and bios/zxs:48.rom on Linux, three dests of install/retrodeck.json. A Windows host refuses them as alternate data streams. The roms root is also checked to stay below the BIOS folder's ancestor it climbs to. --- install.ps1 | 2 +- install.py | 54 ++++++++++++++++++++++++++---------- install.sh | 2 +- install/launchers/index.json | 16 +++++------ tests/test_install.py | 28 +++++++++++++++++++ 5 files changed, 78 insertions(+), 24 deletions(-) diff --git a/install.ps1 b/install.ps1 index 345233037..fcab5282b 100644 --- a/install.ps1 +++ b/install.ps1 @@ -16,7 +16,7 @@ # and Invoke-WebRequest throws on a cut transfer before the hash is read: # the site's build rewrites this line there (scripts/sitebootstrap.py). $mismatchAdvice = "The download was most likely cut short: try again. Within minutes of an update of this project the two files can come from caches of different ages. Waiting five minutes settles that." - $defaultInstallSha256 = "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd" + $defaultInstallSha256 = "e076488b9766904d7f859e0463e55214fed9386dcaaca362b04a371a820c6c7c" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index b95a04bbb..29335e500 100644 --- a/install.py +++ b/install.py @@ -50,7 +50,7 @@ import weakref import xml.etree.ElementTree as ET import zipfile from collections.abc import Callable, Collection, Iterable, Iterator, Mapping -from pathlib import Path, PurePosixPath +from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, BinaryIO, ContextManager, NamedTuple, NoReturn, TextIO # Built as C extensions that a Python compiled without their libraries @@ -3456,12 +3456,18 @@ def _read_limited_json( raise ValueError(f"invalid {label}: {exc}") from exc -def _path_characters(value: object, field: str) -> str: +def _path_characters(value: object, field: str, colons: bool = True) -> str: """The character rules every manifest-controlled relative path meets. No backslash, control character, empty segment, trailing slash or drive prefix, and at most 1024 characters: on Windows a backslash or a C: - segment leaves the folder the path is joined to. + segment leaves the folder the path is joined to. A segment that Windows + reads as a drive (C:x, any segment of one letter and a colon in front) + or that ends with a colon is refused wherever it stands, since a later + segment is joined as a drive-relative path. ``colons`` keeps a colon + inside a name: RetroDECK's ares reads bios/snes:st018.data.rom on Linux. + A Windows host refuses it all the same, as an alternate data stream of + the file before the colon. """ if not isinstance(value, str) or not value or len(value) > 1024: raise ValueError(f"invalid {field}") @@ -3476,12 +3482,20 @@ def _path_characters(value: object, field: str) -> str: # x]0;PWN.bin set the window title in the progress line, # and the file was written under that name. raise ValueError(f"unsafe {field}: {value!r}") + for part in value.split("/"): + if ":" not in part: + continue + if ( + PureWindowsPath(part).drive or part.endswith(":") + or not colons or os.name == "nt" + ): + raise ValueError(f"unsafe {field}: {value!r}") return value -def _safe_relative_path(value: object, field: str) -> PurePosixPath: +def _safe_relative_path(value: object, field: str, colons: bool = True) -> PurePosixPath: """Validate a manifest-controlled relative POSIX path.""" - path = PurePosixPath(_path_characters(value, field)) + path = PurePosixPath(_path_characters(value, field, colons)) if path.is_absolute() or any(part in ("", ".", "..") for part in path.parts): raise ValueError(f"unsafe {field}: {value!r}") return path @@ -3502,7 +3516,7 @@ def _roms_relation(value: object, field: str) -> tuple[int, PurePosixPath]: """ if value == ".": return 0, PurePosixPath() - path = PurePosixPath(_path_characters(value, field)) + path = PurePosixPath(_path_characters(value, field, colons=False)) parts = path.parts ups = next((i for i, part in enumerate(parts) if part != ".."), len(parts)) names = parts[ups:] @@ -3527,7 +3541,9 @@ def _roms_root(bios_path: Path, from_bios: str) -> Path | None: ancestor = bios_path if not ups else bios_path.parents[ups - 1] expected = ancestor.joinpath(*names.parts) joined = Path(os.path.normpath(bios_path.joinpath(*PurePosixPath(from_bios).parts))) - return expected if joined == Path(os.path.normpath(expected)) else None + if joined != Path(os.path.normpath(expected)) or not _lexically_under(joined, ancestor): + return None + return expected def _lexically_under(candidate: Path, root: Path) -> bool: @@ -3778,8 +3794,12 @@ def _check_emulators_directory(data: dict) -> None: for template in searched: if not isinstance(template, str) or template.count("{key}") != 1: raise ValueError("invalid emulators_directory.searched") - _safe_relative_path(template.replace("{key}", "key"), "emulators_directory.searched") - _safe_relative_path(layout["config"], "emulators_directory.config") + _safe_relative_path( + template.replace("{key}", "key"), "emulators_directory.searched", colons=False + ) + _safe_relative_path(layout["default"].replace("{key}", "key"), "emulators_directory.default", + colons=False) + _safe_relative_path(layout["config"], "emulators_directory.config", colons=False) def _check_save_directory(data: dict) -> None: @@ -3946,7 +3966,11 @@ def _check_entry( """Check one file entry, return its size and the largest copy it may fetch.""" if not isinstance(entry, dict): raise ValueError(f"{where} must be an object") - dest = str(_safe_relative_path(entry.get("dest"), f"{where}.dest")) + # Below a roms or emulator folder a colon has no use: refused outright. + dest = str(_safe_relative_path( + entry.get("dest"), f"{where}.dest", + colons=entry.get("root") not in ("roms", "emulators"), + )) key = _entry_key({**entry, "dest": dest}) if key in seen_destinations: raise ValueError(f"duplicate manifest destination: {dest}") @@ -5418,7 +5442,8 @@ def place_emulator_entries( layout = manifest.get("emulators_directory") or {} home = bios_path.parent config = ( - home.joinpath(*_safe_relative_path(layout["config"], "emulators_directory.config").parts) + home.joinpath(*_safe_relative_path( + layout["config"], "emulators_directory.config", colons=False).parts) if layout else None ) matches = bool(layout) and bios_path.name == layout.get("bios_folder") and config.is_file() @@ -5429,14 +5454,15 @@ def place_emulator_entries( if entry.get("root") != "emulators": kept.append(entry) continue - key = _safe_relative_path(str(entry["dest"]), "emulators dest").parts[0] + key = _safe_relative_path(str(entry["dest"]), "emulators dest", colons=False).parts[0] places = [ - home.joinpath(*_safe_relative_path(t.replace("{key}", key), "emulators place").parts) + home.joinpath(*_safe_relative_path( + t.replace("{key}", key), "emulators place", colons=False).parts) for t in layout.get("searched") or [] ] default = home.joinpath( *_safe_relative_path(layout.get("default", "").replace("{key}", key), - "emulators default").parts + "emulators default", colons=False).parts ) if matches else None if (matches and key.lower() not in moved and default is not None and all(_lexically_under(place, home) for place in [*places, default])): diff --git a/install.sh b/install.sh index a420e71fa..b5e0bc583 100755 --- a/install.sh +++ b/install.sh @@ -23,7 +23,7 @@ DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/ins # curl or wget stop on a cut transfer before the hash is read: the site's # build rewrites this line there (scripts/sitebootstrap.py). MISMATCH_ADVICE="The download was most likely cut short: try again. Within minutes of an update of this project the two files can come from caches of different ages. Waiting five minutes settles that." -DEFAULT_INSTALL_SHA256="e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd" +DEFAULT_INSTALL_SHA256="e076488b9766904d7f859e0463e55214fed9386dcaaca362b04a371a820c6c7c" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index 009cf11f1..bf070ce79 100644 --- a/install/launchers/index.json +++ b/install/launchers/index.json @@ -27,8 +27,8 @@ { "dest": "roms/ports/retrobios/install.py", "repo_path": "install.py", - "size": 1141574, - "sha256": "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd", + "size": 1142840, + "sha256": "e076488b9766904d7f859e0463e55214fed9386dcaaca362b04a371a820c6c7c", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 1195199, - "sha256": "f603c733b3b1d4f81006a9d1eab60865d0fc4a3e1427e3479794add9c2d1fe11" + "size": 1196465, + "sha256": "7ef6e947618ffe51b95823b28ce2a064c6eddf2b7fb77e2066f9c2d279176f03" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 1141574, - "sha256": "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd", + "size": 1142840, + "sha256": "e076488b9766904d7f859e0463e55214fed9386dcaaca362b04a371a820c6c7c", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 1141574, - "sha256": "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd", + "size": 1142840, + "sha256": "e076488b9766904d7f859e0463e55214fed9386dcaaca362b04a371a820c6c7c", "executable": false }, { diff --git a/tests/test_install.py b/tests/test_install.py index b4977f594..9ca17cabb 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1184,6 +1184,34 @@ class OuterRootsAreUntrusted(unittest.TestCase): with self.subTest(field=field, value=value), self.assertRaises(ValueError): install._check_emulators_directory({"emulators_directory": layout}) + def test_a_drive_or_colon_segment_is_refused_on_either_host(self): + from pathlib import PureWindowsPath + + bios = PureWindowsPath("C:/RetroBat/bios") + for host in ("posix", "nt"): + with unittest.mock.patch.object(install.os, "name", host): + for value in ("../C:x", "..\\x", "../x:", "../Roms:x"): + with self.subTest(host=host, from_bios=value), self.assertRaises(ValueError): + install._check_roms_directory( + {"roms_directory": {**self.ROMS, "from_bios": value}}) + for value in ("C:x/{key}", "x:/{key}", "emu:x/{key}"): + layout = {**self.EMULATORS, "searched": [value], "default": value} + with self.subTest(host=host, searched=value), self.assertRaises(ValueError): + install._check_emulators_directory({"emulators_directory": layout}) + for dest in ("bios/C:x.bin", "bios/x:", "a:b/c.bin"): + with self.subTest(host=host, dest=dest), self.assertRaises(ValueError): + install._safe_relative_path(dest, "dest") + with self.subTest(host=host), self.assertRaises(ValueError): + install._safe_relative_path("pico8/x:y", "dest", colons=False) + # Each of these leaves the folder on Windows once joined. + self.assertEqual(PureWindowsPath(bios, "..", "D:x"), PureWindowsPath("D:x")) + self.assertEqual(PureWindowsPath("C:x").drive, "C:") + # RetroDECK's ares names are kept where the host is not Windows. + with unittest.mock.patch.object(install.os, "name", "posix"): + install._safe_relative_path("bios/snes:st018.data.rom", "dest") + with unittest.mock.patch.object(install.os, "name", "nt"), self.assertRaises(ValueError): + install._safe_relative_path("bios/snes:st018.data.rom", "dest") + def test_an_emulator_folder_stays_below_the_platform_folder(self): with tempfile.TemporaryDirectory() as tmp: home = Path(tmp)