Major exploit fixes and some minor bug fixes/improvements (#117)v1.6.0-rc3

* Added main exploit fixes

* Fixed typo in sigscanning.cpp

* Fully implemented

* Added proper includes for new files

* Update README.md

* typo

* spare me my sanity (fixed ridiculous code)

* Added rest of KittenMemUtils

* Rename KittenMemUtils

* Removed all messy memory edits, implemented NSMem instead

* Update NorthstarDedicatedTest.vcxproj

* [1] Move everything from securitypatches to ExploitFixes

* [2] Move everything from securitypatches to ExploitFixes

* Fixed module offsets in stack trace

* Fixed UTF8 Parsing (Multiplayer Crash)

* Implemented UT8 fix

* Update NorthstarDedicatedTest.vcxproj

* Update hookutils.cpp

* Small fixes

* all my homies hate clang-format

* Temporarily restore README.md

* 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

1 files changed, 143 insertions, 34 deletions

diff --git a/NorthstarDedicatedTest/ExploitFixes.cpp b/NorthstarDedicatedTest/ExploitFixes.cpp
index d36b4175..36bd36f4 100644
--- a/NorthstarDedicatedTest/ExploitFixes.cpp
+++ b/NorthstarDedicatedTest/ExploitFixes.cpp
@@ -3,39 +3,36 @@
 #include "ExploitFixes.h"
 #include "ExploitFixes_UTF8Parser.h"
 #include "NSMem.h"
+#include "cvar.h"
 
 // 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);
-}
+bool ValidateFloats(float a, float b = 0, float c = 0) { return !isnan(a) && !isnan(b) && !isnan(c); }
 
