diff --git a/src/Components/Loader.cpp b/src/Components/Loader.cpp index ae9bbde7..e8e6ab30 100644 --- a/src/Components/Loader.cpp +++ b/src/Components/Loader.cpp @@ -83,6 +83,7 @@ namespace Components Loader::Register(new ModelSurfs()); Loader::Register(new PlayerName()); Loader::Register(new QuickPatch()); + Loader::Register(new Security()); Loader::Register(new ServerInfo()); Loader::Register(new ServerList()); Loader::Register(new SlowMotion()); diff --git a/src/Components/Loader.hpp b/src/Components/Loader.hpp index 23f07b4e..34b278d9 100644 --- a/src/Components/Loader.hpp +++ b/src/Components/Loader.hpp @@ -85,6 +85,7 @@ namespace Components #include "Modules/Network.hpp" #include "Modules/Theatre.hpp" #include "Modules/QuickPatch.hpp" +#include "Modules/Security.hpp" #include "Modules/Node.hpp" #include "Modules/RCon.hpp" #include "Modules/Party.hpp" // Destroys the order, but requires network classes :D diff --git a/src/Components/Modules/PlayerName.cpp b/src/Components/Modules/PlayerName.cpp index d90cadf7..5f29acc0 100644 --- a/src/Components/Modules/PlayerName.cpp +++ b/src/Components/Modules/PlayerName.cpp @@ -8,7 +8,7 @@ namespace Components { if (!sv_allowColoredNames.get()) { - char nameBuffer[64] = { 0 }; + char nameBuffer[64] = {0}; TextRenderer::StripColors(name, nameBuffer, sizeof(nameBuffer)); TextRenderer::StripAllTextIcons(nameBuffer, buffer, size); } @@ -26,12 +26,12 @@ namespace Components } } - __declspec(naked) void PlayerName::ClientUserinfoChanged() + __declspec(naked) void PlayerName::ClientCleanName() { __asm { mov eax, [esp + 4h] // length - //sub eax, 1 + push eax push ecx // name @@ -53,12 +53,57 @@ namespace Components return buf; } + char* PlayerName::CleanStrStub(char* string) { TextRenderer::StripColors(string, string, strlen(string) + 1); return string; } + bool PlayerName::CopyClientNameCheck(char* dest, const char* source, int size) + { + Utils::Hook::Call(0x4D6F80)(dest, source, size); // I_strncpyz + + auto i = 0; + while (i < size - 1 && dest[i] != '\0') + { + if (dest[i] > 125 || dest[i] < 32 || dest[i] == '%') + { + return false; // Illegal string + } + + ++i; + } + + return true; + } + + __declspec(naked) void PlayerName::SV_UserinfoChangedStub() + { + __asm + { + call CopyClientNameCheck + test al, al + + jnz returnSafe + + pushad + + push 1 // tellThem + push INVALID_NAME_MSG // reason + push edi // drop + mov eax, 0x4D1600 // SV_DropClient + call eax + add esp, 0xC + + popad + + returnSafe: + push 0x401988 + retn + } + } + PlayerName::PlayerName() { sv_allowColoredNames = Dvar::Register("sv_allowColoredNames", true, Game::dvar_flag::DVAR_NONE, "Allow colored names on the server"); @@ -66,13 +111,17 @@ namespace Components // Disable SV_UpdateUserinfo_f, to block changing the name ingame Utils::Hook::Set(0x6258D0, 0xC3); - // Allow colored names ingame - Utils::Hook(0x5D8B40, ClientUserinfoChanged, HOOK_JUMP).install()->quick(); + // Allow colored names ingame. Hook placed in ClientUserinfoChanged + Utils::Hook(0x5D8B40, ClientCleanName, HOOK_JUMP).install()->quick(); // Though, don't apply that to overhead names. Utils::Hook(0x581932, GetClientName, HOOK_CALL).install()->quick(); // Patch I_CleanStr Utils::Hook(0x4AD470, CleanStrStub, HOOK_JUMP).install()->quick(); + + // Detect invalid characters including '%' to prevent format string vulnerabilities. + // Kicks the player as soon as possible + Utils::Hook(0x401983, SV_UserinfoChangedStub, HOOK_JUMP).install()->quick(); } } diff --git a/src/Components/Modules/PlayerName.hpp b/src/Components/Modules/PlayerName.hpp index 8335afda..b1beaaa7 100644 --- a/src/Components/Modules/PlayerName.hpp +++ b/src/Components/Modules/PlayerName.hpp @@ -11,9 +11,14 @@ namespace Components private: static Dvar::Var sv_allowColoredNames; + // Message used when kicking players + static constexpr auto INVALID_NAME_MSG = "Invalid name detected"; static char* CleanStrStub(char* string); - static void ClientUserinfoChanged(); + static void ClientCleanName(); static char* GetClientName(int localClientNum, int index, char* buf, size_t size); + + static bool CopyClientNameCheck(char* dest, const char* source, int size); + static void SV_UserinfoChangedStub(); }; } diff --git a/src/Components/Modules/QuickPatch.cpp b/src/Components/Modules/QuickPatch.cpp index d8c1d1f6..92e09546 100644 --- a/src/Components/Modules/QuickPatch.cpp +++ b/src/Components/Modules/QuickPatch.cpp @@ -47,62 +47,6 @@ namespace Components } } - int QuickPatch::MsgReadBitsCompressCheckSV(const char *from, char *to, int size) - { - static char buffer[0x8000]; - - if (size > 0x800) return 0; - size = Game::MSG_ReadBitsCompress(from, buffer, size); - - if (size > 0x800) return 0; - std::memcpy(to, buffer, size); - - return size; - } - - int QuickPatch::MsgReadBitsCompressCheckCL(const char *from, char *to, int size) - { - static char buffer[0x100000]; - - if (size > 0x20000) return 0; - size = Game::MSG_ReadBitsCompress(from, buffer, size); - - if (size > 0x20000) return 0; - std::memcpy(to, buffer, size); - - return size; - } - - int QuickPatch::SVCanReplaceServerCommand(Game::client_t* /*client*/, const char* /*cmd*/) - { - // This is a fix copied from V2. As I don't have time to investigate, let's simply trust them - return -1; - } - - long QuickPatch::AtolAdjustPlayerLimit(const char* string) - { - return std::min(atol(string), 18l); - } - - void QuickPatch::SelectStringTableEntryInDvarStub() - { - Command::ClientParams params; - - if (params.size() >= 4) - { - const auto* dvarName = params[3]; - const auto* dvar = Game::Dvar_FindVar(dvarName); - - if (Command::Find(dvarName) || - (dvar != nullptr && dvar->flags & (Game::DVAR_WRITEPROTECTED | Game::DVAR_CHEAT | Game::DVAR_READONLY))) - { - return; - } - } - - Game::CL_SelectStringTableEntryInDvar_f(); - } - __declspec(naked) void QuickPatch::JavelinResetHookStub() { __asm @@ -117,69 +61,6 @@ namespace Components } } - __declspec(naked) int QuickPatch::G_GetClientScore() - { - __asm - { - mov eax, [esp + 4] // index - mov ecx, ds : 1A831A8h // level: &g_clients - - test ecx, ecx; - jz invalid_ptr; - - imul eax, 366Ch - mov eax, [eax + ecx + 3134h] - ret - - invalid_ptr: - xor eax, eax - ret - } - } - - bool QuickPatch::InvalidNameCheck(char* dest, const char* source, int size) - { - Utils::Hook::Call(0x4D6F80)(dest, source, size); // I_strncpyz - - for (int i = 0; i < size - 1; i++) - { - if (!dest[i]) break; - - if (dest[i] > 125 || dest[i] < 32 || dest[i] == '%') - { - return false; - } - } - - return true; - } - - __declspec(naked) void QuickPatch::InvalidNameStub() - { - static const char* kick_reason = "Invalid name detected."; - - __asm - { - call InvalidNameCheck; - test al, al - - jnz returnSafe; - - pushad; - push 1; - push kick_reason; - push edi; - mov eax, 0x004D1600; // SV_DropClientInternal - call eax; - add esp, 12; - popad; - - returnSafe: - push 0x00401988; - retn; - } - } - Game::dvar_t* QuickPatch::g_antilag; __declspec(naked) void QuickPatch::ClientEventsFireWeaponStub() { @@ -378,9 +259,6 @@ namespace Components Utils::Hook(0x5D6D56, QuickPatch::ClientEventsFireWeaponStub, HOOK_JUMP).install()->quick(); Utils::Hook(0x5D6D6A, QuickPatch::ClientEventsFireWeaponMeleeStub, HOOK_JUMP).install()->quick(); - // Disallow invalid player names - Utils::Hook(0x401983, QuickPatch::InvalidNameStub, HOOK_JUMP).install()->quick(); - // Javelin fix Utils::Hook(0x578F52, QuickPatch::JavelinResetHookStub, HOOK_JUMP).install()->quick(); @@ -644,21 +522,6 @@ namespace Components } }); - // Exploit fixes - Utils::Hook::Set(0x412370, 0xC3); // SV_SteamAuthClient - Utils::Hook::Set(0x5A8C70, 0xC3); // CL_HandleRelayPacket - Utils::Hook(0x414D92, QuickPatch::MsgReadBitsCompressCheckSV, HOOK_CALL).install()->quick(); // SV_ExecuteClientCommands - Utils::Hook(0x4A9F56, QuickPatch::MsgReadBitsCompressCheckCL, HOOK_CALL).install()->quick(); // CL_ParseServerMessage - Utils::Hook(0x407376, QuickPatch::SVCanReplaceServerCommand , HOOK_CALL).install()->quick(); // SV_CanReplaceServerCommand - Utils::Hook(0x5B67ED, QuickPatch::AtolAdjustPlayerLimit , HOOK_CALL).install()->quick(); // PartyHost_HandleJoinPartyRequest - Utils::Hook::Nop(0x41698E, 5); // Disable Svcmd_EntityList_f - - // Patch selectStringTableEntryInDvar - Utils::Hook::Set(0x405959, QuickPatch::SelectStringTableEntryInDvarStub); - - // Patch G_GetClientScore for uninitialised game - Utils::Hook(0x469AC0, QuickPatch::G_GetClientScore, HOOK_JUMP).install()->quick(); - // Ignore call to print 'Offhand class mismatch when giving weapon...' Utils::Hook(0x5D9047, 0x4BB9B0, HOOK_CALL).install()->quick(); diff --git a/src/Components/Modules/QuickPatch.hpp b/src/Components/Modules/QuickPatch.hpp index 6eac85a2..62377cfd 100644 --- a/src/Components/Modules/QuickPatch.hpp +++ b/src/Components/Modules/QuickPatch.hpp @@ -12,21 +12,8 @@ namespace Components static void UnlockStats(); private: - static void SelectStringTableEntryInDvarStub(); - - static int SVCanReplaceServerCommand(Game::client_t *client, const char *cmd); - static int G_GetClientScore(); - - static int MsgReadBitsCompressCheckSV(const char *from, char *to, int size); - static int MsgReadBitsCompressCheckCL(const char *from, char *to, int size); - - static long AtolAdjustPlayerLimit(const char* string); - static void JavelinResetHookStub(); - static bool InvalidNameCheck(char* dest, const char* source, int size); - static void InvalidNameStub(); - static Dvar::Var r_customAspectRatio; static Game::dvar_t* Dvar_RegisterAspectRatioDvar(const char* dvarName, const char** valueList, int defaultIndex, unsigned __int16 flags, const char* description); static void SetAspectRatioStub(); diff --git a/src/Components/Modules/Security.cpp b/src/Components/Modules/Security.cpp new file mode 100644 index 00000000..b2493cf1 --- /dev/null +++ b/src/Components/Modules/Security.cpp @@ -0,0 +1,112 @@ +#include + +namespace Components +{ + int Security::MsgReadBitsCompressCheckSV(const char* from, char* to, int size) + { + static char buffer[0x8000]; + + if (size > 0x800) return 0; + size = Game::MSG_ReadBitsCompress(from, buffer, size); + + if (size > 0x800) return 0; + std::memcpy(to, buffer, size); + + return size; + } + + int Security::MsgReadBitsCompressCheckCL(const char* from, char* to, int size) + { + static char buffer[0x100000]; + + if (size > 0x20000) return 0; + size = Game::MSG_ReadBitsCompress(from, buffer, size); + + if (size > 0x20000) return 0; + std::memcpy(to, buffer, size); + + return size; + } + + int Security::SVCanReplaceServerCommand(Game::client_t* /*client*/, const char* /*cmd*/) + { + // This is a fix copied from V2. As I don't have time to investigate, let's simply trust them + return -1; + } + + long Security::AtolAdjustPlayerLimit(const char* string) + { + return std::min(std::atol(string), 18); + } + + void Security::SelectStringTableEntryInDvarStub() + { + Command::ClientParams params; + + if (params.size() >= 4) + { + const auto* dvarName = params[3]; + const auto* dvar = Game::Dvar_FindVar(dvarName); + + if (Command::Find(dvarName) || + (dvar != nullptr && dvar->flags & (Game::DVAR_WRITEPROTECTED | Game::DVAR_CHEAT | Game::DVAR_READONLY))) + { + Logger::Print(0, "CL_SelectStringTableEntryInDvar_f: illegal parameter\n"); + return; + } + } + + Game::CL_SelectStringTableEntryInDvar_f(); + } + + __declspec(naked) int Security::G_GetClientScore() + { + __asm + { + mov eax, [esp + 4] // index + mov ecx, ds:1A831A8h // level: &g_clients + + test ecx, ecx + jz invalid_ptr + + imul eax, 366Ch + mov eax, [eax + ecx + 3134h] + ret + + invalid_ptr: + xor eax, eax + ret + } + } + + void Security::G_LogPrintfStub(const char* fmt) + { + Game::G_LogPrintf("%s", fmt); + } + + Security::Security() + { + // Exploit fixes + Utils::Hook(0x414D92, MsgReadBitsCompressCheckSV, HOOK_CALL).install()->quick(); // SV_ExecuteClientCommands + Utils::Hook(0x4A9F56, MsgReadBitsCompressCheckCL, HOOK_CALL).install()->quick(); // CL_ParseServerMessage + Utils::Hook(0x407376, SVCanReplaceServerCommand, HOOK_CALL).install()->quick(); // SV_CanReplaceServerCommand + + Utils::Hook::Set(0x412370, 0xC3); // SV_SteamAuthClient + Utils::Hook::Set(0x5A8C70, 0xC3); // CL_HandleRelayPacket + + Utils::Hook::Nop(0x41698E, 5); // Disable Svcmd_EntityList_f + + // Patch selectStringTableEntryInDvar + Utils::Hook::Set(0x405959, Security::SelectStringTableEntryInDvarStub); + + // Patch G_GetClientScore for uninitialized game + Utils::Hook(0x469AC0, G_GetClientScore, HOOK_JUMP).install()->quick(); + + // Requests can be malicious + Utils::Hook(0x5B67ED, AtolAdjustPlayerLimit, HOOK_CALL).install()->quick(); // PartyHost_HandleJoinPartyRequest + + // Patch unsecure call to G_LogPrint inside GScr_LogPrint + // This function is unsafe because IW devs forgot to G_LogPrintf("%s", fmt) + Utils::Hook(0x5F70B5, G_LogPrintfStub, HOOK_CALL).install()->quick(); + } +} diff --git a/src/Components/Modules/Security.hpp b/src/Components/Modules/Security.hpp new file mode 100644 index 00000000..82c605bd --- /dev/null +++ b/src/Components/Modules/Security.hpp @@ -0,0 +1,24 @@ +#pragma once + +namespace Components +{ + class Security : public Component + { + public: + Security(); + + private: + static int MsgReadBitsCompressCheckSV(const char* from, char* to, int size); + static int MsgReadBitsCompressCheckCL(const char* from, char* to, int size); + + static int SVCanReplaceServerCommand(Game::client_t* client, const char* cmd); + + static long AtolAdjustPlayerLimit(const char* string); + + static void SelectStringTableEntryInDvarStub(); + + static int G_GetClientScore(); + + static void G_LogPrintfStub(const char* fmt); + }; +} diff --git a/src/Components/Modules/TextRenderer.cpp b/src/Components/Modules/TextRenderer.cpp index 2a11a676..2b7fcbf2 100644 --- a/src/Components/Modules/TextRenderer.cpp +++ b/src/Components/Modules/TextRenderer.cpp @@ -1356,11 +1356,11 @@ namespace Components if (*in) // height in++; - if(*in) // material name length + material name characters + if (*in) // material name length + material name characters { const auto materialNameLength = *in; in++; - for(auto i = 0; i < materialNameLength; i++) + for (auto i = 0; i < materialNameLength; i++) { if (*in) in++; @@ -1370,7 +1370,7 @@ namespace Components continue; } - if(*in == FONT_ICON_SEPARATOR_CHARACTER) + if (*in == FONT_ICON_SEPARATOR_CHARACTER) { const auto* fontIconEndPos = &in[1]; FontIconInfo fontIcon{}; @@ -1386,6 +1386,7 @@ namespace Components ++current; ++in; } + *out = '\0'; } diff --git a/src/Game/Functions.cpp b/src/Game/Functions.cpp index e368ad5e..ecebf15a 100644 --- a/src/Game/Functions.cpp +++ b/src/Game/Functions.cpp @@ -153,6 +153,7 @@ namespace Game FS_IsShippedIWD_t FS_IsShippedIWD = FS_IsShippedIWD_t(0x642440); FS_Delete_t FS_Delete = FS_Delete_t(0x48A5B0); + G_LogPrintf_t G_LogPrintf = G_LogPrintf_t(0x4B0150); G_GetWeaponIndexForName_t G_GetWeaponIndexForName = G_GetWeaponIndexForName_t(0x49E540); G_SpawnEntitiesFromString_t G_SpawnEntitiesFromString = G_SpawnEntitiesFromString_t(0x4D8840); G_PrintEntities_t G_PrintEntities = G_PrintEntities_t(0x4E6A50); diff --git a/src/Game/Functions.hpp b/src/Game/Functions.hpp index c65f8cb1..8da04c3d 100644 --- a/src/Game/Functions.hpp +++ b/src/Game/Functions.hpp @@ -380,6 +380,9 @@ namespace Game typedef int(__cdecl* FS_Delete_t)(const char* fileName); extern FS_Delete_t FS_Delete; + typedef void(__cdecl * G_LogPrintf_t)(const char* fmt, ...); + extern G_LogPrintf_t G_LogPrintf; + typedef unsigned int(__cdecl * G_GetWeaponIndexForName_t)(const char*); extern G_GetWeaponIndexForName_t G_GetWeaponIndexForName; diff --git a/src/Utils/String.hpp b/src/Utils/String.hpp index 51012e8a..72af16e6 100644 --- a/src/Utils/String.hpp +++ b/src/Utils/String.hpp @@ -11,7 +11,7 @@ namespace Utils static_assert(Buffers != 0 && MinBufferSize != 0, "Buffers and MinBufferSize mustn't be 0"); VAProvider() : currentBuffer(0) {} - ~VAProvider() {} + ~VAProvider() = default; const char* get(const char* format, va_list ap) { @@ -25,7 +25,7 @@ namespace Utils while (true) { - int res = vsnprintf_s(entry->buffer, entry->size, _TRUNCATE, format, ap); + const auto res = _vsnprintf_s(entry->buffer, entry->size, _TRUNCATE, format, ap); if (res > 0) break; // Success if (res == 0) return ""; // Error