completely remove old hooking

1 files changed, 74 insertions, 102 deletions

diff --git a/NorthstarDLL/ExploitFixes.cpp b/NorthstarDLL/ExploitFixes.cpp
index 8f41ac32..b347310c 100644
--- a/NorthstarDLL/ExploitFixes.cpp
+++ b/NorthstarDLL/ExploitFixes.cpp
@@ -1,11 +1,12 @@
 #include "pch.h"

 

-#include "ExploitFixes.h"

 #include "ExploitFixes_UTF8Parser.h"

 #include "NSMem.h"

 #include "cvar.h"

 #include "tier0.h"

 

+AUTOHOOK_INIT()

+

 ConVar* ns_exploitfixes_log;

 #define SHOULD_LOG (ns_exploitfixes_log->m_Value.m_nValue > 0)

 #define BLOCKED_INFO(s)                                                                                                                    \

@@ -56,20 +57,29 @@ struct Angle
 	}

 };

 

-#define BLOCK_NETMSG_FUNC(name, pattern)                                                                                                   \

-	KHOOK(name, ("engine.dll", pattern), bool, __fastcall, (void* thisptr, void* buffer))                                                  \

-	{                                                                                                                                      \

-		return false;                                                                                                                      \

-	}

-

+// block bad netmessages

 // Servers can literally request a screenshot from any client, yeah no

-BLOCK_NETMSG_FUNC(CLC_Screenshot_WriteToBuffer, "48 89 5C 24 ? 57 48 83 EC 20 8B 42 10");

-BLOCK_NETMSG_FUNC(CLC_Screenshot_ReadFromBuffer, "48 89 5C 24 ? 48 89 6C 24 ? 48 89 74 24 ? 57 48 83 EC 20 48 8B DA 48 8B 52 38");

+AUTOHOOK(CLC_Screenshot_WriteToBuffer, engine.dll + 0x22AF20, 

+bool, __fastcall, (void* thisptr, void* buffer)) // 48 89 5C 24 ? 57 48 83 EC 20 8B 42 10

+{

+	return false;

+}

 

-// This is unused ingame and a big exploit vector

-BLOCK_NETMSG_FUNC(Base_CmdKeyValues_ReadFromBuffer, "40 55 48 81 EC ? ? ? ? 48 8D 6C 24 ? 48 89 5D 70");

+AUTOHOOK(CLC_Screenshot_ReadFromBuffer, engine.dll + 0x221F00, 

+bool, __fastcall, (void* thisptr, void* buffer)) // 48 89 5C 24 ? 48 89 6C 24 ? 48 89 74 24 ? 57 48 83 EC 20 48 8B DA 48 8B 52 38

+{

+	return false;

+}

+

+// This is unused ingame and a big client=>client exploit vector

+AUTOHOOK(Base_CmdKeyValues_ReadFromBuffer, engine.dll + 0x220040,

+bool, __fastcall, (void* thisptr, void* buffer)) // 40 55 48 81 EC ? ? ? ? 48 8D 6C 24 ? 48 89 5D 70

+{

+	return false;

+}

 

-KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48 FF 60 10"), bool, __fastcall, (void* pMsg))

+AUTOHOOK(CClient_ProcessSetConVar, engine.dll + 0x75CF0,

+bool, __fastcall, (void* pMsg)) // 48 8B D1 48 8B 49 18 48 8B 01 48 FF 60 10

 {

 

 	constexpr int ENTRY_STR_LEN = 260;

@@ -92,25 +102,11 @@ KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48
 	};

 

 	auto msg = (NET_SetConVar*)pMsg;

+	bool bIsServerFrame = Tier0::ThreadInServerFrameThread();

 

-	bool areWeServer;

+	std::string BLOCK_PREFIX = std::string {"NET_SetConVar ("} + (bIsServerFrame ? "server" : "client") + "): Blocked dangerous/invalid msg: ";

 

