From 10ca8523b51e7646dd01a7ac7d40492f89b50461 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:13:47 +0100 Subject: [PATCH] fix: check manifest roms roots like paths roms_directory.from_bios and the emulators_directory folders come from a downloaded list. from_bios now meets the character rules of every manifest path (shared with _safe_relative_path: no backslash, drive, empty segment, trailing slash or control), climbs at most two folders (../../Roms, the deepest the rom folders files record) before plain names, and its joined form must equal the BIOS folder's ancestor at that depth: "." is the BIOS folder, null leaves the files out. Every emulator folder the installer builds goes through _safe_relative_path and stays below the folder holding the BIOS folder. --- install.ps1 | 2 +- install.py | 105 ++++++++++++++++++++++----- install.sh | 2 +- install/launchers/index.json | 16 ++-- schemas/install-manifest.schema.json | 5 +- tests/test_install.py | 69 ++++++++++++++++++ 6 files changed, 170 insertions(+), 29 deletions(-) diff --git a/install.ps1 b/install.ps1 index a7b1581f4..345233037 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 = "67c84f81ad5a452a73ae6b6f36bbdf593a48728754dbb0f77296c381012f2fd2" + $defaultInstallSha256 = "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 6c765d906..b95a04bbb 100644 --- a/install.py +++ b/install.py @@ -3456,8 +3456,13 @@ def _read_limited_json( raise ValueError(f"invalid {label}: {exc}") from exc -def _safe_relative_path(value: object, field: str) -> PurePosixPath: - """Validate a manifest-controlled relative POSIX path.""" +def _path_characters(value: object, field: str) -> 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. + """ if not isinstance(value, str) or not value or len(value) > 1024: raise ValueError(f"invalid {field}") if ( @@ -3471,12 +3476,69 @@ def _safe_relative_path(value: object, field: str) -> PurePosixPath: # 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}") - path = PurePosixPath(value) + return value + + +def _safe_relative_path(value: object, field: str) -> PurePosixPath: + """Validate a manifest-controlled relative POSIX path.""" + path = PurePosixPath(_path_characters(value, field)) if path.is_absolute() or any(part in ("", ".", "..") for part in path.parts): raise ValueError(f"unsafe {field}: {value!r}") return path +# The most folders a roms root climbs from the BIOS folder: two, as the rom +# folders files of MinUI, NextUI, MyMinUI (../../Roms) and muOS (../../ROMS) +# record it (platforms/romfolders/*.yml, roms_root.from_bios). +MAX_ROMS_UPS = 2 + + +def _roms_relation(value: object, field: str) -> tuple[int, PurePosixPath]: + """A roms root relative to the BIOS folder: (folders up, names down). + + "." is the BIOS folder itself. Any other value climbs one to + MAX_ROMS_UPS folders, then names plain folders: nothing climbs again + once it went down, and nothing goes down without climbing first. + """ + if value == ".": + return 0, PurePosixPath() + path = PurePosixPath(_path_characters(value, field)) + parts = path.parts + ups = next((i for i, part in enumerate(parts) if part != ".."), len(parts)) + names = parts[ups:] + if ( + path.is_absolute() or not 1 <= ups <= MAX_ROMS_UPS + or any(part in ("", ".", "..") for part in names) + ): + raise ValueError(f"unsafe {field}: {value!r}") + return ups, PurePosixPath(*names) + + +def _roms_root(bios_path: Path, from_bios: str) -> Path | None: + """The roms root of a BIOS folder, on the host's own path type. + + The BIOS folder's ancestor at the depth the relation climbs, then the + named folders below it. None when the joined and normalized path is + anything else. + """ + ups, names = _roms_relation(from_bios, "roms_directory.from_bios") + if ups > len(bios_path.parents): + return 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 + + +def _lexically_under(candidate: Path, root: Path) -> bool: + """Whether a path built from checked parts stays below *root*.""" + try: + Path(os.path.normpath(candidate)).relative_to(Path(os.path.normpath(root))) + except ValueError: + return False + return True + + def _within(candidate: Path, root: Path) -> bool: """Whether *candidate* stays under *root* once both are resolved.""" try: @@ -3689,15 +3751,9 @@ def _check_roms_directory(data: dict) -> None: not isinstance(layout, dict) or set(layout) != {"bios_folder", "from_bios"} or len(_safe_relative_path(layout["bios_folder"], "roms_directory.bios_folder").parts) != 1 - or not isinstance(layout["from_bios"], str) ): raise ValueError("invalid roms_directory") - # The roms root relative to the BIOS folder: up some levels, then down - # into named folders. Nothing climbs again once it went down. - parts = PurePosixPath(layout["from_bios"]).parts - ups = next((i for i, part in enumerate(parts) if part != ".."), len(parts)) - if not ups or any(part in ("..", ".", "/") or _CONTROLS.search(part) for part in parts[ups:]): - raise ValueError("invalid roms_directory.from_bios") + _roms_relation(layout["from_bios"], "roms_directory.from_bios") def _check_emulators_directory(data: dict) -> None: @@ -5311,8 +5367,12 @@ def place_roms_entries( if not below: return files, [] layout = manifest.get("roms_directory") or {} - if layout and bios_path.name == layout.get("bios_folder"): - _ROMS_DIRS[bios_path] = Path(os.path.normpath(bios_path / layout["from_bios"])) + roms = ( + _roms_root(bios_path, layout["from_bios"]) + if layout and bios_path.name == layout.get("bios_folder") else None + ) + if roms is not None: + _ROMS_DIRS[bios_path] = roms return files, [] if say: print(f" {_plural(len(below), 'file')} read from {display}'s roms folder left out: " @@ -5357,7 +5417,10 @@ def place_emulator_entries( return files, [] layout = manifest.get("emulators_directory") or {} home = bios_path.parent - config = home / layout["config"] if layout else None + config = ( + home.joinpath(*_safe_relative_path(layout["config"], "emulators_directory.config").parts) + if layout else None + ) matches = bool(layout) and bios_path.name == layout.get("bios_folder") and config.is_file() moved = _config_keys(config) if matches else set() kept: list[dict] = [] @@ -5366,11 +5429,19 @@ def place_emulator_entries( if entry.get("root") != "emulators": kept.append(entry) continue - key = str(entry["dest"]).split("/", 1)[0] - if matches and key.lower() not in moved: - places = [home / t.replace("{key}", key) for t in layout["searched"]] + key = _safe_relative_path(str(entry["dest"]), "emulators dest").parts[0] + places = [ + home.joinpath(*_safe_relative_path(t.replace("{key}", key), "emulators place").parts) + for t in layout.get("searched") or [] + ] + default = home.joinpath( + *_safe_relative_path(layout.get("default", "").replace("{key}", key), + "emulators default").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])): found = next((place for place in places if place.is_dir()), None) - _EMULATOR_DIRS[(bios_path, key)] = found or home / layout["default"].replace("{key}", key) + _EMULATOR_DIRS[(bios_path, key)] = found or default kept.append(entry) continue unplaced.append({ diff --git a/install.sh b/install.sh index 8087b7e89..a420e71fa 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="67c84f81ad5a452a73ae6b6f36bbdf593a48728754dbb0f77296c381012f2fd2" +DEFAULT_INSTALL_SHA256="e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index db1837125..009cf11f1 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": 1138987, - "sha256": "67c84f81ad5a452a73ae6b6f36bbdf593a48728754dbb0f77296c381012f2fd2", + "size": 1141574, + "sha256": "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 1192612, - "sha256": "90e6c3ec8b3eae2d9f8f1b07874c083e4d555fb325590999ef4690fb67125426" + "size": 1195199, + "sha256": "f603c733b3b1d4f81006a9d1eab60865d0fc4a3e1427e3479794add9c2d1fe11" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 1138987, - "sha256": "67c84f81ad5a452a73ae6b6f36bbdf593a48728754dbb0f77296c381012f2fd2", + "size": 1141574, + "sha256": "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 1138987, - "sha256": "67c84f81ad5a452a73ae6b6f36bbdf593a48728754dbb0f77296c381012f2fd2", + "size": 1141574, + "sha256": "e69e2fb4572b085b9aa33fcdc697a6cc4216bed1feb5ea04d9fdf9dfc576b0fd", "executable": false }, { diff --git a/schemas/install-manifest.schema.json b/schemas/install-manifest.schema.json index c34a87586..76d7512f5 100644 --- a/schemas/install-manifest.schema.json +++ b/schemas/install-manifest.schema.json @@ -528,7 +528,7 @@ } }, "roms_directory": { - "description": "Where the platform keeps its roms root, present when an entry carries root: roms. from_bios is that root relative to the BIOS folder, as platforms/romfolders/.yml records it from the OS's own source (roms_root.from_bios). It is followed only from a BIOS folder named bios_folder (the platform's base_destination); any other leaves root: roms entries out (no_roms_directory).", + "description": "Where the platform keeps its roms root, present when an entry carries root: roms. from_bios is that root relative to the BIOS folder (\".\" for the folder itself, else one or two \"..\" then plain names), as platforms/romfolders/.yml records it from the OS's own source (roms_root.from_bios). It is followed only from a BIOS folder named bios_folder (the platform's base_destination); any other leaves root: roms entries out (no_roms_directory).", "type": "object", "additionalProperties": false, "required": [ @@ -543,7 +543,8 @@ }, "from_bios": { "type": "string", - "pattern": "^\\.\\.(/\\.\\.)*(/[^/.][^/]*)*$" + "pattern": "^(\\.|\\.\\.(/\\.\\.)?(/[^/\\\\:.][^/\\\\:]*)*)$", + "maxLength": 1024 } } }, diff --git a/tests/test_install.py b/tests/test_install.py index 262a033ed..b4977f594 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1135,6 +1135,75 @@ class StandaloneCopiesReadWhereThePackPutTheFile(unittest.TestCase): install._validate_manifest(manifest, "emudeck") +class OuterRootsAreUntrusted(unittest.TestCase): + """roms_directory and emulators_directory come from a downloaded list: each + path in them meets the rules a BIOS destination meets, and each folder + they resolve to stays where the platform's layout puts it.""" + + ROMS = {"bios_folder": "bios", "from_bios": ".."} + EMULATORS = { + "bios_folder": "bios", "searched": ["frontend/{key}", "emulators/{key}"], + "default": "emulators/{key}", "config": "frontend/launcher.cfg", + } + UNSAFE = ("..\\x", "C:", "C:/x", "..//x", "../", "../x\x01", "x" * 1025) + + def test_unsafe_from_bios_is_refused(self): + for value in (*self.UNSAFE, "../../../x", "../x/..", "x", "./x", "/x", "..x", None): + with self.subTest(from_bios=value), self.assertRaises(ValueError): + install._check_roms_directory({"roms_directory": {**self.ROMS, "from_bios": value}}) + + def test_the_relations_the_rom_folders_files_use_are_accepted(self): + bios = Path("/card/system/bios") + expected = { + ".": bios, "..": Path("/card/system"), "../roms": Path("/card/system/roms"), + "../Roms": Path("/card/system/Roms"), "../../Roms": Path("/card/Roms"), + "../../ROMS": Path("/card/ROMS"), + } + for value, root in expected.items(): + with self.subTest(from_bios=value): + install._check_roms_directory({"roms_directory": {**self.ROMS, "from_bios": value}}) + self.assertEqual(install._roms_root(bios, value), root) + self.assertEqual(install.MAX_ROMS_UPS, 2) + + def test_a_list_without_a_roms_root_leaves_the_files_out(self): + files = [{"dest": "pico-8/pico8_64", "root": "roms", "cores": ["x"]}] + kept, left = install.place_roms_entries(files, Path("/card/roms/bios"), {}, "x", say=False) + self.assertEqual((kept, [o["reason"] for o in left]), ([], ["no_roms_directory"])) + + def test_unsafe_emulator_folders_are_refused(self): + for field in ("searched", "default", "config"): + for value in self.UNSAFE + ("../{key}", "/abs/{key}"): + layout = dict(self.EMULATORS) + if field == "searched": + layout["searched"] = [value if "{key}" in value else value + "/{key}"] + layout["default"] = layout["searched"][0] + elif field == "default": + layout["default"] = value + else: + layout["config"] = value + with self.subTest(field=field, value=value), self.assertRaises(ValueError): + install._check_emulators_directory({"emulators_directory": layout}) + + def test_an_emulator_folder_stays_below_the_platform_folder(self): + with tempfile.TemporaryDirectory() as tmp: + home = Path(tmp) + (home / "bios").mkdir() + (home / "frontend").mkdir() + (home / "frontend" / "launcher.cfg").write_text("") + manifest = {"emulators_directory": self.EMULATORS} + for dest in ("../x/prog", "C:/prog", "prog\\..\\..\\x/a"): + with self.subTest(dest=dest), self.assertRaises(ValueError): + install.place_emulator_entries( + [{"dest": dest, "root": "emulators"}], home / "bios", manifest, "x", + say=False) + kept, left = install.place_emulator_entries( + [{"dest": "prog/prog.exe", "root": "emulators"}], home / "bios", manifest, "x", + say=False) + self.assertEqual((len(kept), left), (1, [])) + folder, rest = install._entry_place(home / "bios", kept[0]) + self.assertEqual((folder, rest), (home / "emulators" / "prog", "prog.exe")) + + class DetectionFollowsTheRegistry(unittest.TestCase): """The registry declared RetroPie at ~/RetroPie/BIOS and BizHawk at ~/.config/BizHawk/Firmware, every manifest published it, and the