Merge pull request #97 from xThrasherrr/fix/custom-maps-mod-loading

Make mod loading tolerate damaged mods and BOM manifests

Engine/Game/World/custom_level_manifest.cpp

@@ -86,9 +86,9 @@ void exact_fields(const Json& object, std::initializer_list<std::string_view> ex
 CustomLevelManifest parse(std::string_view text) {
     if (text.size() > custom_level_manifest_max_bytes)
         throw std::runtime_error("custom-level manifest exceeds 64 KiB");
-    if (text.starts_with("\xef\xbb\xbf"))
-        throw std::runtime_error("custom-level manifest must not contain a UTF-8 BOM");
 
+    // Json::parse skips a byte-order mark, which several Windows editors still
+    // write. The launcher and the server accept one, so the game must too.
     const auto root = Json::parse(text, JsonLimits{
         custom_level_manifest_max_bytes, 8, 8192});
     exact_fields(root, {"schema", "levels"}, "custom-level manifest");

Engine/Game/World/custom_level_manifest.h

@@ -23,7 +23,7 @@ struct CustomLevelManifest {
     std::string issue;
 };
 
-// Parse the Studio-authored Patch/dingosdk-levels.json contract. Invalid input
+// Parse a mod's or the Patch folder's reskate-levels.json. Invalid input
 // fails closed: no partial set of destinations is returned.
 CustomLevelManifest parse_custom_level_manifest(std::string_view text) noexcept;
 CustomLevelManifest read_custom_level_manifest(const std::filesystem::path& path) noexcept;

Engine/Resource/binary_bundle.cpp

@@ -167,6 +167,8 @@ BinaryBundle read_binary_bundle(const std::span<const std::byte> data) {
     const auto metadataOffset = reader.u32(endian);
     const auto metadataSize = reader.u32(endian);
     if (strings > data.size()) throw std::out_of_range("Binary bundle string table is out of range");
+    const auto tables = std::uint64_t{ebxCount} * 28 + std::uint64_t{resourceCount} * 56 + std::uint64_t{chunkCount} * 44; // SHA-1 plus entry bytes
+    if (tables > reader.remaining()) throw std::out_of_range("Binary bundle asset counts exceed the file");
 
     std::vector<Sha1> hashes(total);
     for (auto& hash : hashes) hash = reader.sha1();

Engine/Resource/toc.cpp

@@ -133,6 +133,10 @@ std::string c_string_at(const std::span<const std::byte> data, const std::size_t
     throw std::runtime_error("TOC name is unterminated");
 }
 
+void expect_table(const BinaryReader& reader, std::size_t count, std::size_t entrySize, const char* what) { // before sizing by a file's count
+    if (count > reader.remaining() / entrySize) throw std::out_of_range(what);
+}
+
 class HuffmanNames final {
 public:
     HuffmanNames(const std::span<const std::byte> data,
@@ -142,6 +146,7 @@ public:
                  const std::size_t tableCount) {
         BinaryReader reader(data);
         reader.seek(namesOffset);
+        expect_table(reader, namesCount, 4, "TOC name count exceeds the file");
         words_.reserve(namesCount);
         for (std::size_t index = 0; index < namesCount; ++index) words_.push_back(reader.u32(Endian::big));
         reader.seek(tableOffset);
@@ -364,9 +369,11 @@ TocDocument read_toc(const std::span<const std::byte> data,
     TocDocument result;
     result.flags = flags;
     reader.seek(bundleHashOffset);
+    expect_table(reader, bundlesSize, 4, "TOC bundle count exceeds the file");
     result.bundleHashMap.reserve(bundlesSize);
     for (std::size_t index = 0; index < bundlesSize; ++index) result.bundleHashMap.push_back(reader.i32(Endian::big));
     reader.seek(bundleDataOffset);
+    expect_table(reader, bundlesSize, 16, "TOC bundle count exceeds the file");
     result.bundles.reserve(bundlesSize);
     for (std::size_t index = 0; index < bundlesSize; ++index) {
         const auto nameOffset = reader.u32(Endian::big);
@@ -389,12 +396,15 @@ TocDocument read_toc(const std::span<const std::byte> data,
     }
     if (chunksSize) {
         reader.seek(chunkHashOffset);
+        expect_table(reader, chunksSize, 4, "TOC chunk count exceeds the file");
         result.chunkHashMap.reserve(chunksSize);
         for (std::size_t index = 0; index < chunksSize; ++index) result.chunkHashMap.push_back(reader.i32(Endian::big));
-        std::vector<std::uint32_t> words(static_cast<std::size_t>(dataCount));
         reader.seek(chunkDataOffset);
+        expect_table(reader, static_cast<std::size_t>(dataCount), 4, "TOC chunk data count exceeds the file");
+        std::vector<std::uint32_t> words(static_cast<std::size_t>(dataCount));
         for (auto& value : words) value = reader.u32(Endian::big);
         reader.seek(chunkGuidOffset);
+        expect_table(reader, chunksSize, 20, "TOC chunk count exceeds the file");
         result.chunks.reserve(chunksSize);
         for (std::size_t index = 0; index < chunksSize; ++index) {
             const auto guidBytes = reader.view(16);

Engine/Vfs/game_archives.cpp

@@ -1,6 +1,7 @@
 #include "game_archives.h"
 #include "Engine/Resource/cas_codec.h"
 #include "Engine/Core/Platform/path_text.h"
+#include <charconv>
 #include <cstdio>
 #include <cstring>
 #include <fstream>
@@ -151,8 +152,10 @@ const std::string& GameArchives::directory(std::uint32_t installChunk) const {
 std::optional<std::uint16_t> GameArchives::archive_index(const std::string& stem) {
     const auto digits = stem.find_last_not_of("0123456789");
     if (digits == std::string::npos || digits + 1 >= stem.size()) return {};
-    const auto value = std::stoul(stem.substr(digits + 1));
-    if (value > 0xFFFF) return {};
-    return static_cast<std::uint16_t>(value);
+    std::uint16_t value{};
+    const auto end = stem.data() + stem.size();
+    const auto [last, error] = std::from_chars(stem.data() + digits + 1, end, value); // range error past 65535, never throws
+    if (error != std::errc{} || last != end) return {};
+    return value;
 }
 }

Engine/Vfs/mod_catalog.cpp

@@ -225,9 +225,9 @@ Catalog load_catalog(const std::filesystem::path& data_root, const MergeObserver
             // Merge, and whenever a mod could not be merged cleanly, merge again
             // without it. Removing one mod cannot break another, but a few rounds
             // are allowed in case two damaged mods hid each other's problems.
-            for (int round = 0; round < 4; ++round) {
+            for (int round = 0;; ++round) {
                 merge = merge_mods(result, observe);
-                if (!merge.issue.empty()) break;
+                if (!merge.issue.empty() || round == 4) break; // a fifth merge only rebuilds without round four's removals
                 bool removed = false;
                 for (std::size_t i = result.mods.size(); i-- > 0;) {
                     const auto found = merge.problems.find(result.mods[i].name);

Engine/Vfs/mod_merge.cpp

@@ -217,11 +217,12 @@ MergeReport merge_mods(const Catalog& catalog, const MergeObserver& observe, con
             }
             for (const auto& relative : files.archives) {
                 const auto path = fs::path(relative);
-                const auto stem = path.stem().string();
-                const auto digits = stem.find_last_not_of("0123456789");
-                if (digits == std::string::npos || digits + 1 >= stem.size())
-                    throw std::runtime_error("Unexpected archive name: " + relative);
-                const auto number = static_cast<std::uint16_t>(std::stoul(stem.substr(digits + 1)));
+                const auto index = vfs::GameArchives::archive_index(path.stem().string());
+                if (!index) { // the mod's problem: the catalog merges again without it
+                    report.problems[mod->name].push_back("Unexpected archive name: " + relative);
+                    continue;
+                }
+                const auto number = *index;
                 // CasStore keys its directories relative to Win32, so the mod's
                 // own Win32/ prefix comes off before the two are matched up.
                 auto directory = lower(path.parent_path().generic_string());
@@ -339,7 +340,12 @@ MergeReport merge_mods(const Catalog& catalog, const MergeObserver& observe, con
                 Source source;
                 source.root = mod->directory;
                 source.placement = &placements[mod];
-                source.toc = fb::read_toc(read_file(mod->directory / fs::path(relative)));
+                try {
+                    source.toc = fb::read_toc(read_file(mod->directory / fs::path(relative)));
+                } catch (const std::exception& failure) { // the mod's problem: the catalog merges again without it
+                    report.problems[mod->name].push_back(relative + " could not be read: " + failure.what());
+                    continue;
+                }
                 sources.push_back(std::move(source));
             }
             auto merged = combine(baseRoot / fs::path(relative), baseRoot, sources, report,

Engine/Vfs/mod_merge_files.cpp

@@ -40,7 +40,7 @@ void write_file(const fs::path& path, std::span<const std::byte> bytes) {
     {
         std::ofstream output(temporary, std::ios::binary | std::ios::trunc);
         if (!output || !output.write(reinterpret_cast<const char*>(bytes.data()),
-                                     static_cast<std::streamsize>(bytes.size())))
+                                     static_cast<std::streamsize>(bytes.size())) || !output.flush()) // ~ofstream ignores a failed flush
             throw std::runtime_error("Cannot write " + path_utf8(temporary));
     }
     if (!MoveFileExW(temporary.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH)) {

Engine/Vfs/mod_scoring.cpp

@@ -201,6 +201,7 @@ ScoringCheck check_scoring(const Catalog& catalog, std::vector<std::string>* not
             result.assets.push_back(key.substr(4));
             for (const auto& sha1 : versions) hash = fnv(hash, key + ' ' + hex(sha1) + '\n');
         }
+        std::ranges::sort(result.assets); // "ebx "/"res " keys sort apart once the prefix is gone
         result.assets.erase(std::unique(result.assets.begin(), result.assets.end()), result.assets.end());
         result.fingerprint = hash ? hash : 1;
     } catch (const std::exception& failure) {

Extension/Assets/loose_files.cpp

@@ -47,7 +47,7 @@ struct State {
 // Hooks and the native mount live until process exit, like the pinned runtime.
 State& state() { static auto* value = new State; return *value; }
 
-// Supported September 8 build, checked after runtime image validation.
+// Supported game build, checked after runtime image validation.
 bool validate_contract(std::uintptr_t base) {
     struct Contract { std::uintptr_t rva; std::span<const unsigned char> bytes; };
     const Contract contracts[]{
@@ -58,7 +58,8 @@ bool validate_contract(std::uintptr_t base) {
     for (const auto& contract : contracts) {
         std::array<unsigned char, 32> actual{};
         SIZE_T count{};
-        if (!ReadProcessMemory(GetCurrentProcess(), reinterpret_cast<const void*>(base + contract.rva),
+        if (contract.bytes.size() > actual.size() || // a longer prefix would overrun actual
+            !ReadProcessMemory(GetCurrentProcess(), reinterpret_cast<const void*>(base + contract.rva),
                 actual.data(), contract.bytes.size(), &count) || count != contract.bytes.size() ||
             std::memcmp(actual.data(), contract.bytes.data(), contract.bytes.size())) return false;
     }

Extension/Assets/mod_layers.cpp

@@ -127,7 +127,7 @@ bool read_memory(std::uintptr_t address, void* destination, std::size_t size) no
         destination, size, &read) && read == size;
 }
 
-// Supported September 8 build, checked after runtime image validation.
+// Supported game build, checked after runtime image validation.
 bool validate_contract(std::uintptr_t base) noexcept {
     struct Contract { std::uintptr_t rva; std::span<const std::uint8_t> bytes; };
     const Contract contracts[]{
@@ -135,7 +135,8 @@ bool validate_contract(std::uintptr_t base) noexcept {
         {layers::layout_bootstrap, layers::layout_bootstrap_prefix}};
     for (const auto& contract : contracts) {
         std::array<std::uint8_t, 32> actual{};
-        if (!read_memory(base + contract.rva, actual.data(), contract.bytes.size()) ||
+        if (contract.bytes.size() > actual.size() || // a longer prefix would overrun actual
+            !read_memory(base + contract.rva, actual.data(), contract.bytes.size()) ||
             std::memcmp(actual.data(), contract.bytes.data(), contract.bytes.size())) return false;
     }
     std::uintptr_t slot{};