More exploit fixes by KIttenPopo (#126)

* Quick fix for a bug I caused

* Typo

* Update kitten-fixes branch to my repo (#122)

* Added string hash macro

* Added convenient vtfunc macro

* Made lil ConCommand creation macro

* Fixed multiple NET_SetConVar exploits

* Quick fixerino

* Fix convar struct (and other things)

* Revive clang-format (but good, i think)

* Update .clang-format

* Reformatted code to meet .clang-format requirements

* Minor formatting fixes

* Fixed Northstar "crashing" when console is closed

* Update .clang-format

* Quick fix for a bug I caused

* Typo

* NSMem Update

* ExplotFixes: Only block excessive convar counts if server

* Update ExploitFixes.cpp

* Update ExploitFixes.cpp

* Updated bytepatch format

* reformatted all code for clang-format

* Updated my clang-format to v13.0.0

* 3 fixes in 1

- ANTITAMPER fixed
- NSMem simplification update
- Fixed bad byte string in serverauthentication.cpp

* Improved ExploitFixes logging and NET_SetConVar patch

* clang-format unironically sabotaged my code

* Made ns_exploitfixes_log on by default

* Fixed IsMemoryReadable (oops)

Co-authored-by: KittenPopo <Pokeberry123@gmail.com>

1 files changed, 127 insertions, 28 deletions

diff --git a/NorthstarDedicatedTest/ExploitFixes.cpp b/NorthstarDedicatedTest/ExploitFixes.cpp
index 36bd36f4..4c91ef75 100644
--- a/NorthstarDedicatedTest/ExploitFixes.cpp
+++ b/NorthstarDedicatedTest/ExploitFixes.cpp
@@ -5,6 +5,19 @@
 #include "NSMem.h"
 #include "cvar.h"
 
+ConVar* ns_exploitfixes_log;
+#define SHOULD_LOG (ns_exploitfixes_log->m_Value.m_nValue > 0)
+#define BLOCKED_INFO(s) \
+	([=]() -> bool {                   \
+		if (SHOULD_LOG) \
+		{ \
+			std::stringstream stream; \
+			stream << "ExploitFixes.cpp: " << BLOCK_PREFIX << s; \
+			spdlog::error(stream.str()); \
+		} \
+		return false; \
+	}())
+
 // Make sure 3 or less floats are valid
 bool ValidateFloats(float a, float b = 0, float c = 0) { return !isnan(a) && !isnan(b) && !isnan(c); }
 
@@ -66,9 +79,31 @@ KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48
 
 	auto msg = (NET_SetConVar*)pMsg;
 
-	constexpr int SETCONVAR_SANITY_AMOUNT_LIMIT = 20;
-	if (msg->m_ConVars_count < 1 || msg->m_ConVars_count > SETCONVAR_SANITY_AMOUNT_LIMIT)
-		return false; // Nope
+	bool areWeServer;
+
+	{ 
+		// 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)
+	{
+		constexpr int SETCONVAR_SANITY_AMOUNT_LIMIT = 69;
+		if (msg->m_ConVars_count < 1 || msg->m_ConVars_count > SETCONVAR_SANITY_AMOUNT_LIMIT)
+		{
+			return BLOCKED_INFO("Invalid m_ConVars_count (" << msg->m_ConVars_count << ")");
+		}
+	}
 
 	for (int i = 0; i < msg->m_ConVars_count; i++)
 	{
@@ -89,25 +124,41 @@ KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48
 			}
 
 			if (!nameValid || !valValid)
-				return false; // Missing null terminators
+				return BLOCKED_INFO("Missing null terminators");
 
 			auto realVar = g_pCVar->FindVar(entry->name);
 
-			if (!realVar)
-				// Not an actual cvar, no thanks
-				return false;
+			if (realVar)
+				memcpy(entry->name, realVar->m_ConCommandBase.m_pszName, strlen(realVar->m_ConCommandBase.m_pszName) + 1); // Force name to match case
 
-			// Force name to match case
-			memcpy(entry->name, realVar->m_ConCommandBase.m_pszName, strlen(realVar->m_ConCommandBase.m_pszName) + 1);
+			bool isValidFlags = true;
+			if (areWeServer)
+			{
+				if (realVar)
+					isValidFlags = ConVar::IsFlagSet(realVar, FCVAR_USERINFO); // ConVar MUST be userinfo var
+			}
+			else
+			{
+				// TODO: Should probably have some sanity checks, but can't find any that are consistent
+			}
 
-			if (!ConVar::IsFlagSet(realVar, FCVAR_USERINFO) || ConVar::IsFlagSet(realVar, FCVAR_CHEAT))
+			if (!isValidFlags)
 			{
-				return false;
+				if (!realVar)
+				{
+					return BLOCKED_INFO("Invalid flags on nonexistant cvar (how tho???)");
+				}
+				else
+				{
+					return BLOCKED_INFO(
+						"Invalid flags (" << std::hex << "0x" << realVar->m_ConCommandBase.m_nFlags << "), var is " << entry->name);
+				}
+				
 			}
 		}
 		else
 		{
-			return false; // Not risking that one, they all gotta be readable
+			return BLOCKED_INFO("Unreadable memory at " << (void*)entry); // Not risking that one, they all gotta be readable
 		}
 	}
 
@@ -130,15 +181,27 @@ KHOOK(CClient_ProcessUsercmds, ("engine.dll", "40 55 56 48 83 EC 58"), bool, __f
 
 	auto msg = (CLC_Move*)pMsg;
 
-	if (msg->m_nBackupCommands < 0 || msg->m_nNewCommands < 1)
-		return false; // Nice try buster
+	const char* BLOCK_PREFIX = "ProcessUserCmds: ";
+
+	if (msg->m_nBackupCommands < 0)
+	{
+		return BLOCKED_INFO("Invalid m_nBackupCommands (" << msg->m_nBackupCommands << ")");
+	}
+
+	if (msg->m_nNewCommands < 0)
+	{
+		return BLOCKED_INFO("Invalid m_nNewCommands (" << msg->m_nNewCommands << ")");
+	}
 
 	constexpr int NUMCMD_SANITY_LIMIT = 16;
 	if ((msg->m_nNewCommands + msg->m_nBackupCommands) > NUMCMD_SANITY_LIMIT)
-		return false; // Im good
+	{
+		return BLOCKED_INFO("Command count is too high (new: " << msg->m_nNewCommands << ", backup: " << msg->m_nBackupCommands << ")");
+
+	}
 
 	if (msg->m_nLength <= 0)
-		return false;
+		return BLOCKED_INFO("Invalid message length (" << msg->m_nLength << ")");
 
 	return oCClient_ProcessUsercmds(thisptr, pMsg);
 }
@@ -175,23 +238,56 @@ KHOOK(ReadUsercmd, ("server.dll", "4C 89 44 24 ? 53 55 56 57"), void, __fastcall
 		DWORD dword98;
 		float frameTime;
 	};
+
 	auto cmd = (SV_CUserCmd*)pCmd_move;
+	auto fromCmd = (SV_CUserCmd*)pCmd_from;
+
+	std::string BLOCK_PREFIX = "ReadUsercmd (command_number delta: " + std::to_string(cmd->command_number - fromCmd->command_number) + "): ";
+
+	if (cmd->worldViewAngles.IsInvalid())
+	{
+		BLOCKED_INFO("CMD has invalid worldViewAngles");
+		goto INVALID_CMD;
+	}
 
-	if (cmd->worldViewAngles.IsInvalid() || cmd->localViewAngles.IsInvalid() || cmd->attackangles.IsInvalid() ||
-		cmd->cameraAngles.IsInvalid())
+	if (cmd->attackangles.IsInvalid())
 	{
+		BLOCKED_INFO("CMD has invalid attackangles");
+		goto INVALID_CMD;
+	}
+
+	if (cmd->localViewAngles.IsInvalid())
+	{
+		BLOCKED_INFO("CMD has invalid localViewAngles");
+		goto INVALID_CMD;
+	}
+
+	if (cmd->cameraAngles.IsInvalid())
+	{
+		BLOCKED_INFO("CMD has invalid cameraAngles");
 		goto INVALID_CMD;
 	}
 
 	if (cmd->frameTime <= 0 || cmd->tick_count == 0 || cmd->command_time <= 0)
+	{
+		BLOCKED_INFO(
+			"Bogus cmd timing (tick_count: " << cmd->tick_count << ", frameTime: " << cmd->frameTime
+											 << ", commandTime : " << cmd->command_time << ")");
 		goto INVALID_CMD; // No simulation of bogus-timed cmds
+	}
+		
 
-	if (!cmd->move.IsValid() ||	   // Prevent player freeze (and even server crash) exploit
-		!cmd->cameraPos.IsValid()) // IIRC this can crash spectating clients or anyone watching replays
+	if (!cmd->move.IsValid())
+	{
+		BLOCKED_INFO("Invalid move vector");
 		goto INVALID_CMD;
+	}
 
-	if (!ValidateFloats(cmd->cameraPos.x, cmd->cameraPos.y, cmd->cameraPos.z))
-		goto INVALID_CMD; // IIRC this can crash spectating clients or anyone watching replays
+	if (!cmd->cameraPos.IsValid())
+	{
+		BLOCKED_INFO("Invalid cameraPos"); // IIRC this can crash spectating clients or anyone watching replays
+		goto INVALID_CMD;
+	}
 
 	return;
 INVALID_CMD:
@@ -227,7 +323,8 @@ KHOOK(
 	{
 		if (!ExploitFixes_UTF8Parser::CheckValid(a1, a2, strData))
 		{
-			spdlog::warn("ParseUTF8 Hook: Ignoring potentially-crashing utf8 string");
+			const char* BLOCK_PREFIX = "ParseUTF8 Hook: ";
+			BLOCKED_INFO("Ignoring potentially-crashing utf8 string");
 			return false;
 		}
 	}
@@ -244,8 +341,8 @@ void DoBytePatches()
 
 	// 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, {0xEB, 0x11});
-	NSMem::BytePatch(engineBase + 0x4FBAC, {0xEB, 0x16});
+	NSMem::BytePatch(engineBase + 0x4FB65, "EB 11");
+	NSMem::BytePatch(engineBase + 0x4FBAC, "EB 16");
 
 	// disconnect concommand
 	{
@@ -266,16 +363,15 @@ void DoBytePatches()
 		for (auto exportName : ANTITAMPER_EXPORTS)
 		{
 
-			auto address = (uintptr_t)GetProcAddress(NULL, exportName);
+			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, {0xC3});
+				NSMem::BytePatch(address, "C3");
 
 				spdlog::info("Patched AntiTamper function export \"{}\"", exportName);
 			}
@@ -302,4 +398,7 @@ void ExploitFixes::LoadCallback(HMODULE unused)
 		MessageBoxA(0, "FAILED to initialize all exploit patches.", "Northstar", MB_ICONERROR);
 		exit(0);
 	}
+
+	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