-	{

-		// Figure out of we are the client or the server

-		//	To do this, we utilize the msg's m_pMessageHandler pointer

-		//	m_pMessageHandler points to a virtual class that handles all net messages

-		//	The first virtual table function of our m_pMessageHandler will differ if it is IServerMessageHandler or IClientMessageHandler

-		void* msgHandlerVTableFirstFunc = **(void****)(msg->m_pMessageHandler);

-		static auto engineBaseAddress = (uintptr_t)GetModuleHandleA("engine.dll");

-		auto offset = uintptr_t(msgHandlerVTableFirstFunc) - engineBaseAddress;

-

-		constexpr uintptr_t CLIENTSTATE_FIRST_VFUNC_OFFSET = 0x8A15C;

-		areWeServer = offset != CLIENTSTATE_FIRST_VFUNC_OFFSET;

-	}

-

-	std::string BLOCK_PREFIX = std::string {"NET_SetConVar ("} + (areWeServer ? "server" : "client") + "): Blocked dangerous/invalid msg: ";

-

-	if (areWeServer)

+	if (bIsServerFrame)

 	{

 		constexpr int SETCONVAR_SANITY_AMOUNT_LIMIT = 69;

 		if (msg->m_ConVars_count < 1 || msg->m_ConVars_count > SETCONVAR_SANITY_AMOUNT_LIMIT)

@@ -149,7 +145,7 @@ KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48
 					strlen(realVar->m_ConCommandBase.m_pszName) + 1); // Force name to match case

 

 			bool isValidFlags = true;

-			if (areWeServer)

+			if (bIsServerFrame)

 			{

 				if (realVar)

 					isValidFlags = realVar->IsFlagSet(FCVAR_USERINFO); // ConVar MUST be userinfo var

@@ -178,11 +174,12 @@ KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48
 		}

 	}

 

-	return oCClient_ProcessSetConVar(msg);

+	return CClient_ProcessSetConVar(msg);

 }

 

-// Purpose: prevent invalid user CMDs

-KHOOK(CClient_ProcessUsercmds, ("engine.dll", "40 55 56 48 83 EC 58"), bool, __fastcall, (void* thisptr, void* pMsg))

+// prevent invalid user CMDs

