diff --git a/docs/AOT_OVERLAY_PLAN.md b/docs/AOT_OVERLAY_PLAN.md index 77d51027..a6031d1e 100644 --- a/docs/AOT_OVERLAY_PLAN.md +++ b/docs/AOT_OVERLAY_PLAN.md @@ -331,7 +331,10 @@ additively merging every valid snapshot. Development configs may also set a project-relative `overlay_capture_persist_dir`. This creates one atomically published, immutable -JSON snapshot per changed capture. Absolute paths and `..` components are rejected. +JSON snapshot per changed capture. Rooted, UNC, drive-qualified and +drive-relative paths are rejected, as is any `..` component or an embedded NUL, +under both `/` and `\` separators, so a config is accepted or rejected +identically on every host. Production configs normally omit this key and retain only the naive executable- local addendum. Both history modes are off by default and contain game-derived bytes, so their outputs remain private/gitignored artifacts. diff --git a/recompiler/src/config_loader.cpp b/recompiler/src/config_loader.cpp index a83c6f0b..25fca050 100644 --- a/recompiler/src/config_loader.cpp +++ b/recompiler/src/config_loader.cpp @@ -229,6 +229,41 @@ static bool parse_aspect_ratio(const std::string& s, int* num, int* den) { return true; } +// Reject a config path that is not confined to the project directory. +// +// A game.toml is checked in and read on every host, so a path in it must mean +// the same thing everywhere; std::filesystem::path answers by the *host* +// grammar, so a Windows-authored escape passes on POSIX. Scan the raw bytes +// under both separator conventions instead. NUL is rejected outright because +// the runtime hands this value on as a C string — anything past a NUL would +// silently vanish between what is validated and what is used. +static void validate_project_relative(const std::string& value, + const char* field) { + if (value.find('\0') != std::string::npos) { + throw std::runtime_error( + fmt::format("{} must not contain a NUL byte", field)); + } + // Rooted ("/x", "\x"), UNC ("\\server\share"), drive-qualified ("C:/x", + // "C:\x") and drive-relative ("C:x") alike. + const bool rooted = !value.empty() && (value[0] == '/' || value[0] == '\\'); + const bool drive = value.size() >= 2 && value[1] == ':' && + std::isalpha(static_cast(value[0])); + if (rooted || drive) { + throw std::runtime_error( + fmt::format("{} must be project-relative", field)); + } + for (size_t start = 0;;) { + const size_t sep = value.find_first_of("/\\", start); + const size_t end = (sep == std::string::npos) ? value.size() : sep; + if (value.compare(start, end - start, "..") == 0) { + throw std::runtime_error( + fmt::format("{} must stay inside the project", field)); + } + if (sep == std::string::npos) break; + start = sep + 1; + } +} + // Parse the optional [runtime] block. All fields optional; absent fields // leave has_* = false on the returned struct. Paths are resolved relative // to `root` (project root). @@ -395,17 +430,8 @@ static RuntimeConfig parse_runtime_block(const toml::value& cfg, const fs::path& if (runtime.contains("overlay_capture_persist_dir")) { rt.overlay_capture_persist_dir = toml::find(runtime, "overlay_capture_persist_dir"); - const std::filesystem::path persist(rt.overlay_capture_persist_dir); - if (persist.is_absolute()) { - throw std::runtime_error( - "runtime.overlay_capture_persist_dir must be project-relative"); - } - for (const auto& component : persist) { - if (component == "..") { - throw std::runtime_error( - "runtime.overlay_capture_persist_dir must stay inside the project"); - } - } + validate_project_relative(rt.overlay_capture_persist_dir, + "runtime.overlay_capture_persist_dir"); } if (runtime.contains("turbo_loads")) { rt.turbo_loads = toml::find(runtime, "turbo_loads"); diff --git a/recompiler/src/config_loader.h b/recompiler/src/config_loader.h index 51d5d864..6c514fa5 100644 --- a/recompiler/src/config_loader.h +++ b/recompiler/src/config_loader.h @@ -235,9 +235,11 @@ struct RuntimeConfig { bool overlay_capture_history = false; // overlay_capture_persist_dir: optional DEV-only, project-relative safe - // directory for one immutable JSON file per changed snapshot. Absolute - // paths and `..` components are rejected. Production normally leaves this - // unset and retains only the addendum beside the executable. + // directory for one immutable JSON file per changed snapshot. Rooted, + // UNC, drive-qualified and drive-relative paths are rejected, as is any + // `..` component or an embedded NUL, under both `/` and `\` separators. + // Production normally leaves this unset and retains only the addendum + // beside the executable. std::string overlay_capture_persist_dir; // turbo_loads: OPT-IN per game. While the game is loading (CD data diff --git a/recompiler/tests/recompiler_patch_test.cpp b/recompiler/tests/recompiler_patch_test.cpp index 8975b6f0..b5c8d393 100644 --- a/recompiler/tests/recompiler_patch_test.cpp +++ b/recompiler/tests/recompiler_patch_test.cpp @@ -466,6 +466,60 @@ overlay_capture_persist_dir = "C:/outside" check_throws([&] { (void)PSXRecompV4::load_game_config(absolute); }, "must be project-relative", "parser rejects absolute capture history directory"); + + // A game.toml travels between hosts, so each of these must be rejected on + // every host, not only on the one whose path grammar it is written in. + // TOML literal strings ('...') are used because a backslash is an escape + // inside a TOML basic string. Cases marked POSIX-visible are the ones a + // host-native std::filesystem check silently accepted under POSIX. + struct RejectedDir { + const char* value; // as written in game.toml + const char* needle; // expected message fragment + const char* name; + }; + static constexpr RejectedDir rejected[] = { + {"'/outside'", "must be project-relative", + "parser rejects rooted capture history directory"}, + {R"('\outside')", "must be project-relative", + "parser rejects backslash-rooted capture history directory"}, + {R"('C:\outside')", "must be project-relative", + "parser rejects backslash drive-qualified capture history directory"}, + {"'C:outside'", "must be project-relative", + "parser rejects drive-relative capture history directory"}, + {R"('\\server\share\outside')", "must be project-relative", + "parser rejects UNC capture history directory"}, + {R"('..\outside')", "must stay inside the project", + "parser rejects backslash-escaping capture history directory"}, + {R"('nested\..\..\outside')", "must stay inside the project", + "parser rejects nested backslash-escaping capture history directory"}, + {"'nested/..'", "must stay inside the project", + "parser rejects a trailing .. component"}, + // A basic string, not a literal one: TOML forbids a raw NUL byte but + // allows the \u0000 escape that produces it. The runtime passes this + // value on as a C string, so a NUL would truncate it after validation. + {R"("..\u0000keep")", "must not contain a NUL byte", + "parser rejects an embedded NUL escape"}, + }; + for (size_t i = 0; i < std::size(rejected); ++i) { + const auto& c = rejected[i]; + const auto cfg_path = write_config( + root, fmt::format("capture-history-rejected-{}", i), + fmt::format("[runtime]\noverlay_capture_persist_dir = {}\n", + c.value)); + check_throws([&] { (void)PSXRecompV4::load_game_config(cfg_path); }, + c.needle, c.name); + } + + // A dotfile is not a `..` component; the guard must not over-reject. + const auto dotted = write_config(root, "capture-history-dotted", R"toml( +[runtime] +overlay_capture_history = true +overlay_capture_persist_dir = ".aot_capture_history/..keep/TEST-00000" +)toml"); + const auto dotted_cfg = PSXRecompV4::load_game_config(dotted); + check(dotted_cfg.runtime.overlay_capture_persist_dir == + ".aot_capture_history/..keep/TEST-00000", + "parser accepts a capture history component that merely starts with dots"); } void codegen_tests() {