fix: show outside text, never obey it

Pack and list paths with a control character are refused, the names a
list records, card labels, folders and events pass one filter, and a
folder a configuration names with one is not taken.

install.ps1

@@ -11,7 +11,7 @@
 
     $ErrorActionPreference = "Stop"
     $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
-    $defaultInstallSha256 = "f4f2d041d923e32d46c828222f43f4359c5241fcce7ccc29bfc9b3e8565bba0b"
+    $defaultInstallSha256 = "455524a94cfddee646331cc5b92358ef52c06f8ed23a0a73be35355efa6528ea"
     $maximumInstallerBytes = 2MB
     $installerExit = 0
     $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null }

install.py

@@ -632,7 +632,11 @@ class EventStream:
         keys = EVENT_KEYS[kind]
         if set(fields) != set(keys):
             raise ValueError(f"event {kind}: keys {sorted(fields)} != {sorted(keys)}")
-        line = json.dumps({"event": kind, **{key: fields[key] for key in keys}})
+        # Every string a script reads goes through the terminal's filter,
+        # a new line kept: a consumer printing a folder or a name obeyed
+        # the control characters a card label or a list held.
+        line = json.dumps({"event": kind, **{key: _clean_strings(fields[key], "\n")
+                                             for key in keys}})
         with self._lock:
             self.stream.write(line + "\n")
             self.stream.flush()
@@ -941,8 +945,11 @@ def shown_path(path: Path | str) -> str:
     match = (re.match(rf"^{re.escape(root)}([a-z])(?:/(.*))?$", text)
              if _WSL["active"] else None)
     if match:
-        return f"{match.group(1).upper()}:\\" + (match.group(2) or "").replace("/", "\\")
-    return text
+        text = f"{match.group(1).upper()}:\\" + (match.group(2) or "").replace("/", "\\")
+    # A folder's name comes from whoever named it: a card's label, a
+    # retroarch.cfg, a pack. Printed raw, a control character in it is a
+    # terminal command (code_audit_18 P2-1).
+    return screen_text(text)
 
 
 def _wsl_dest(args: argparse.Namespace) -> None:
@@ -2009,6 +2016,8 @@ def launchbox_bios_dirs(root: Path) -> dict[str, Path]:
 
 # (folder, why in words) for each folder detection could not look in.
 _DETECTION_REFUSED: list[tuple[Path, str]] = []
+# The platforms whose configuration names a folder with a control character.
+_UNNAMEABLE: list[str] = []
 
 
 def refused_words(folder: Path, why: str, shown: str = "") -> str:
@@ -2360,7 +2369,21 @@ def detect_platforms(os_type: str) -> list[tuple[str, Path]]:
     _detect_by_path(os_type, found)
     # An embedded system's config read again as the home folder's one (HOME is
     # /storage on the ELEC family) is the same folder.
-    return list(dict.fromkeys(found))
+    kept = []
+    for name, path in dict.fromkeys(found):
+        if _CONTROLS.search(str(path)):
+            # A config naming a folder with a control character: printed,
+            # the name is a terminal command, and the folder one nobody can
+            # type (code_audit_18 P2-1). Its default is then no answer: the
+            # setup reads the folder named.
+            if name not in _UNNAMEABLE:
+                _UNNAMEABLE.append(name)
+            print(f"  {display_name(name)}: the folder its configuration names holds a "
+                  f"control character ({shown_path(path)}). Name its BIOS folder with "
+                  "--dest.", file=sys.stderr)
+            continue
+        kept.append((name, path))
+    return kept
 
 
 def _emudeck_windows_bios() -> Path | None:
@@ -2728,11 +2751,14 @@ def _safe_relative_path(value: object, field: str) -> PurePosixPath:
         raise ValueError(f"invalid {field}")
     if (
         "\\" in value
-        or "\x00" in value
+        or _CONTROLS.search(value)
         or "//" in value
         or value.endswith("/")
         or re.match(r"^[A-Za-z]:", value)
     ):
+        # A control character is refused with NUL: a pack's member named
+        # x<ESC>]0;PWN<BEL>.bin set the window title in the progress line,
+        # and the file was written under that name (code_audit_18 P2-1).
         raise ValueError(f"unsafe {field}: {value!r}")
     path = PurePosixPath(value)
     if path.is_absolute() or any(part in ("", ".", "..") for part in path.parts):
