From e0995c822b8a1123651b1a6b694b140413b59f2a Mon Sep 17 00:00:00 2001 From: Federico Romero Date: Wed, 19 Aug 2026 12:54:58 -0300 Subject: [PATCH 1/4] Fix crash in CPathFind's per-area path node loader CPathFind's per-area path node loader calls malloc() for m_pPathNodes[area] without checking the result, then loops over it using the stored node count. If the allocation fails, that loop dereferences a null pointer and crashes. This hook checks whether m_pPathNodes[area] is null right where the node count is loaded for the loop's bound check, forcing it to 0 so the loop is skipped instead of crashing. Both hooks in this file live well past the address range MTA's shared Mem*Fast/IsSlowMem helpers normally cover, so they install through the Fast path directly (with a matching unprotect call added for that page) instead of the general hook installer, which would otherwise assert in debug builds. --- Client/game_sa/CGameSA.cpp | 2 + Client/game_sa/CPathFindSA.cpp | 89 +++++++++++++++++++++++++++++ Client/game_sa/CPathFindSA.h | 18 ++++++ Shared/sdk/SharedUtil.MemAccess.hpp | 3 + 4 files changed, 112 insertions(+) create mode 100644 Client/game_sa/CPathFindSA.cpp create mode 100644 Client/game_sa/CPathFindSA.h diff --git a/Client/game_sa/CGameSA.cpp b/Client/game_sa/CGameSA.cpp index b9ef4b1fff8..5fd2281ddcd 100644 --- a/Client/game_sa/CGameSA.cpp +++ b/Client/game_sa/CGameSA.cpp @@ -39,6 +39,7 @@ #include "CKeyGenSA.h" #include "CObjectGroupPhysicalPropertiesSA.h" #include "CPadSA.h" +#include "CPathFindSA.h" #include "CPickupsSA.h" #include "CPlayerInfoSA.h" #include "CPointLightsSA.h" @@ -238,6 +239,7 @@ CGameSA::CGameSA() CPlayerPedSA::StaticSetHooks(); CRenderWareSA::StaticSetHooks(); CRenderWareSA::StaticSetClothesReplacingHooks(); + CPathFindSA::StaticSetHooks(); CTasksSA::StaticSetHooks(); CPedSA::StaticSetHooks(); CSettingsSA::StaticSetHooks(); diff --git a/Client/game_sa/CPathFindSA.cpp b/Client/game_sa/CPathFindSA.cpp new file mode 100644 index 00000000000..7557fe79f4e --- /dev/null +++ b/Client/game_sa/CPathFindSA.cpp @@ -0,0 +1,89 @@ +/***************************************************************************** + * + * PROJECT: Multi Theft Auto v1.0 + * LICENSE: See LICENSE in the top level directory + * FILE: game_sa/CPathFindSA.cpp + * PURPOSE: Path find (CPathFind) hooks + * + * Multi Theft Auto is available from https://www.multitheftauto.com/ + * + *****************************************************************************/ + +#include "StdInc.h" +#include "CPathFindSA.h" + +// CPathFind's per map area path node loader calls malloc() for m_pPathNodes[area] (offset +// 0x804) without checking the result, then loops over it using m_dwNumNodes[area] (offset +// 0xfa4) as the count. If the allocation fails, that loop dereferences a null pointer and +// crashes. +// +// Right where the node count gets loaded for the loop's bound check, this hook also checks +// whether m_pPathNodes[area] is null, and if so forces the count to 0 so the loop is skipped. +#define HOOKPOS_CPathFind_LoadPathNodeCount_Mid 0x0156F966 +#define HOOKSIZE_CPathFind_LoadPathNodeCount_Mid 7 +DWORD RETURN_CPathFind_LoadPathNodeCount_Mid = 0x0156F96D; +static void __declspec(naked) HOOK_CPathFind_LoadPathNodeCount_Mid() +{ + MTA_VERIFY_HOOK_LOCAL_SIZE; + + // clang-format off + __asm + { + // Replicate the overwritten instruction: EAX = m_dwNumNodes[area] + mov eax, dword ptr [esi + edi*4 + 0x0fa4] + + // If m_pPathNodes[area] failed to allocate, force the node count to 0 for this loop + cmp dword ptr [esi + edi*4 + 0x804], 0 + jne nodesOk + xor eax, eax + nodesOk: + jmp RETURN_CPathFind_LoadPathNodeCount_Mid + } + // clang-format on +} + +// Test-only: deliberately reproduces the crash above by forcing every m_pPathNodes[area] +// allocation to look like it failed, the moment after it's stored. Off unless explicitly +// compiled in, so it can never affect a normal build. Used to confirm the crash happens without +// the fix above (undo HOOK_CPathFind_LoadPathNodeCount_Mid's install to test that) and is gone +// with it. +// #define MTA_DEBUG_REPRO_PATHFIND_NULL_CRASH + +#ifdef MTA_DEBUG_REPRO_PATHFIND_NULL_CRASH + #define HOOKPOS_CPathFind_ReproForceNull_Mid 0x0156F7F6 + #define HOOKSIZE_CPathFind_ReproForceNull_Mid 7 +DWORD RETURN_CPathFind_ReproForceNull_Mid = 0x0156F7FD; +static void __declspec(naked) HOOK_CPathFind_ReproForceNull_Mid() +{ + MTA_VERIFY_HOOK_LOCAL_SIZE; + + // clang-format off + __asm + { + // Replicate the overwritten instruction (unrelated field, must still run) + mov eax, dword ptr [esi + edi*4 + 0x1304] + + // Force the just-stored m_pPathNodes[area] to null, as if malloc() had failed + mov dword ptr [esi + edi*4 + 0x804], 0 + + jmp RETURN_CPathFind_ReproForceNull_Mid + } + // clang-format on +} +#endif + +// Both hooks above live past 0x8A4000, past the range IsSlowMem normally covers, so they +// install through the Fast path instead of EZHookInstall (see SetInitialVirtualProtect). +void CPathFindSA::StaticSetHooks() +{ + BYTE jumpBytes[MAX_JUMPCODE_SIZE]; + MemSetFast(jumpBytes, 0x90, MAX_JUMPCODE_SIZE); + CreateJump(HOOKPOS_CPathFind_LoadPathNodeCount_Mid, (DWORD)HOOK_CPathFind_LoadPathNodeCount_Mid, jumpBytes); + MemCpyFast((PVOID)HOOKPOS_CPathFind_LoadPathNodeCount_Mid, jumpBytes, HOOKSIZE_CPathFind_LoadPathNodeCount_Mid); + +#ifdef MTA_DEBUG_REPRO_PATHFIND_NULL_CRASH + MemSetFast(jumpBytes, 0x90, MAX_JUMPCODE_SIZE); + CreateJump(HOOKPOS_CPathFind_ReproForceNull_Mid, (DWORD)HOOK_CPathFind_ReproForceNull_Mid, jumpBytes); + MemCpyFast((PVOID)HOOKPOS_CPathFind_ReproForceNull_Mid, jumpBytes, HOOKSIZE_CPathFind_ReproForceNull_Mid); +#endif +} diff --git a/Client/game_sa/CPathFindSA.h b/Client/game_sa/CPathFindSA.h new file mode 100644 index 00000000000..2f81bfbedf1 --- /dev/null +++ b/Client/game_sa/CPathFindSA.h @@ -0,0 +1,18 @@ +/***************************************************************************** + * + * PROJECT: Multi Theft Auto v1.0 + * LICENSE: See LICENSE in the top level directory + * FILE: game_sa/CPathFindSA.h + * PURPOSE: Header file for path find (CPathFind) hooks + * + * Multi Theft Auto is available from https://www.multitheftauto.com/ + * + *****************************************************************************/ + +#pragma once + +class CPathFindSA +{ +public: + static void StaticSetHooks(); +}; diff --git a/Shared/sdk/SharedUtil.MemAccess.hpp b/Shared/sdk/SharedUtil.MemAccess.hpp index e4a487e0f69..bf831fa1f11 100644 --- a/Shared/sdk/SharedUtil.MemAccess.hpp +++ b/Shared/sdk/SharedUtil.MemAccess.hpp @@ -49,6 +49,9 @@ namespace SharedUtil VirtualProtect((LPVOID)0x6AE000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); VirtualProtect((LPVOID)0x729000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); VirtualProtect((LPVOID)0x742000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); + + // Used by CPathFind's hooks in game_sa/CPathFindSA.cpp, installed via the Fast path. + VirtualProtect((LPVOID)0x156F000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); } // Returns true if pAddr is in slow mem area From c233394fded373f87f02a47d81fd9c4ec2e18b78 Mon Sep 17 00:00:00 2001 From: Federico Romero Date: Wed, 19 Aug 2026 20:10:00 -0300 Subject: [PATCH 2/4] Remove forced crash --- Client/game_sa/CPathFindSA.cpp | 41 +++------------------------------- 1 file changed, 3 insertions(+), 38 deletions(-) diff --git a/Client/game_sa/CPathFindSA.cpp b/Client/game_sa/CPathFindSA.cpp index 7557fe79f4e..5c06d2464cd 100644 --- a/Client/game_sa/CPathFindSA.cpp +++ b/Client/game_sa/CPathFindSA.cpp @@ -42,48 +42,13 @@ static void __declspec(naked) HOOK_CPathFind_LoadPathNodeCount_Mid() // clang-format on } -// Test-only: deliberately reproduces the crash above by forcing every m_pPathNodes[area] -// allocation to look like it failed, the moment after it's stored. Off unless explicitly -// compiled in, so it can never affect a normal build. Used to confirm the crash happens without -// the fix above (undo HOOK_CPathFind_LoadPathNodeCount_Mid's install to test that) and is gone -// with it. -// #define MTA_DEBUG_REPRO_PATHFIND_NULL_CRASH - -#ifdef MTA_DEBUG_REPRO_PATHFIND_NULL_CRASH - #define HOOKPOS_CPathFind_ReproForceNull_Mid 0x0156F7F6 - #define HOOKSIZE_CPathFind_ReproForceNull_Mid 7 -DWORD RETURN_CPathFind_ReproForceNull_Mid = 0x0156F7FD; -static void __declspec(naked) HOOK_CPathFind_ReproForceNull_Mid() -{ - MTA_VERIFY_HOOK_LOCAL_SIZE; - - // clang-format off - __asm - { - // Replicate the overwritten instruction (unrelated field, must still run) - mov eax, dword ptr [esi + edi*4 + 0x1304] - - // Force the just-stored m_pPathNodes[area] to null, as if malloc() had failed - mov dword ptr [esi + edi*4 + 0x804], 0 - - jmp RETURN_CPathFind_ReproForceNull_Mid - } - // clang-format on -} -#endif - -// Both hooks above live past 0x8A4000, past the range IsSlowMem normally covers, so they -// install through the Fast path instead of EZHookInstall (see SetInitialVirtualProtect). +// This address is further into the executable than IsSlowMem recognizes, so the normal +// EZHookInstall path would assert in Debug builds. Unprotected up front in +// SetInitialVirtualProtect and installed via the Fast path instead, like the other hot addresses. void CPathFindSA::StaticSetHooks() { BYTE jumpBytes[MAX_JUMPCODE_SIZE]; MemSetFast(jumpBytes, 0x90, MAX_JUMPCODE_SIZE); CreateJump(HOOKPOS_CPathFind_LoadPathNodeCount_Mid, (DWORD)HOOK_CPathFind_LoadPathNodeCount_Mid, jumpBytes); MemCpyFast((PVOID)HOOKPOS_CPathFind_LoadPathNodeCount_Mid, jumpBytes, HOOKSIZE_CPathFind_LoadPathNodeCount_Mid); - -#ifdef MTA_DEBUG_REPRO_PATHFIND_NULL_CRASH - MemSetFast(jumpBytes, 0x90, MAX_JUMPCODE_SIZE); - CreateJump(HOOKPOS_CPathFind_ReproForceNull_Mid, (DWORD)HOOK_CPathFind_ReproForceNull_Mid, jumpBytes); - MemCpyFast((PVOID)HOOKPOS_CPathFind_ReproForceNull_Mid, jumpBytes, HOOKSIZE_CPathFind_ReproForceNull_Mid); -#endif } From f5ba8e3d3672dceec7e75b2c62be53462eb365d4 Mon Sep 17 00:00:00 2001 From: TheCrazy17 Date: Sat, 29 Aug 2026 19:34:34 -0300 Subject: [PATCH 3/4] Add HookInstallFast for hooks in pre unprotected memory Mirrors HookInstall but copies with MemCpyFast, for install addresses outside the ranges MemCpy knows how to unprotect. --- Client/game_sa/CPathFindSA.cpp | 7 ++----- Client/game_sa/HookSystem.h | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Client/game_sa/CPathFindSA.cpp b/Client/game_sa/CPathFindSA.cpp index 5c06d2464cd..c4167fb3a4e 100644 --- a/Client/game_sa/CPathFindSA.cpp +++ b/Client/game_sa/CPathFindSA.cpp @@ -44,11 +44,8 @@ static void __declspec(naked) HOOK_CPathFind_LoadPathNodeCount_Mid() // This address is further into the executable than IsSlowMem recognizes, so the normal // EZHookInstall path would assert in Debug builds. Unprotected up front in -// SetInitialVirtualProtect and installed via the Fast path instead, like the other hot addresses. +// SetInitialVirtualProtect and installed with HookInstallFast void CPathFindSA::StaticSetHooks() { - BYTE jumpBytes[MAX_JUMPCODE_SIZE]; - MemSetFast(jumpBytes, 0x90, MAX_JUMPCODE_SIZE); - CreateJump(HOOKPOS_CPathFind_LoadPathNodeCount_Mid, (DWORD)HOOK_CPathFind_LoadPathNodeCount_Mid, jumpBytes); - MemCpyFast((PVOID)HOOKPOS_CPathFind_LoadPathNodeCount_Mid, jumpBytes, HOOKSIZE_CPathFind_LoadPathNodeCount_Mid); + HookInstallFast(HOOKPOS_CPathFind_LoadPathNodeCount_Mid, (DWORD)HOOK_CPathFind_LoadPathNodeCount_Mid, HOOKSIZE_CPathFind_LoadPathNodeCount_Mid); } diff --git a/Client/game_sa/HookSystem.h b/Client/game_sa/HookSystem.h index a67466a40d3..f29a6aeae95 100644 --- a/Client/game_sa/HookSystem.h +++ b/Client/game_sa/HookSystem.h @@ -74,6 +74,24 @@ bool HookInstall(DWORD dwInstallAddress, T dwHookHandler, int iJmpCodeSize = 5) } } +// For install addresses outside the ranges MemCpy knows how to unprotect; the page must have +// been made writable beforehand (see SetInitialVirtualProtect) +template +bool HookInstallFast(DWORD dwInstallAddress, T dwHookHandler, int iJmpCodeSize = 5) +{ + BYTE JumpBytes[MAX_JUMPCODE_SIZE]; + MemSetFast(JumpBytes, 0x90, MAX_JUMPCODE_SIZE); + if (CreateJump(dwInstallAddress, (DWORD)FunctionPointerToVoidP(dwHookHandler), JumpBytes)) + { + MemCpyFast((PVOID)dwInstallAddress, JumpBytes, iJmpCodeSize); + return true; + } + else + { + return false; + } +} + // Auto detect requirement of US/EU hook installation #define EZHookInstall(type) HookInstall(HOOKPOS_##type, (DWORD)HOOK_##type, HOOKSIZE_##type); From edd852ebdcd5d55abdd02691b1d19cb8e661dd33 Mon Sep 17 00:00:00 2001 From: TheCrazy17 Date: Thu, 10 Sep 2026 21:43:43 -0300 Subject: [PATCH 4/4] Move CPathFind crash fix hook into CMultiplayerSA_CrashFixHacks Moved per FileEX's review comment on PR #5245. --- Client/game_sa/CGameSA.cpp | 2 - Client/game_sa/CPathFindSA.cpp | 51 ------------------- Client/game_sa/CPathFindSA.h | 18 ------- .../CMultiplayerSA_CrashFixHacks.cpp | 39 ++++++++++++++ Shared/sdk/SharedUtil.MemAccess.hpp | 2 +- 5 files changed, 40 insertions(+), 72 deletions(-) delete mode 100644 Client/game_sa/CPathFindSA.cpp delete mode 100644 Client/game_sa/CPathFindSA.h diff --git a/Client/game_sa/CGameSA.cpp b/Client/game_sa/CGameSA.cpp index 5fd2281ddcd..b9ef4b1fff8 100644 --- a/Client/game_sa/CGameSA.cpp +++ b/Client/game_sa/CGameSA.cpp @@ -39,7 +39,6 @@ #include "CKeyGenSA.h" #include "CObjectGroupPhysicalPropertiesSA.h" #include "CPadSA.h" -#include "CPathFindSA.h" #include "CPickupsSA.h" #include "CPlayerInfoSA.h" #include "CPointLightsSA.h" @@ -239,7 +238,6 @@ CGameSA::CGameSA() CPlayerPedSA::StaticSetHooks(); CRenderWareSA::StaticSetHooks(); CRenderWareSA::StaticSetClothesReplacingHooks(); - CPathFindSA::StaticSetHooks(); CTasksSA::StaticSetHooks(); CPedSA::StaticSetHooks(); CSettingsSA::StaticSetHooks(); diff --git a/Client/game_sa/CPathFindSA.cpp b/Client/game_sa/CPathFindSA.cpp deleted file mode 100644 index c4167fb3a4e..00000000000 --- a/Client/game_sa/CPathFindSA.cpp +++ /dev/null @@ -1,51 +0,0 @@ -/***************************************************************************** - * - * PROJECT: Multi Theft Auto v1.0 - * LICENSE: See LICENSE in the top level directory - * FILE: game_sa/CPathFindSA.cpp - * PURPOSE: Path find (CPathFind) hooks - * - * Multi Theft Auto is available from https://www.multitheftauto.com/ - * - *****************************************************************************/ - -#include "StdInc.h" -#include "CPathFindSA.h" - -// CPathFind's per map area path node loader calls malloc() for m_pPathNodes[area] (offset -// 0x804) without checking the result, then loops over it using m_dwNumNodes[area] (offset -// 0xfa4) as the count. If the allocation fails, that loop dereferences a null pointer and -// crashes. -// -// Right where the node count gets loaded for the loop's bound check, this hook also checks -// whether m_pPathNodes[area] is null, and if so forces the count to 0 so the loop is skipped. -#define HOOKPOS_CPathFind_LoadPathNodeCount_Mid 0x0156F966 -#define HOOKSIZE_CPathFind_LoadPathNodeCount_Mid 7 -DWORD RETURN_CPathFind_LoadPathNodeCount_Mid = 0x0156F96D; -static void __declspec(naked) HOOK_CPathFind_LoadPathNodeCount_Mid() -{ - MTA_VERIFY_HOOK_LOCAL_SIZE; - - // clang-format off - __asm - { - // Replicate the overwritten instruction: EAX = m_dwNumNodes[area] - mov eax, dword ptr [esi + edi*4 + 0x0fa4] - - // If m_pPathNodes[area] failed to allocate, force the node count to 0 for this loop - cmp dword ptr [esi + edi*4 + 0x804], 0 - jne nodesOk - xor eax, eax - nodesOk: - jmp RETURN_CPathFind_LoadPathNodeCount_Mid - } - // clang-format on -} - -// This address is further into the executable than IsSlowMem recognizes, so the normal -// EZHookInstall path would assert in Debug builds. Unprotected up front in -// SetInitialVirtualProtect and installed with HookInstallFast -void CPathFindSA::StaticSetHooks() -{ - HookInstallFast(HOOKPOS_CPathFind_LoadPathNodeCount_Mid, (DWORD)HOOK_CPathFind_LoadPathNodeCount_Mid, HOOKSIZE_CPathFind_LoadPathNodeCount_Mid); -} diff --git a/Client/game_sa/CPathFindSA.h b/Client/game_sa/CPathFindSA.h deleted file mode 100644 index 2f81bfbedf1..00000000000 --- a/Client/game_sa/CPathFindSA.h +++ /dev/null @@ -1,18 +0,0 @@ -/***************************************************************************** - * - * PROJECT: Multi Theft Auto v1.0 - * LICENSE: See LICENSE in the top level directory - * FILE: game_sa/CPathFindSA.h - * PURPOSE: Header file for path find (CPathFind) hooks - * - * Multi Theft Auto is available from https://www.multitheftauto.com/ - * - *****************************************************************************/ - -#pragma once - -class CPathFindSA -{ -public: - static void StaticSetHooks(); -}; diff --git a/Client/multiplayer_sa/CMultiplayerSA_CrashFixHacks.cpp b/Client/multiplayer_sa/CMultiplayerSA_CrashFixHacks.cpp index 27f4936fa65..daac9f42881 100644 --- a/Client/multiplayer_sa/CMultiplayerSA_CrashFixHacks.cpp +++ b/Client/multiplayer_sa/CMultiplayerSA_CrashFixHacks.cpp @@ -4092,6 +4092,40 @@ static int _cdecl CFileLoader_LoadVehicleObject_sscanf(const char* s, const char rearWheelSize, wheelUpgradeClass); } +////////////////////////////////////////////////////////////////////////////////////////// +// +// CPathFind's per map area path node loader calls malloc() for m_pPathNodes[area] (offset +// 0x804) without checking the result, then loops over it using m_dwNumNodes[area] (offset +// 0xfa4) as the count. If the allocation fails, that loop dereferences a null pointer and +// crashes. +// +// Right where the node count gets loaded for the loop's bound check, this hook also checks +// whether m_pPathNodes[area] is null, and if so forces the count to 0 so the loop is skipped. +// +////////////////////////////////////////////////////////////////////////////////////////// +#define HOOKPOS_CPathFind_LoadPathNodeCount_Mid 0x0156F966 +#define HOOKSIZE_CPathFind_LoadPathNodeCount_Mid 7 +DWORD RETURN_CPathFind_LoadPathNodeCount_Mid = 0x0156F96D; +static void __declspec(naked) HOOK_CPathFind_LoadPathNodeCount_Mid() +{ + MTA_VERIFY_HOOK_LOCAL_SIZE; + + // clang-format off + __asm + { + // Replicate the overwritten instruction: EAX = m_dwNumNodes[area] + mov eax, dword ptr [esi + edi*4 + 0x0fa4] + + // If m_pPathNodes[area] failed to allocate, force the node count to 0 for this loop + cmp dword ptr [esi + edi*4 + 0x804], 0 + jne nodesOk + xor eax, eax + nodesOk: + jmp RETURN_CPathFind_LoadPathNodeCount_Mid + } + // clang-format on +} + ////////////////////////////////////////////////////////////////////////////////////////// // // Setup hooks for CrashFixHacks @@ -4191,4 +4225,9 @@ void CMultiplayerSA::InitHooks_CrashFixHacks() // Fix uninitialized wheel scale in CFileLoader::LoadVehicleObject on Win11 24H2 HookInstallCall(CALL_CFileLoader_LoadVehicleObject_sscanf, (DWORD)CFileLoader_LoadVehicleObject_sscanf); + + // This address sits further into the executable than EZHookInstall's normal range check + // allows, so it is installed with HookInstallFast instead; the page is unprotected up front + // in SetInitialVirtualProtect + HookInstallFast(HOOKPOS_CPathFind_LoadPathNodeCount_Mid, (DWORD)HOOK_CPathFind_LoadPathNodeCount_Mid, HOOKSIZE_CPathFind_LoadPathNodeCount_Mid); } diff --git a/Shared/sdk/SharedUtil.MemAccess.hpp b/Shared/sdk/SharedUtil.MemAccess.hpp index bf831fa1f11..3e3ac4af822 100644 --- a/Shared/sdk/SharedUtil.MemAccess.hpp +++ b/Shared/sdk/SharedUtil.MemAccess.hpp @@ -50,7 +50,7 @@ namespace SharedUtil VirtualProtect((LPVOID)0x729000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); VirtualProtect((LPVOID)0x742000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); - // Used by CPathFind's hooks in game_sa/CPathFindSA.cpp, installed via the Fast path. + // Used by the CPathFind crash fix in multiplayer_sa/CMultiplayerSA_CrashFixHacks.cpp, installed via the Fast path. VirtualProtect((LPVOID)0x156F000, 0x1000, PAGE_EXECUTE_READWRITE, &oldProt); }