+AUTOHOOK(CClient_ProcessUsercmds, engine.dll + 0x1040F0,

+bool, __fastcall, (void* thisptr, void* pMsg)) // 40 55 56 48 83 EC 58

 {

 	struct CLC_Move

 	{

@@ -208,7 +205,7 @@ KHOOK(CClient_ProcessUsercmds, ("engine.dll", "40 55 56 48 83 EC 58"), bool, __f
 	{

 		return BLOCKED_INFO("Invalid m_nNewCommands (" << msg->m_nNewCommands << ")");

 	}

-	

+

 	// removing, as vanilla already limits num usercmds per frame

 	/*constexpr int NUMCMD_SANITY_LIMIT = 16;

 	if ((msg->m_nNewCommands + msg->m_nBackupCommands) > NUMCMD_SANITY_LIMIT)

@@ -220,13 +217,14 @@ KHOOK(CClient_ProcessUsercmds, ("engine.dll", "40 55 56 48 83 EC 58"), bool, __f
 	if (msg->m_nLength <= 0)

 		return BLOCKED_INFO("Invalid message length (" << msg->m_nLength << ")");

 

-	return oCClient_ProcessUsercmds(thisptr, pMsg);

+	return CClient_ProcessUsercmds(thisptr, pMsg);

 }

 

-KHOOK(ReadUsercmd, ("server.dll", "4C 89 44 24 ? 53 55 56 57"), void, __fastcall, (void* buf, void* pCmd_move, void* pCmd_from))

+AUTOHOOK(ReadUsercmd, server.dll + 0x2603F0,

+void, __fastcall, (void* buf, void* pCmd_move, void* pCmd_from)) // 4C 89 44 24 ? 53 55 56 57

 {

 	// Let normal usercmd read happen first, it's safe

-	oReadUsercmd(buf, pCmd_move, pCmd_from);

+	ReadUsercmd(buf, pCmd_move, pCmd_from);

 

 	// Now let's make sure the CMD we read isnt messed up to prevent numerous exploits (including server crashing)

 	struct __declspec(align(4)) SV_CUserCmd

@@ -307,6 +305,7 @@ KHOOK(ReadUsercmd, ("server.dll", "4C 89 44 24 ? 53 55 56 57"), void, __fastcall
 	}

 

 	return;

+

 INVALID_CMD:

 

 	// Fix any gameplay-affecting cmd properties

@@ -318,26 +317,22 @@ INVALID_CMD:
 	cmd->meleetarget = 0;

 }

 

-// basically: by default r2 isn't set as a valve mod, meaning that m_bRestrictServerCommands is false

-// this is HORRIBLE for security, because it means servers can run arbitrary concommands on clients

-// especially since we have script commands this could theoretically be awful

-KHOOK(IsValveMod, ("engine.dll", "48 83 EC 28 48 8B 0D ? ? ? ? 48 8D 15 ? ? ? ? E8 ? ? ? ? 85 C0 74 63"), bool, __fastcall, ())

+// ensure that GetLocalBaseClient().m_bRestrictServerCommands is set correctly, which the return value of this function controls

+// this is IsValveMod in source, but we're making it IsRespawnMod now since valve didn't make this one

+AUTOHOOK(IsRespawnMod, engine.dll + 0x1C6360,

+bool, __fastcall, (const char* pModName)) // 48 83 EC 28 48 8B 0D ? ? ? ? 48 8D 15 ? ? ? ? E8 ? ? ? ? 85 C0 74 63

 {

-	return !Tier0::CommandLine()->CheckParm("-norestrictservercommands");

+	return (!strcmp("r2", pModName) || !strcmp("r1", pModName)) 

+		&& !Tier0::CommandLine()->CheckParm("-norestrictservercommands");

 }

 

-// Fix respawn's crappy UTF8 parser so it doesn't crash -_-

-// This also means you can launch multiplayer with "communities_enabled 1" and not crash, you're welcome

-KHOOK(

-	CrashFunc_ParseUTF8,

-	("engine.dll", "48 89 5C 24 ? 48 89 6C 24 ? 48 89 74 24 ? 57 41 54 41 55 41 56 41 57 48 83 EC 20 8B 1A"),

-	bool,

-	__fastcall,

-	(INT64 * a1, DWORD* a2, char* strData))

+// prevent utf8 parser from crashing when provided bad data, which can be sent through user-controlled openinvites 

+AUTOHOOK(Rson_ParseUTF8, engine.dll + 0xEF670, 

+bool, __fastcall, (INT64 * a1, DWORD* a2, char* strData)) // 48 89 5C 24 ? 48 89 6C 24 ? 48 89 74 24 ? 57 41 54 41 55 41 56 41 57 48 83 EC 20 8B 1A

 {

-

 	static void* targetRetAddr = NSMem::PatternScan("engine.dll", "84 C0 75 2C 49 8B 16");

 

+	// only call if we're parsing utf8 data from the network (i.e. communities), otherwise we get perf issues

 	if (_ReturnAddress() == targetRetAddr)

 	{

 		if (!ExploitFixes_UTF8Parser::CheckValid(a1, a2, strData))

@@ -348,68 +343,45 @@ KHOOK(
 		}

 	}

 

-	return oCrashFunc_ParseUTF8(a1, a2, strData);

+	return Rson_ParseUTF8(a1, a2, strData);

 }

 

-//////////////////////////////////////////////////

-

-void DoBytePatches()

+ON_DLL_LOAD("engine.dll", EngineExploitFixes, (HMODULE baseAddress))

 {

-	uintptr_t engineBase = (uintptr_t)GetModuleHandleA("engine.dll");

-	uintptr_t serverBase = (uintptr_t)GetModuleHandleA("server.dll");

-

-	// patches to make commands run from client/ui script still work

-	// note: this is likely preventable in a nicer way? test prolly

-	NSMem::BytePatch(engineBase + 0x4FB65, "EB 11");

-	NSMem::BytePatch(engineBase + 0x4FBAC, "EB 16");

-

-	{ // Dumb ANTITAMPER patches (they negatively impact performance and security)

+	AUTOHOOK_DISPATCH_MODULE(engine.dll)

 

-		constexpr const char* ANTITAMPER_EXPORTS[] = {

-			"ANTITAMPER_SPOTCHECK_CODEMARKER",

-			"ANTITAMPER_TESTVALUE_CODEMARKER",

-			"ANTITAMPER_TRIGGER_CODEMARKER",

-		};

-

-		// Prevent thesefrom actually doing anything

-		for (auto exportName : ANTITAMPER_EXPORTS)

-		{

-

-			auto address = (uintptr_t)GetProcAddress(GetModuleHandleA("server.dll"), exportName);

-			if (!address)

-			{

-				spdlog::warn("Failed to find AntiTamper function export \"{}\"", exportName);

-			}

-			else

-			{

-				// Just return, none of them have any args or are userpurge

-				NSMem::BytePatch(address, "C3");

-				spdlog::info("Patched AntiTamper function export \"{}\"", exportName);

-			}

-		}

-	}

+	// allow client/ui to run clientcommands despite restricting servercommands

+	NSMem::BytePatch((uintptr_t)baseAddress + 0x4FB65, "EB 11");

+	NSMem::BytePatch((uintptr_t)baseAddress + 0x4FBAC, "EB 16");

 }

 

-ON_DLL_LOAD_RELIESON("server.dll", ExploitFixes, ConVar, [](HMODULE baseAddress)

+ON_DLL_LOAD_RELIESON("server.dll", ServerExploitFixes, ConVar, (HMODULE baseAddress))

 {

-	spdlog::info("ExploitFixes::LoadCallback ...");

+	AUTOHOOK_DISPATCH_MODULE(server.dll)

 

-	spdlog::info("\tByte patching...");

-	DoBytePatches();

+	// Dumb ANTITAMPER patches (they negatively impact performance and security)

+	constexpr const char* ANTITAMPER_EXPORTS[] = {

+		"ANTITAMPER_SPOTCHECK_CODEMARKER",

+		"ANTITAMPER_TESTVALUE_CODEMARKER",

+		"ANTITAMPER_TRIGGER_CODEMARKER",

+	};

 

-	if (KHook::InitAllHooks())

-	{

-		spdlog::info("\tInitialized " + std::to_string(KHook::_allHooks.size()) + " exploit-patch hooks.");

-	}

-	else

+	// Prevent these from actually doing anything

+	for (auto exportName : ANTITAMPER_EXPORTS)

 	{

-		spdlog::critical("\tFAILED to initialize all exploit patches.");

-

-		// Force exit?

-		MessageBoxA(0, "FAILED to initialize all exploit patches.", "Northstar", MB_ICONERROR);

-		exit(0);

+		uintptr_t address = (uintptr_t)GetProcAddress(baseAddress, exportName);

+		if (!address)

+		{

+			spdlog::warn("Failed to find AntiTamper function export \"{}\"", exportName);

+		}

+		else

+		{

+			// Just return, none of them have any args or are userpurge

+			NSMem::BytePatch(address, "C3");

+			spdlog::info("Patched AntiTamper function export \"{}\"", exportName);

+		}

 	}

 

 	ns_exploitfixes_log =

 		new ConVar("ns_exploitfixes_log", "1", FCVAR_GAMEDLL, "Whether to log whenever ExploitFixes.cpp blocks/corrects something");

-})
\ No newline at end of file
+}
\ No newline at end of file