From 2e6882ede1e072f99a412832c7764bc07c8cb2e2 Mon Sep 17 00:00:00 2001 From: ReSkate local Date: Mon, 5 Oct 2026 06:31:13 +0100 Subject: [PATCH] Music: guard the playlist lookup against unresolved song entries The music player's playlist walk loads a song entry, masks it and reads a flag at [rcx+0x14] before it checks the entry's identity. An unresolved or NULL entry makes that read fault and takes the game down while a playlist is built or opened. Install a 13-byte in-place guard at the reviewed RVA 0x14f80c7 when the music contracts pass. It keeps the original flag test, shrinks the leading mov eax,0 to xor eax,eax, and adds a jrcxz that skips the dereference when the entry is NULL, landing on the loop increment. The site is matched against its contract bytes before anything is written, so an unsupported image is left untouched, and the guard is applied with VirtualProtect and FlushInstructionCache and then made read-only again. The contract and patch bytes live with the other music contracts in Engine/Game/Build/20260929/local_music.h; the installer is Extension/Music/local_music_safety.{h,cpp}; local_music_ui.cpp invokes it from initialize_music_functions; cmake/Runtime.cmake builds it. A new dingosdk_music_safety_tests (CTest music_safety) pins the contract length, the decoded instructions and the branch target arithmetic without needing the game. --- Engine/Game/Build/20260929/local_music.h | 17 +++++ .../Music/Test/local_music_safety_tests.cpp | 40 +++++++++++ Extension/Music/local_music_safety.cpp | 68 +++++++++++++++++++ Extension/Music/local_music_safety.h | 10 +++ Extension/Music/local_music_ui.cpp | 5 ++ cmake/Apps.cmake | 4 ++ cmake/Runtime.cmake | 1 + 7 files changed, 145 insertions(+) create mode 100644 Extension/Music/Test/local_music_safety_tests.cpp create mode 100644 Extension/Music/local_music_safety.cpp create mode 100644 Extension/Music/local_music_safety.h diff --git a/Engine/Game/Build/20260929/local_music.h b/Engine/Game/Build/20260929/local_music.h index 99b1b57..da5c2da 100644 --- a/Engine/Game/Build/20260929/local_music.h +++ b/Engine/Game/Build/20260929/local_music.h @@ -2,6 +2,7 @@ #include "Engine/Game/Build/fingerprint.h" #include "engine.h" #include +#include #include namespace dingosdk::game::build::v20260929::local_music { @@ -49,4 +50,20 @@ inline constexpr std::array music_ui_contracts{{ {context_insert, {0x40,0x53,0x55,0x56,0x57,0x41,0x54,0x41,0x55,0x41,0x56,0x41,0x57,0x48,0x83,0xec,0x38,0x49,0x8b,0xf9,0x49,0x8b,0xd8,0x48,0x8b,0xf2,0x4c,0x8b,0xf9,0x33,0xed,0x48}}, {complete_delegate, {0x48,0x83,0xec,0x28,0x48,0x8b,0x11,0x48,0x8b,0x0a,0x48,0x8b,0x41,0x40,0x48,0x85,0xc0,0x74,0x0d,0x80,0x79,0x68,0x00,0x75,0x07,0xff,0xd0,0x48,0x83,0xc4,0x28,0xc3}} }}; +// Playlist lookup crash guard. The shipped loop loads a song entry (rcx), masks it with +// `and rcx, -5` and tests a flag at [rcx+0x14]; an unresolved or NULL entry makes that +// dereference fault and crashes the game. The 13-byte patch replaces the leading 5-byte +// `mov eax, 0` with a 2-byte `xor eax, eax`, a 2-byte `jrcxz` that jumps past the whole +// dereference when rcx is zero, and a padding `nop`, so the untouched tail stays aligned. +inline constexpr std::uintptr_t playlist_lookup_safety_rva = 0x14f80c7; +inline constexpr std::array playlist_lookup_safety_contract{ + 0xb8, 0x00, 0x00, 0x00, 0x00, 0x48, 0x83, 0xe1, 0xfb, 0x66, 0x85, 0x71, 0x14}; +inline constexpr std::array playlist_lookup_patch_bytes{ + 0x31, 0xc0, 0x48, 0x83, 0xe1, 0xfb, 0xe3, 0x29, 0x66, 0x85, 0x71, 0x14, 0x90}; +// The jrcxz sits at +6 (after the 2-byte xor and 4-byte and) and ends at +8; its +0x29 +// displacement lands on the loop increment (`inc r9d; add r8, 4`) instead of the fault. +inline constexpr std::size_t playlist_lookup_jump_next = 8; +inline constexpr std::size_t playlist_lookup_jump_displacement = 0x29; +inline constexpr std::uintptr_t playlist_lookup_jump_target = + playlist_lookup_safety_rva + playlist_lookup_jump_next + playlist_lookup_jump_displacement; } diff --git a/Extension/Music/Test/local_music_safety_tests.cpp b/Extension/Music/Test/local_music_safety_tests.cpp new file mode 100644 index 0000000..1c64bf8 --- /dev/null +++ b/Extension/Music/Test/local_music_safety_tests.cpp @@ -0,0 +1,40 @@ +// Contract tests for the playlist lookup crash guard. No game binary is needed: +// the patch site bytes, the jrcxz displacement and the branch target arithmetic +// are all fixed data of the supported build, so they are checked here. +#include "Engine/Game/Build/20260929/local_music.h" +#include +#include + +namespace music = dingosdk::game::build::v20260929::local_music; + +int main() { + int failures = 0; + const auto check = [&](bool ok, const char* what) { + if (!ok) { std::printf("FAIL: %s\n", what); ++failures; } + }; + + check(music::playlist_lookup_safety_contract.size() == 13, "the contract is 13 bytes"); + check(music::playlist_lookup_patch_bytes.size() == 13, "the patch is 13 bytes"); + check(music::playlist_lookup_safety_contract.size() == music::playlist_lookup_patch_bytes.size(), + "the patch replaces the contract byte for byte"); + check(music::playlist_lookup_safety_rva == 0x14f80c7, "the site is the reviewed RVA"); + + // The patch decodes as xor eax,eax; and rcx,-5; jrcxz; the preserved flag test; nop. + const auto& patch = music::playlist_lookup_patch_bytes; + check(patch[0] == 0x31 && patch[1] == 0xc0, "mov eax,0 becomes xor eax,eax"); + check(patch[2] == 0x48 && patch[3] == 0x83 && patch[4] == 0xe1 && patch[5] == 0xfb, + "and rcx,-5 is preserved"); + check(patch[6] == 0xe3, "jrcxz is installed after the mask"); + check(patch[7] == music::playlist_lookup_jump_displacement, "the jrcxz displacement is declared"); + check(patch[8] == 0x66 && patch[9] == 0x85 && patch[10] == 0x71 && patch[11] == 0x14, + "the [rcx+0x14] flag test is preserved"); + check(patch[12] == 0x90, "the trailing byte pads the 5-byte mov"); + + // The 2-byte jrcxz sits at +6, so its relative displacement is measured from +8. + check(music::playlist_lookup_jump_next == 8, "jrcxz ends 8 bytes into the site"); + check(music::playlist_lookup_jump_target == 0x14f80f8, + "the guard jumps to the loop increment (inc r9d; add r8,4)"); + + std::printf(failures ? "%d failure(s)\n" : "all passed\n", failures); + return failures ? 1 : 0; +} diff --git a/Extension/Music/local_music_safety.cpp b/Extension/Music/local_music_safety.cpp new file mode 100644 index 0000000..42a34fb --- /dev/null +++ b/Extension/Music/local_music_safety.cpp @@ -0,0 +1,68 @@ +#include "local_music_safety.h" +#include "Engine/Core/Log/logging.h" +#include "Engine/Core/Platform/memory.h" +#include "Engine/Game/Build/20260929/local_music.h" +#include "Engine/Game/Build/supported_build.h" +#include +#include +#include + +namespace dingosdk::profile_runtime { +namespace { +using namespace dingosdk::game::build::v20260929; +using local_music::playlist_lookup_patch_bytes; +using local_music::playlist_lookup_safety_contract; +using local_music::playlist_lookup_safety_rva; + +void report(logging::Level level, const char* message) noexcept { + logging::write(level, logging::Channel::music, message); +} + +bool write_patch(std::uintptr_t address, const std::array& bytes) noexcept { + DWORD previous{}; + auto* const destination = reinterpret_cast(address); + if (!VirtualProtect(destination, bytes.size(), PAGE_EXECUTE_READWRITE, &previous)) return false; + std::memcpy(destination, bytes.data(), bytes.size()); + const bool flushed = FlushInstructionCache(GetCurrentProcess(), destination, bytes.size()) != FALSE; + DWORD discarded{}; + const bool restored = VirtualProtect(destination, bytes.size(), previous, &discarded) != FALSE; + return flushed && restored; +} +} + +bool install_playlist_lookup_safety(std::uintptr_t base) noexcept { + const DWORD saved = GetLastError(); + constexpr auto size = playlist_lookup_safety_contract.size(); + const auto rva = playlist_lookup_safety_rva; + if (base == 0 || rva >= supported_build::game_image_size || + size > supported_build::game_image_size - rva) { + report(logging::Level::error, "Music playlist lookup safety patch: the site is outside the game image."); + SetLastError(saved); + return false; + } + const auto address = base + rva; + std::array actual{}; + if (!memory::read_bytes(address, actual.data(), actual.size())) { + report(logging::Level::error, "Music playlist lookup safety patch: the site is unreadable."); + SetLastError(saved); + return false; + } + if (actual == playlist_lookup_patch_bytes) { + SetLastError(saved); // Already guarded by an earlier install. + return true; + } + if (actual != playlist_lookup_safety_contract) { + report(logging::Level::error, "Music playlist lookup safety patch: the site does not match the supported build."); + SetLastError(saved); + return false; + } + if (!write_patch(address, playlist_lookup_patch_bytes)) { + report(logging::Level::error, "Music playlist lookup safety patch: could not make the site writable."); + SetLastError(saved); + return false; + } + logging::event(logging::Channel::music, "{\"event\":\"music_playlist_lookup_safety\"}"); + SetLastError(saved); + return true; +} +} diff --git a/Extension/Music/local_music_safety.h b/Extension/Music/local_music_safety.h new file mode 100644 index 0000000..653b82d --- /dev/null +++ b/Extension/Music/local_music_safety.h @@ -0,0 +1,10 @@ +#pragma once +#include + +namespace dingosdk::profile_runtime { +// Installs the in-place playlist-lookup crash guard described in +// Engine/Game/Build/20260929/local_music.h. Verifies the 13 contract bytes at +// `playlist_lookup_safety_rva` before writing, so an unsupported image is left +// untouched. Never throws; a failed install is logged and reported as false. +bool install_playlist_lookup_safety(std::uintptr_t base) noexcept; +} diff --git a/Extension/Music/local_music_ui.cpp b/Extension/Music/local_music_ui.cpp index b249242..975cd47 100644 --- a/Extension/Music/local_music_ui.cpp +++ b/Extension/Music/local_music_ui.cpp @@ -3,6 +3,7 @@ #include "Extension/Customization/local_customization_runtime.h" #include "Extension/Customization/local_player_card_runtime.h" #include "local_music_assets.h" +#include "local_music_safety.h" #include "local_music_ui.h" #include "Engine/Game/Abi/native_data.h" #include "Engine/Game/Build/addresses.h" @@ -42,6 +43,10 @@ bool initialize_music_functions(std::uintptr_t base) { if (!read(base + music::playlist_control_vtable, playlist) || !read(base + music::song_control_vtable, song) || playlist != rebased(music::playlist_control_slots) || song != rebased(music::song_control_slots)) return false; + // Guard the native playlist walk before any mod song can reach it. A failed + // install leaves music usable; the crash guard is a safety net, not a gate. + if (!install_playlist_lookup_safety(base)) + logging::event(logging::Channel::music, "{\"event\":\"music_playlist_lookup_safety_failed\"}"); auto& f = music_ui_runtime().functions; f.construct_playlist = reinterpret_cast(base + music::construct_playlist); f.construct_song = reinterpret_cast(base + music::construct_song); diff --git a/cmake/Apps.cmake b/cmake/Apps.cmake index 35c239c..75e6261 100644 --- a/cmake/Apps.cmake +++ b/cmake/Apps.cmake @@ -48,6 +48,10 @@ if(DINGOSDK_BUILD_LAUNCHER_TESTS AND WIN32) target_link_libraries(dingosdk_mod_music_tests PRIVATE dingosdk_mods) target_include_directories(dingosdk_mod_music_tests PRIVATE "${PROJECT_SOURCE_DIR}") add_test(NAME mod_music COMMAND dingosdk_mod_music_tests) + add_executable(dingosdk_music_safety_tests Extension/Music/Test/local_music_safety_tests.cpp) + target_include_directories(dingosdk_music_safety_tests PRIVATE "${PROJECT_SOURCE_DIR}") + dingosdk_configure_target(dingosdk_music_safety_tests) + add_test(NAME music_safety COMMAND dingosdk_music_safety_tests) add_executable(dingosdk_mod_merge_unshift_tests Engine/Vfs/Test/mod_merge_unshift_tests.cpp) target_link_libraries(dingosdk_mod_merge_unshift_tests PRIVATE dingosdk_mods) target_include_directories(dingosdk_mod_merge_unshift_tests PRIVATE "${PROJECT_SOURCE_DIR}") diff --git a/cmake/Runtime.cmake b/cmake/Runtime.cmake index 5989d22..56cc38d 100644 --- a/cmake/Runtime.cmake +++ b/cmake/Runtime.cmake @@ -141,6 +141,7 @@ add_library(dingosdk_runtime SHARED Extension/News/live_news.cpp Extension/Objects/local_object_runtime.cpp Extension/Music/local_music_assets.cpp + Extension/Music/local_music_safety.cpp Extension/Music/local_music_ui.cpp Extension/Objects/local_buildkit_labels.cpp Extension/Objects/local_buildkit_limits.cpp