@@ -2759,7 +2785,7 @@ def _safe_target_dir(value: object, field: str) -> str:
     """
     if not isinstance(value, str) or not value or len(value) > 2048:
         raise ValueError(f"invalid {field}")
-    if "\x00" in value:
+    if _CONTROLS.search(value):
         raise ValueError(f"unsafe {field}: {value!r}")
     parts = re.split(r"[\\/]", value)
     if any(part == ".." for part in parts):
@@ -3126,6 +3152,21 @@ def _check_data_directory(entry: object, where: str) -> None:
             raise ValueError(f"{where}.files[{rel!r}] is invalid")
 
 
+def _clean_strings(value: object, keep: str = "") -> object:
+    """value with a "?" for each control character of its strings, keys
+    included, those of keep left: the one filter between a list, a card or
+    a config and the terminal, its paths refused before (_safe_relative_path)."""
+    if isinstance(value, str):
+        return _CONTROLS.sub(lambda found: found.group() if found.group() in keep else "?",
+                             value)
+    if isinstance(value, dict):
+        return {_clean_strings(key, keep): _clean_strings(item, keep)
+                for key, item in value.items()}
+    if isinstance(value, (list, tuple)):
+        return [_clean_strings(item, keep) for item in value]
+    return value
+
+
 def _validate_manifest(data: object, plat: str) -> dict:
     """Validate the untrusted install-manifest boundary using stdlib only."""
     if not isinstance(data, dict):
@@ -3284,7 +3325,9 @@ def _validate_manifest(data: object, plat: str) -> dict:
                 raise ValueError(f"invalid standalone copy targets at index {index}")
             for value in values:
                 _safe_target_dir(value, f"standalone_copies[{index}].targets")
-    return data
+    # Its paths checked above, every other string (a system's title, a
+    # core's name) is shown as the terminal shows text.
+    return _clean_strings(data)
 
 
 def _validate_targets(data: object) -> dict[str, dict]:
@@ -6771,7 +6814,7 @@ def _default_dest(os_type: str, platform_name: str) -> Path | None:
     C:\\userdata\\bios, reported done.
     """
     default = _built_in_dest(os_type, platform_name)
-    if default is not None and _refused_near(default):
+    if default is not None and (_refused_near(default) or platform_name in _UNNAMEABLE):
         # The Flatpak's own folder at 000 hid the retroarch.cfg naming
         # ~/Games/bios, and 6.6 GB were offered to a folder it does not read.
         return None
@@ -13584,7 +13627,8 @@ def _mount_name(mount: Path) -> str:
     if (_WSL["active"] and mount.parent == Path(_WSL["root"]) and len(mount.name) == 1
             and mount.name.isalpha()):
         return f"{mount.name.upper()}:"
-    return mount.name or str(mount)
+    # The label is whoever formatted the card's: shown, never obeyed.
+    return screen_text(mount.name or str(mount))
 
 
 def _mounted_cards() -> list[Path]:

install.sh

@@ -18,7 +18,7 @@ esac
 TEMP_INSTALLER=""
 TEMP_DIRECTORY=""
 DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