-struct Vector {
+struct Vector
+{
 	float x, y, z;
 
 	Vector(float x = 0, float y = 0, float z = 0) : x(x), y(y), z(z) {}
 
-	bool IsValid() {
-		return ValidateFloats(x, y, z);
-	}
+	bool IsValid() { return ValidateFloats(x, y, z); }
 };
 
-struct Angle {
+struct Angle
+{
 	float pitch, yaw, roll;
 
 	Angle(float pitch = 0, float yaw = 0, float roll = 0) : pitch(pitch), yaw(yaw), roll(roll) {}
 
-	bool IsInvalid() {
+	bool IsInvalid()
+	{
 		if (!ValidateFloats(pitch, yaw, roll))
 			return false;
 
-		return
-			(pitch > 90 || pitch < -90)
-			|| (yaw > 180 || yaw < -180)
-			|| (roll > 180 || roll < -180);
+		return (pitch > 90 || pitch < -90) || (yaw > 180 || yaw < -180) || (roll > 180 || roll < -180);
 	}
 };
 
-#define BLOCK_NETMSG_FUNC(name, pattern) \
+#define BLOCK_NETMSG_FUNC(name, pattern)                                                                                                   \
 	KHOOK(name, ("engine.dll", pattern), bool, __fastcall, (void* thisptr, void* buffer)) { return false; }
 
 // Servers can literally request a screenshot from any client, yeah no
@@ -45,9 +42,83 @@ BLOCK_NETMSG_FUNC(CLC_Screenshot_ReadFromBuffer, "48 89 5C 24 ? 48 89 6C 24 ? 48
 // 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");
 
+KHOOK(CClient_ProcessSetConVar, ("engine.dll", "48 8B D1 48 8B 49 18 48 8B 01 48 FF 60 10"), bool, __fastcall, (void* pMsg))
+{
+
+	constexpr int ENTRY_STR_LEN = 260;
+	struct SetConVarEntry
+	{
+		char name[ENTRY_STR_LEN];
+		char val[ENTRY_STR_LEN];
+	};
+
+	struct NET_SetConVar
+	{
+		void* vtable;
+		void* unk1;
+		void* unk2;
+		void* m_pMessageHandler;
+		SetConVarEntry* m_ConVars; // convar entry array
+		void* unk5;				   // these 2 unks are just vector capacity or whatever
+		void* unk6;
+		int m_ConVars_count; // amount of cvar entries in array (this will not be out of bounds)
+	};
+
+	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
+
+	for (int i = 0; i < msg->m_ConVars_count; i++)
+	{
+		auto entry = msg->m_ConVars + i;
+
+		// Safety check for memory access
+		if (NSMem::IsMemoryReadable(entry, sizeof(*entry)))
+		{
+
+			// Find null terminators
+			bool nameValid = false, valValid = false;
+			for (int i = 0; i < ENTRY_STR_LEN; i++)
+			{
+				if (!entry->name[i])
+					nameValid = true;
+				if (!entry->val[i])
+					valValid = true;
+			}
+
+			if (!nameValid || !valValid)
+				return false; // Missing null terminators
+
+			auto realVar = g_pCVar->FindVar(entry->name);
+
+			if (!realVar)
+				// Not an actual cvar, no thanks
+				return false;
+
+			// Force name to match case
+			memcpy(entry->name, realVar->m_ConCommandBase.m_pszName, strlen(realVar->m_ConCommandBase.m_pszName) + 1);
+
+			if (!ConVar::IsFlagSet(realVar, FCVAR_USERINFO) || ConVar::IsFlagSet(realVar, FCVAR_CHEAT))
+			{
+				return false;
+			}
+		}
+		else
+		{
+			return false; // Not risking that one, they all gotta be readable
+		}
+	}
+
+	return oCClient_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)) {
-	struct __declspec(align(8)) CLC_Move {
+KHOOK(CClient_ProcessUsercmds, ("engine.dll", "40 55 56 48 83 EC 58"), bool, __fastcall, (void* thisptr, void* pMsg))
+{
+	struct CLC_Move
+	{
 		BYTE gap0[24];
 		void* m_pMessageHandler;
 		int m_nBackupCommands;
@@ -72,7 +143,8 @@ KHOOK(CClient_ProcessUsercmds, ("engine.dll", "40 55 56 48 83 EC 58"), bool, __f
 	return oCClient_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)) {
+KHOOK(ReadUsercmd, ("server.dll", "4C 89 44 24 ? 53 55 56 57"), void, __fastcall, (void* buf, void* pCmd_move, void* pCmd_from))
+{
 	// Let normal usercmd read happen first, it's safe
 	oReadUsercmd(buf, pCmd_move, pCmd_from);
 
@@ -105,20 +177,18 @@ KHOOK(ReadUsercmd, ("server.dll", "4C 89 44 24 ? 53 55 56 57"), void, __fastcall
 	};
 	auto cmd = (SV_CUserCmd*)pCmd_move;
 
-	if (
-		cmd->worldViewAngles.IsInvalid() ||
-		cmd->localViewAngles.IsInvalid() ||
-		cmd->attackangles.IsInvalid() ||
-		cmd->cameraAngles.IsInvalid()) {
+	if (cmd->worldViewAngles.IsInvalid() || cmd->localViewAngles.IsInvalid() || cmd->attackangles.IsInvalid() ||
+		cmd->cameraAngles.IsInvalid())
+	{
 		goto INVALID_CMD;
 	}
 
 	if (cmd->frameTime <= 0 || cmd->tick_count == 0 || cmd->command_time <= 0)
 		goto INVALID_CMD; // No simulation of bogus-timed cmds
 
-	if (!cmd->move.IsValid() || // Prevent player freeze (and even server crash) exploit
+	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
-		goto INVALID_CMD; 
+		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
@@ -128,7 +198,7 @@ INVALID_CMD:
 
 	// Fix any gameplay-affecting cmd properties
 	// NOTE: Currently tickcount/frametime is set to 0, this ~shouldn't~ cause any problems
-	cmd->worldViewAngles = cmd->localViewAngles = cmd->attackangles = cmd->cameraAngles = Angle(0,0,0);
+	cmd->worldViewAngles = cmd->localViewAngles = cmd->attackangles = cmd->cameraAngles = Angle(0, 0, 0);
 	cmd->tick_count = cmd->frameTime = 0;
 	cmd->move = cmd->cameraPos = Vector(0, 0, 0);
 	cmd->buttons = 0;
@@ -139,19 +209,24 @@ INVALID_CMD:
 // 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
 #include "gameutils.h"
-KHOOK(IsValveMod, ("engine.dll", "48 83 EC 28 48 8B 0D ? ? ? ? 48 8D 15 ? ? ? ? E8 ? ? ? ? 85 C0 74 63"), bool, __fastcall, ()) {
+KHOOK(IsValveMod, ("engine.dll", "48 83 EC 28 48 8B 0D ? ? ? ? 48 8D 15 ? ? ? ? E8 ? ? ? ? 85 C0 74 63"), bool, __fastcall, ())
+{
 	return !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)) {
+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))
+{
 
 	static void* targetRetAddr = NSMem::PatternScan("engine.dll", "84 C0 75 2C 49 8B 16");
 
-	if (_ReturnAddress() == targetRetAddr) {
-		if (!ExploitFixes_UTF8Parser::CheckValid(a1, a2, strData)) {
+	if (_ReturnAddress() == targetRetAddr)
+	{
+		if (!ExploitFixes_UTF8Parser::CheckValid(a1, a2, strData))
+		{
 			spdlog::warn("ParseUTF8 Hook: Ignoring potentially-crashing utf8 string");
 			return false;
 		}
@@ -162,8 +237,10 @@ KHOOK(CrashFunc_ParseUTF8, ("engine.dll", "48 89 5C 24 ? 48 89 6C 24 ? 48 89 74
 
 //////////////////////////////////////////////////
 
-void DoBytePatches() {
+void DoBytePatches()
+{
 	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
@@ -176,17 +253,49 @@ void DoBytePatches() {
 		int val = *(int*)addr | FCVAR_SERVER_CAN_EXECUTE;
 		NSMem::BytePatch(addr, (BYTE*)&val, sizeof(int));
 	}
+
+	{ // Dumb ANTITAMPER patches (they negatively impact performance and security)
+
+		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(NULL, 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});
+
+				spdlog::info("Patched AntiTamper function export \"{}\"", exportName);
+			}
+		}
+	}
 }
 
-void ExploitFixes::LoadCallback(HMODULE unused) {
+void ExploitFixes::LoadCallback(HMODULE unused)
+{
 	spdlog::info("ExploitFixes::LoadCallback ...");
 
 	spdlog::info("\tByte patching...");
 	DoBytePatches();
 
-	if (KHook::InitAllHooks()) {
+	if (KHook::InitAllHooks())
+	{
 		spdlog::info("\tInitialized " + std::to_string(KHook::_allHooks.size()) + " exploit-patch hooks.");
-	} else {
+	}
+	else
+	{
 		spdlog::critical("\tFAILED to initialize all exploit patches.");
 
 		// Force exit?