feat: Security improvements for spoofed packets (#1201)

* Add cheat detection for spoofed packets

* Add config option for ip loggin

* remove packet saving

dGame/UserManager.cpp

@@ -29,6 +29,7 @@
 #include "eConnectionType.h"
 #include "eChatInternalMessageType.h"
 #include "BitStreamUtils.h"
+#include "CheatDetection.h"
 
 UserManager* UserManager::m_Address = nullptr;
 
@@ -391,15 +392,14 @@ void UserManager::DeleteCharacter(const SystemAddress& sysAddr, Packet* packet)
 
 	Game::logger->Log("UserManager", "Received char delete req for ID: %llu (%u)", objectID, charID);
 
-	//Check if this user has this character:
+	bool hasCharacter = CheatDetection::VerifyLwoobjidIsSender(
-	bool hasCharacter = false;
+		objectID,
-	std::vector<Character*>& characters = u->GetCharacters();
+		sysAddr,
-	for (size_t i = 0; i < characters.size(); ++i) {
+		CheckType::User,
-		if (characters[i]->GetID() == charID) { hasCharacter = true; }
+		"User %i tried to delete a character that it does not own!",
-	}
+		u->GetAccountID());
 
 	if (!hasCharacter) {
-		Game::logger->Log("UserManager", "User %i tried to delete a character that it does not own!", u->GetAccountID());
 		WorldPackets::SendCharacterDeleteResponse(sysAddr, false);
 	} else {
 		Game::logger->Log("UserManager", "Deleting character %i", charID);
@@ -494,16 +494,24 @@ void UserManager::RenameCharacter(const SystemAddress& sysAddr, Packet* packet)
 	Character* character = nullptr;
 
 	//Check if this user has this character:
-	bool hasCharacter = false;
+	bool ownsCharacter = CheatDetection::VerifyLwoobjidIsSender(
-	std::vector<Character*>& characters = u->GetCharacters();
+		objectID,
-	for (size_t i = 0; i < characters.size(); ++i) {
+		sysAddr,
-		if (characters[i]->GetID() == charID) { hasCharacter = true; character = characters[i]; }
+		CheckType::User,
-	}
+		"User %i tried to rename a character that it does not own!",
+		u->GetAccountID());
 
-	if (!hasCharacter || !character) {
+	std::find_if(u->GetCharacters().begin(), u->GetCharacters().end(), [&](Character* c) {
-		Game::logger->Log("UserManager", "User %i tried to rename a character that it does not own!", u->GetAccountID());
+		if (c->GetID() == charID) {
+			character = c;
+			return true;
+		}
+		return false;
+		});
+
+	if (!ownsCharacter || !character) {
 		WorldPackets::SendCharacterRenameResponse(sysAddr, eRenameResponse::UNKNOWN_ERROR);
-	} else if (hasCharacter && character) {
+	} else if (ownsCharacter && character) {
 		if (newName == character->GetName()) {
 			WorldPackets::SendCharacterRenameResponse(sysAddr, eRenameResponse::NAME_UNAVAILABLE);
 			return;

dGame/dGameMessages/GameMessages.cpp

@@ -4909,13 +4909,6 @@ void GameMessages::HandleParseChatMessage(RakNet::BitStream* inStream, Entity* e
 		inStream->Read(character);
 		wsString.push_back(character);
 	}
-	
-	auto player = Player::GetPlayer(sysAddr);
-	if (!player || !player->GetCharacter()) return;
-	if (player->GetObjectID() != entity->GetObjectID()) {
-		Game::logger->Log("GameMessages", "Player %s is trying to send a chat message from an entity %llu they do not own!", player->GetCharacter()->GetName().c_str(), entity->GetObjectID());
-		return;
-	}
 
 	if (wsString[0] == L'/') {
 		SlashCommandHandler::HandleChatCommand(wsString, entity, sysAddr);

dGame/dUtilities/CMakeLists.txt

@@ -1,4 +1,5 @@
 set(DGAME_DUTILITIES_SOURCES "BrickDatabase.cpp"
+	"CheatDetection.cpp"
 	"GUID.cpp"
 	"Loot.cpp"
 	"Mail.cpp"

dGame/dUtilities/CheatDetection.cpp

@@ -0,0 +1,137 @@
+#include "CheatDetection.h"
+#include "Database.h"
+#include "Entity.h"
+#include "PossessableComponent.h"
+#include "Player.h"
+#include "Game.h"
+#include "EntityManager.h"
+#include "Character.h"
+#include "User.h"
+#include "UserManager.h"
+#include "dConfig.h"
+
+Entity* GetPossessedEntity(const LWOOBJID& objId) {
+	auto* entity = Game::entityManager->GetEntity(objId);
+	if (!entity) return nullptr;
+
+	auto* possessableComponent = entity->GetComponent<PossessableComponent>();
+	// If no possessable, then this entity is the most possessed entity.
+	if (!possessableComponent) return entity;
+
+	// If not, get the entity that possesses the fetched entity.
+	return Game::entityManager->GetEntity(possessableComponent->GetPossessor());
+}
+
+void ReportCheat(User* user, const SystemAddress& sysAddr, const char* messageIfNotSender, va_list args) {
+	if (!user) {
+		Game::logger->Log("CheatDetection", "WARNING: User is null, using defaults.");
+	}
+	std::unique_ptr<sql::PreparedStatement> stmt(Database::CreatePreppedStmt(
+		"INSERT INTO player_cheat_detections (account_id, name, violation_msg, violation_system_address) VALUES (?, ?, ?, ?)")
+	);
+	stmt->setInt(1, user ? user->GetAccountID() : 0);
+	stmt->setString(2, user ? user->GetUsername().c_str() : "User is null.");
+
+	constexpr int32_t bufSize = 4096;
+	char buffer[bufSize];
+	vsnprintf(buffer, bufSize, messageIfNotSender, args);
+
+	stmt->setString(3, buffer);
+	stmt->setString(4, Game::config->GetValue("log_ip_addresses_for_anti_cheat") == "1" ? sysAddr.ToString() : "IP logging disabled.");
+	stmt->execute();
+	Game::logger->Log("CheatDetection", "Anti-cheat message: %s", buffer);
+}
+
+void LogAndSaveFailedAntiCheatCheck(const LWOOBJID& id, const SystemAddress& sysAddr, const CheckType checkType, const char* messageIfNotSender, va_list args) {
+	User* toReport = nullptr;
+	switch (checkType) {
+	case CheckType::Entity: {
+		auto* player = Player::GetPlayer(sysAddr);
+		auto* entity = GetPossessedEntity(id);
+		
+		// If player exists and entity exists in world, use both for logging info.
+		if (entity && player) {
+			Game::logger->Log("CheatDetection", "Player (%s) (%llu) at system address (%s) with sending player (%s) (%llu) does not match their own.",
+				player->GetCharacter()->GetName().c_str(), player->GetObjectID(),
+				sysAddr.ToString(),
+				entity->GetCharacter()->GetName().c_str(), entity->GetObjectID());
+		// In the case that the target entity id did not exist, just log the player info.
+		} else if (player) {
+			Game::logger->Log("CheatDetection", "Player (%s) (%llu) at system address (%s) with sending player (%llu) does not match their own.",
+				player->GetCharacter()->GetName().c_str(), player->GetObjectID(),
+				sysAddr.ToString(), id);
+		// In the rare case that the player does not exist, just log the system address and who the target id was.
+		} else {
+			Game::logger->Log("CheatDetection", "Player at system address (%s) with sending player (%llu) does not match their own.",
+				sysAddr.ToString(), id);
+		}
+		toReport = player->GetParentUser();
+		break;
+	}
+	case CheckType::User: {
+		auto* user = UserManager::Instance()->GetUser(sysAddr);
+		
+		if (user) {
+			Game::logger->Log("CheatDetection", "User at system address (%s) (%s) (%llu) sent a packet as (%i) which is not an id they own.",
+				sysAddr.ToString(), user->GetLastUsedChar()->GetName().c_str(), user->GetLastUsedChar()->GetObjectID(), static_cast<int32_t>(id));
+		// Can't know sending player. Just log system address for IP banning.
+		} else {
+			Game::logger->Log("CheatDetection", "No user found for system address (%s).", sysAddr.ToString());
+		}
+		toReport = user;
+		break;
+	}
+	};
+	ReportCheat(toReport, sysAddr, messageIfNotSender, args);
+}
+
+void CheatDetection::ReportCheat(User* user, const SystemAddress& sysAddr, const char* messageIfNotSender, ...) {
+	va_list args;
+	va_start(args, messageIfNotSender);
+	ReportCheat(user, sysAddr, messageIfNotSender, args);
+	va_end(args);
+}
+
+bool CheatDetection::VerifyLwoobjidIsSender(const LWOOBJID& id, const SystemAddress& sysAddr, const CheckType checkType, const char* messageIfNotSender, ...) {
+	// Get variables we'll need for the whole function
+	bool invalidPacket = false;
+	switch (checkType) {
+	case CheckType::Entity: {
+		// In this case, the sender may be an entity in the world.
+		auto* entity = GetPossessedEntity(id);
+		if (entity) {
+			invalidPacket = entity->IsPlayer() && entity->GetSystemAddress() != sysAddr;
+		}
+		break;
+	}
+	case CheckType::User: {
+		// In this case, the player is not an entity in the world, but may be a user still in world server if they are connected.
+		// Check here if the system address has a character with id matching the lwoobjid after unsetting the flag bits.
+		auto* sendingUser = UserManager::Instance()->GetUser(sysAddr);
+		if (!sendingUser) {
+			Game::logger->Log("CheatDetection", "No user found for system address (%s).", sysAddr.ToString());
+			return false;
+		}
+		invalidPacket = true;
+		const uint32_t characterId = static_cast<uint32_t>(id);
+		// Check to make sure the ID provided is one of the user's characters.
+		for (const auto& character : sendingUser->GetCharacters()) {
+			if (character && character->GetID() == characterId) {
+				invalidPacket = false;
+				break;
+			}
+		}
+	}
+	};
+
+	// This will be true if the player does not possess the entity they are trying to send a packet as.
+	// or if the user does not own the character they are trying to send a packet as.
+	if (invalidPacket) {
+		va_list args;
+		va_start(args, messageIfNotSender);
+		LogAndSaveFailedAntiCheatCheck(id, sysAddr, checkType, messageIfNotSender, args);
+		va_end(args);
+	}
+
+	return !invalidPacket;
+}

dGame/dUtilities/CheatDetection.h

@@ -0,0 +1,30 @@
+#ifndef __CHEATDETECTION__H__
+#define __CHEATDETECTION__H__
+
+#include "dCommonVars.h"
+
+struct SystemAddress;
+
+enum class CheckType : uint8_t {
+	User,
+	Entity,
+};
+
+namespace CheatDetection {
+	/**
+	 * @brief Verify that the object ID provided in this function is in someway connected to the system address who sent it.
+	 * 
+	 * @param id The object ID to check ownership of
+	 * @param sysAddr The system address which sent the packet
+	 * @param checkType The check type to perform
+	 * @param messageIfNotSender The message to log if the sender is not the owner of the object ID
+	 * @param ... format args
+	 * @return true If the sender is the owner of the object ID
+	 * @return false If the sender is not the owner of the object ID
+	 */
+	bool VerifyLwoobjidIsSender(const LWOOBJID& id, const SystemAddress& sysAddr, const CheckType checkType, const char* messageIfNotSender, ...);
+	void ReportCheat(User* user, const SystemAddress& sysAddr, const char* messageIfNotSender, ...);
+};
+
+#endif  //!__CHEATDETECTION__H__
+

dNet/ClientPackets.cpp

@@ -33,6 +33,7 @@
 #include "Database.h"
 #include "eGameMasterLevel.h"
 #include "eReplicaComponentType.h"
+#include "CheatDetection.h"
 
 void ClientPackets::HandleChatMessage(const SystemAddress& sysAddr, Packet* packet) {
 	User* user = UserManager::Instance()->GetUser(sysAddr);
@@ -65,8 +66,18 @@ void ClientPackets::HandleChatMessage(const SystemAddress& sysAddr, Packet* pack
 
 	std::string playerName = user->GetLastUsedChar()->GetName();
 	bool isMythran = user->GetLastUsedChar()->GetGMLevel() > eGameMasterLevel::CIVILIAN;
-
+	bool isOk = Game::chatFilter->IsSentenceOkay(GeneralUtils::UTF16ToWTF8(message), user->GetLastUsedChar()->GetGMLevel()).empty();
-	if (!user->GetLastChatMessageApproved() && !isMythran) return;
+	Game::logger->LogDebug("ClientPackets", "Msg: %s was approved previously? %i", GeneralUtils::UTF16ToWTF8(message).c_str(), user->GetLastChatMessageApproved());
+	if (!isOk) {
+		// Add a limit to the string converted by general utils because it is a user received string and may be a bad actor.
+		CheatDetection::ReportCheat(
+			user,
+			sysAddr,
+			"Player %s attempted to bypass chat filter with message: %s",
+			playerName.c_str(),
+			GeneralUtils::UTF16ToWTF8(message, 512).c_str());
+	}
+	if (!isOk && !isMythran) return;
 
 	std::string sMessage = GeneralUtils::UTF16ToWTF8(message);
 	Game::logger->Log("Chat", "%s: %s", playerName.c_str(), sMessage.c_str());

dWorldServer/WorldServer.cpp

@@ -73,6 +73,7 @@
 #include "eGameMessageType.h"
 #include "ZCompression.h"
 #include "EntityManager.h"
+#include "CheatDetection.h"
 
 namespace Game {
 	dLogger* logger = nullptr;
@@ -957,7 +958,15 @@ void HandlePacket(Packet* packet) {
 		RakNet::BitStream dataStream;
 		bitStream.Read(dataStream, bitStream.GetNumberOfUnreadBits());
 
-		GameMessageHandler::HandleMessage(&dataStream, packet->systemAddress, objectID, messageID);
+		auto isSender = CheatDetection::VerifyLwoobjidIsSender(
+			objectID,
+			packet->systemAddress,
+			CheckType::Entity,
+			"Sending GM with a sending player that does not match their own. GM ID: %i",
+			static_cast<int32_t>(messageID)
+			);
+
+		if (isSender) GameMessageHandler::HandleMessage(&dataStream, packet->systemAddress, objectID, messageID);
 		break;
 	}
 
@@ -972,6 +981,17 @@ void HandlePacket(Packet* packet) {
 
 		LWOOBJID playerID = 0;
 		inStream.Read(playerID);
+
+		bool valid = CheatDetection::VerifyLwoobjidIsSender(
+			playerID,
+			packet->systemAddress,
+			CheckType::User,
+			"Sending login request with a sending player that does not match their own. Player ID: %llu",
+			playerID
+			);
+
+		if (!valid) return;
+
 		GeneralUtils::ClearBit(playerID, eObjectBits::CHARACTER);
 		GeneralUtils::ClearBit(playerID, eObjectBits::PERSISTENT);
 
@@ -1204,6 +1224,7 @@ void HandlePacket(Packet* packet) {
 
 	case eWorldMessageType::MAIL: {
 		RakNet::BitStream bitStream(packet->data, packet->length, false);
+		// FIXME: Change this to the macro to skip the header...
 		LWOOBJID space;
 		bitStream.Read(space);
 		Mail::HandleMailStuff(&bitStream, packet->systemAddress, UserManager::Instance()->GetUser(packet->systemAddress)->GetLastUsedChar()->GetEntity());

migrations/dlu/10_Security_updates.sql

@@ -0,0 +1,8 @@
+CREATE TABLE IF NOT EXISTS player_cheat_detections (
+    id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY,
+    account_id INT REFERENCES accounts(id),
+    name TEXT REFERENCES charinfo(name),
+    violation_msg TEXT NOT NULL,
+    violation_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP(),
+    violation_system_address TEXT NOT NULL
+);

resources/worldconfig.ini

@@ -61,3 +61,6 @@ hardcore_lose_uscore_on_death_percent=10
 
 # Allow civilian players the ability to turn the nameplate above their head off.  Must be exactly 1 to be enabled for civilians.
 allow_nameplate_off=0
+
+# Turn logging of IP addresses for anti-cheat reporting on (1) or off(0)
+log_ip_addresses_for_anti_cheat=1