-DEFAULT_INSTALL_SHA256="f4f2d041d923e32d46c828222f43f4359c5241fcce7ccc29bfc9b3e8565bba0b"
+DEFAULT_INSTALL_SHA256="455524a94cfddee646331cc5b92358ef52c06f8ed23a0a73be35355efa6528ea"
 MAX_INSTALLER_BYTES=2097152
 
 cleanup() {

install/launchers/index.json

@@ -27,8 +27,8 @@
         {
           "dest": "roms/ports/retrobios/install.py",
           "repo_path": "install.py",
-          "size": 784255,
-          "sha256": "f4f2d041d923e32d46c828222f43f4359c5241fcce7ccc29bfc9b3e8565bba0b",
+          "size": 786759,
+          "sha256": "455524a94cfddee646331cc5b92358ef52c06f8ed23a0a73be35355efa6528ea",
           "executable": false
         },
         {
@@ -71,8 +71,8 @@
     "muos": {
       "archive": {
         "dest": "ARCHIVE/RetroBIOS.muxapp",
-        "size": 836804,
-        "sha256": "2a46dfb5f18a6535fed35fc828078657fa09440abeafd3219ba099ca3ef4e580"
+        "size": 839308,
+        "sha256": "4fef6336d600ff82f1b0d5040d537775df190c23d9ae97e022160e60189e127a"
       },
       "members": [
         {
@@ -92,8 +92,8 @@
         {
           "name": "RetroBIOS/install.py",
           "repo_path": "install.py",
-          "size": 784255,
-          "sha256": "f4f2d041d923e32d46c828222f43f4359c5241fcce7ccc29bfc9b3e8565bba0b",
+          "size": 786759,
+          "sha256": "455524a94cfddee646331cc5b92358ef52c06f8ed23a0a73be35355efa6528ea",
           "executable": false
         },
         {
@@ -166,8 +166,8 @@
         {
           "dest": ".config/modules/retrobios/install.py",
           "repo_path": "install.py",
-          "size": 784255,
-          "sha256": "f4f2d041d923e32d46c828222f43f4359c5241fcce7ccc29bfc9b3e8565bba0b",
+          "size": 786759,
+          "sha256": "455524a94cfddee646331cc5b92358ef52c06f8ed23a0a73be35355efa6528ea",
           "executable": false
         },
         {

tests/test_install_tui.py

@@ -8595,5 +8595,119 @@ class AuditEighteenAndEleven(unittest.TestCase):
         self.assertIn("Recalbox SHARE drive", block)
         self.assertEqual(generate_installer_doc.render(page), page)
 
+class NothingFromOutsideReachesTheTerminalRaw(unittest.TestCase):
+    """code_audit_18 P2-1: a pack's member path, a --manifest dest, the
+    names a list records, a card's label and a retroarch.cfg folder reached
+    line mode, the events and the disk with their control characters. Each
+    source is fed escape sequences, and none comes out raw."""
+
+    OSC = "\x1b]0;PWNED\x07"
+    CSI = "\x1b[41m"
+
+    def setUp(self):
+        (REPO_ROOT / "tmp").mkdir(exist_ok=True)
+        self.root = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp"))
+        self.addCleanup(shutil.rmtree, self.root, True)
+        install.LOCAL.reset()
+        self.addCleanup(install.LOCAL.reset)
+        install._UNNAMEABLE.clear()
+        self.addCleanup(install._UNNAMEABLE.clear)
+        self.dest = self.root / "dest"
+        self.dest.mkdir()
+
+    def assertClean(self, text: str, where: str) -> None:
+        self.assertNotIn("\x1b", text, where)
+        self.assertNotIn("\x07", text, where)
+
+    def assertNothingRawOnDisk(self) -> None:
+        for path in self.root.rglob("*"):
+            self.assertIsNone(install._CONTROLS.search(path.name), path)
+
+    def run_install(self, args: argparse.Namespace, mounts: list = (),
+                    env: dict | None = None) -> str:
+        events = io.StringIO()
+        with mock.patch.object(install, "_mounted_cards", return_value=list(mounts)), \
+                mock.patch.object(install, "_mount_points", return_value=frozenset()), \
+                mock.patch.object(install, "detect_frontends", return_value=[]), \
+                mock.patch.object(install, "detect_os", return_value="linux"), \
+                mock.patch.object(install.EVENTS, "stream", events), \
+                mock.patch.dict(os.environ, {install.OFFLINE_VARIABLE: "1", **(env or {})}), \
+                contextlib.redirect_stdout(io.StringIO()) as out, \
+                contextlib.redirect_stderr(io.StringIO()) as err, \
+                contextlib.suppress(SystemExit):
+            install._install(args, io.StringIO())
+        return out.getvalue() + err.getvalue() + events.getvalue()
+
+    def test_a_pack_member_path_is_refused(self):
+        from test_install_offline import _pack, GBA  # noqa: PLC0415
+
+        pack = _pack(self.root, "RetroArch_v1_BIOS_Pack.zip", {f"x{self.OSC}.bin": GBA})
+        text = self.run_install(_args(platform="retroarch", dest=self.dest,
+                                      from_pack=[pack], no_input=True, declared_only=False))
+        self.assertClean(text, "pack")
+        self.assertIn("is not valid: unsafe", text)
+        self.assertEqual(list(self.dest.iterdir()), [])
+        self.assertNothingRawOnDisk()
+
+    def test_a_list_dest_and_its_names_are_refused_or_shown(self):
+        files = [{"dest": f"x{self.CSI}BAD.bin", "sha1": "0" * 40, "size": 1,
+                  "repo_path": "bios/x", "cores": None, "systems": ["a"]}]
+        listing = self.root / "list.json"
+        listing.write_text(json.dumps({"manifest_version": 2, "platform": "retroarch",
+                                       "files": files}))
+        text = self.run_install(_args(platform="retroarch", dest=self.dest, manifest=listing,
+                                      no_input=True, dry_run=True, verbose=True,
+                                      declared_only=False))
+        self.assertClean(text, "--manifest dest")
+        self.assertIn("unsafe", text)
+        named = {"manifest_version": 2, "platform": "retroarch", "files": [
+            {**files[0], "dest": "a.bin", "systems": [f"a{self.CSI}"],
+             "cores": [f"core{self.OSC}"]}],
+            "system_titles": {f"a{self.CSI}": f"Title{self.OSC}"}}
+        cleaned = install._validate_manifest(named, "retroarch")
+        self.assertClean(json.dumps(cleaned, ensure_ascii=False), "names")
+        self.assertNothingRawOnDisk()
+
+    def test_a_card_label_is_shown_and_never_obeyed(self):
+        card = self.root / "media" / f"S{self.CSI}RED"
+        (card / "system").mkdir(parents=True)
+        (card / "system" / "batocera.conf").write_text("system.hostname=BATOCERA\n")
+        listing = self.root / "batocera.json"
+        listing.write_text(json.dumps({"manifest_version": 2, "platform": "batocera",
+                                       "files": []}))
+        with mock.patch.object(install, "detect_platforms", return_value=[]):
+            text = self.run_install(_args(platform="batocera", manifest=listing,
+                                          no_input=True, dry_run=True, events=True,
+                                          declared_only=False), [card])
+        self.assertIn("S?[41mRED", text)
+        self.assertClean(text, "card label")
+        self.assertClean(install.card_line(card), "card row")
+
+    def test_a_folder_a_config_names_is_refused(self):
+        home = self.root / "home"
+        weird = home / f"sys{self.OSC}x"
+        weird.mkdir(parents=True)
+        cfg = home / ".config" / "retroarch" / "retroarch.cfg"
+        cfg.parent.mkdir(parents=True)
+        cfg.write_text(f'system_directory = "{weird}"\n')
+        listing = self.root / "retroarch.json"
+        listing.write_text(json.dumps({"manifest_version": 2, "platform": "retroarch",
+                                       "files": []}))
+        text = self.run_install(
+            _args(manifest=listing, no_input=True, dry_run=True, events=True,
+                  declared_only=False),
+            env={"HOME": str(home), "XDG_CONFIG_HOME": str(home / ".config"),
+                 "RETROBIOS_OS": "linux"})
+        self.assertClean(text, "retroarch.cfg")
+        self.assertIn("holds a control character", text)
+        self.assertNotIn("using default path", text)
+        self.assertNotIn(str(home / ".config" / "retroarch" / "system"), text)
+
+    def test_a_path_with_a_control_character_is_no_relative_path(self):
+        for value in ("a\x1bb.bin", "a\x07.bin", "a\x9bb.bin", "a\nb.bin"):
+            with self.subTest(value=value), self.assertRaises(ValueError):
+                install._safe_relative_path(value, "dest")
+        self.assertEqual(install.shown_path(Path("/a/b\x1b[2Jc")), "/a/b?[2Jc")
+
 if __name__ == "__main__":
     unittest.main()

wiki/installer.md

@@ -1382,12 +1382,20 @@ That keeps the installer's questions working when the script is piped from
 
 The manifest is fetched over the network and treated as untrusted input. Every
 path it carries goes through one validator that refuses an absolute path, a
-drive letter, a backslash, a null byte, a doubled separator, a trailing slash
-and any `..` component, so a destination cannot climb out of the BIOS
-directory. Manifest, target list and payloads each have a size cap, the file
+drive letter, a backslash, a null byte or any other control character, a
+doubled separator, a trailing slash and any `..` component, so a destination
+cannot climb out of the BIOS directory and no file is named with a terminal
+command. Manifest, target list and payloads each have a size cap, the file
 list a count cap, and the sum of the declared sizes is checked against the
 total the manifest claims.
 
+Every other string from outside is shown, never obeyed: the names a list or
+a pack records, a card's label and a folder a configuration names reach the
+terminal, line by line or on the screen, and the `--events` stream with a `?`
+for each control character. A folder a configuration names with one is not
+taken: the run says so and asks for `--dest`, its default being no answer
+for a setup that reads another folder.
+
 Symbolic links met under the BIOS root are followed on purpose: they are the
 user's own layout, and EmuDeck links `bios/shadps4/sys_modules` into shadPS4's
 data directory where the emulator actually reads it. The boundary is lexical,