fix: security vulnerabilities

Tested that all functions related to the touched files work

will test sqlite on a CI build

dGame/dUtilities/BrickDatabase.cpp

@@ -66,13 +66,9 @@ const BrickList& BrickDatabase::GetBricks(const LxfmlPath& lxfmlPath) {
 				std::string materialString(materialList);
 				const auto materials = GeneralUtils::SplitString(materialString, ',');
 
-				if (!materials.empty()) {
-					brick.materialID = std::stoi(materials[0]);
-				} else {
-					brick.materialID = 0;
-				}
+				brick.materialID = GeneralUtils::TryParse(materials[0], 0);
 			} else if (materialID != nullptr) {
-				brick.materialID = std::stoi(materialID);
+				brick.materialID = GeneralUtils::TryParse(materialID, 0);
 			} else {
 				brick.materialID = 0; // This is bad, makes it so the minigame can't be played
 			}

dGame/dUtilities/CheatDetection.cpp

@@ -54,17 +54,23 @@ void LogAndSaveFailedAntiCheatCheck(const LWOOBJID& id, const SystemAddress& sys
 		
 		// If player exists and entity exists in world, use both for logging info.
 		if (entity && player) {
+			const auto* const playerChar = player->GetCharacter();
+			const auto& playerName = playerChar ? playerChar->GetName() : "(null player character)";
+			const auto* const entityChar = entity->GetCharacter();
+			const auto& entityName = entityChar ? entityChar->GetName() : "(null entity character)";
 			LOG("Player (%s) (%llu) at system address (%s) with sending player (%s) (%llu) does not match their own.",
-				player->GetCharacter()->GetName().c_str(), player->GetObjectID(),
+				playerName.c_str(), player->GetObjectID(),
 				sysAddr.ToString(),
-				entity->GetCharacter()->GetName().c_str(), entity->GetObjectID());
-			if (player->GetCharacter()) toReport = player->GetCharacter()->GetParentUser();
+				entityName.c_str(), entity->GetObjectID());
+			if (playerChar) toReport = playerChar->GetParentUser();
 		// In the case that the target entity id did not exist, just log the player info.
 		} else if (player) {
+			const auto* const playerChar = player->GetCharacter();
+			const auto& playerName = playerChar ? playerChar->GetName() : "(null player character)";
 			LOG("Player (%s) (%llu) at system address (%s) with sending player (%llu) does not match their own.",
-				player->GetCharacter()->GetName().c_str(), player->GetObjectID(),
+				playerName.c_str(), player->GetObjectID(),
 				sysAddr.ToString(), id);
-			if (player->GetCharacter()) toReport = player->GetCharacter()->GetParentUser();
+			if (playerChar) toReport = playerChar->GetParentUser();
 		// In the rare case that the player does not exist, just log the system address and who the target id was.
 		} else {
 			LOG("Player at system address (%s) with sending player (%llu) does not match their own.",
@@ -76,8 +82,11 @@ void LogAndSaveFailedAntiCheatCheck(const LWOOBJID& id, const SystemAddress& sys
 		auto* user = UserManager::Instance()->GetUser(sysAddr);
 		
 		if (user) {
+			const auto* const lastChar = user->GetLastUsedChar();
+			const auto& lastName = lastChar ? lastChar->GetName() : "(null last char)";
+			const auto lastObjID = lastChar ? lastChar->GetObjectID() : LWOOBJID_EMPTY;
 			LOG("User at system address (%s) (%s) (%llu) sent a packet as (%llu) which is not an id they own.",
-				sysAddr.ToString(), user->GetLastUsedChar()->GetName().c_str(), user->GetLastUsedChar()->GetObjectID(), id);
+				sysAddr.ToString(), lastName.c_str(), lastObjID, id);
 		// Can't know sending player. Just log system address for IP banning.
 		} else {
 			LOG("No user found for system address (%s).", sysAddr.ToString());

dGame/dUtilities/Loot.cpp

@@ -326,7 +326,7 @@ void DropRegularLoot(Team& team, GameMessages::DropClientLoot& lootMsg, const bo
 void DropLoot(Entity* player, const LWOOBJID source, const std::map<LOT, LootDropInfo>& rolledItems, uint32_t minCoins, uint32_t maxCoins, const bool noTeamLootOnDeath) {
 	player = player->GetOwner(); // if the owner is overwritten, we collect that here
 	const auto playerID = player->GetObjectID();
-	if (!player || !player->IsPlayer()) {
+	if (!player->IsPlayer()) {
 		LOG("Trying to drop loot for non-player %llu:%i", playerID, player->GetLOT());
 		return;
 	}

dGame/dUtilities/Mail.cpp

@@ -74,69 +74,74 @@ namespace Mail {
 	void SendRequest::Handle() {
 		SendResponse response;
 		auto* character = player->GetCharacter();
-		const bool restrictMailOnMute = UserManager::Instance()->GetMuteRestrictMail() && character->GetParentUser()->GetIsMuted();
-		const bool restrictedMailAccess = character->HasPermission(ePermissionMap::RestrictedMailAccess);
-
-		if (character && !(restrictedMailAccess || restrictMailOnMute)) {
-			mailInfo.recipient = std::regex_replace(mailInfo.recipient, std::regex("[^0-9a-zA-Z]+"), "");
-			auto receiverID = Database::Get()->GetCharacterInfo(mailInfo.recipient);
-
-			if (!receiverID) {
-				response.status = eSendResponse::RecipientNotFound;
-			} else if (GeneralUtils::CaseInsensitiveStringCompare(mailInfo.recipient, character->GetName()) || receiverID->id == character->GetID()) {
-				response.status = eSendResponse::CannotMailSelf;
-			} else {
-				uint32_t mailCost = Game::zoneManager->GetWorldConfig().mailBaseFee;
-				uint32_t stackSize = 0;
-
-				auto inventoryComponent = player->GetComponent<InventoryComponent>();
-				Item* item = nullptr;
-
-				bool hasAttachment = mailInfo.itemID != 0 && mailInfo.itemCount > 0;
-
-				if (hasAttachment) {
-					item = inventoryComponent->FindItemById(mailInfo.itemID);
-					if (item) {
-						mailCost += (item->GetInfo().baseValue * Game::zoneManager->GetWorldConfig().mailPercentAttachmentFee);
-						mailInfo.itemLOT = item->GetLot();
-					}
-				}
-
-				if (hasAttachment && !item) {
-					response.status = eSendResponse::AttachmentNotFound;
-				} else if (player->GetCharacter()->GetCoins() - mailCost < 0) {
-					response.status = eSendResponse::NotEnoughCoins;
-				} else {
-					bool removeSuccess = true;
-					// Remove coins and items from the sender
-					player->GetCharacter()->SetCoins(player->GetCharacter()->GetCoins() - mailCost, eLootSourceType::MAIL);
-					if (inventoryComponent && hasAttachment && item) {
-						removeSuccess = inventoryComponent->RemoveItem(mailInfo.itemLOT, mailInfo.itemCount, ALL, true);
-						auto* missionComponent = player->GetComponent<MissionComponent>();
-						if (missionComponent && removeSuccess) missionComponent->Progress(eMissionTaskType::GATHER, mailInfo.itemLOT, LWOOBJID_EMPTY, "", -mailInfo.itemCount);
-					}
-
-					// we passed all the checks, now we can actully send the mail
-					if (removeSuccess) {
-						mailInfo.senderId = character->GetID();
-						mailInfo.senderUsername = character->GetName();
-						mailInfo.receiverId = receiverID->id;
-						mailInfo.itemSubkey = LWOOBJID_EMPTY;
-
-						//clear out the attachementID
-						mailInfo.itemID = 0;
-
-						Database::Get()->InsertNewMail(mailInfo);
-						response.status = eSendResponse::Success;
-						character->SaveXMLToDatabase();
-					} else {
-						response.status = eSendResponse::AttachmentNotFound;
-					}
-				}
-			}
+		if (!character) {
+			response.status = eSendResponse::UnknownError;
 		} else {
-			response.status = eSendResponse::SenderAccountIsMuted;
+			const bool restrictMailOnMute = UserManager::Instance()->GetMuteRestrictMail() && character->GetParentUser()->GetIsMuted();
+			const bool restrictedMailAccess = character->HasPermission(ePermissionMap::RestrictedMailAccess);
+
+			if (character && !(restrictedMailAccess || restrictMailOnMute)) {
+				mailInfo.recipient = std::regex_replace(mailInfo.recipient, std::regex("[^0-9a-zA-Z]+"), "");
+				auto receiverID = Database::Get()->GetCharacterInfo(mailInfo.recipient);
+
+				if (!receiverID) {
+					response.status = eSendResponse::RecipientNotFound;
+				} else if (GeneralUtils::CaseInsensitiveStringCompare(mailInfo.recipient, character->GetName()) || receiverID->id == character->GetID()) {
+					response.status = eSendResponse::CannotMailSelf;
+				} else {
+					uint32_t mailCost = Game::zoneManager->GetWorldConfig().mailBaseFee;
+					uint32_t stackSize = 0;
+
+					auto inventoryComponent = player->GetComponent<InventoryComponent>();
+					Item* item = nullptr;
+
+					bool hasAttachment = mailInfo.itemID != 0 && mailInfo.itemCount > 0;
+
+					if (hasAttachment) {
+						item = inventoryComponent->FindItemById(mailInfo.itemID);
+						if (item) {
+							mailCost += (item->GetInfo().baseValue * Game::zoneManager->GetWorldConfig().mailPercentAttachmentFee);
+							mailInfo.itemLOT = item->GetLot();
+						}
+					}
+
+					if (hasAttachment && !item) {
+						response.status = eSendResponse::AttachmentNotFound;
+					} else if (player->GetCharacter()->GetCoins() - mailCost < 0) {
+						response.status = eSendResponse::NotEnoughCoins;
+					} else {
+						bool removeSuccess = true;
+						// Remove coins and items from the sender
+						player->GetCharacter()->SetCoins(player->GetCharacter()->GetCoins() - mailCost, eLootSourceType::MAIL);
+						if (inventoryComponent && hasAttachment && item) {
+							removeSuccess = inventoryComponent->RemoveItem(mailInfo.itemLOT, mailInfo.itemCount, ALL, true);
+							auto* missionComponent = player->GetComponent<MissionComponent>();
+							if (missionComponent && removeSuccess) missionComponent->Progress(eMissionTaskType::GATHER, mailInfo.itemLOT, LWOOBJID_EMPTY, "", -mailInfo.itemCount);
+						}
+
+						// we passed all the checks, now we can actully send the mail
+						if (removeSuccess) {
+							mailInfo.senderId = character->GetID();
+							mailInfo.senderUsername = character->GetName();
+							mailInfo.receiverId = receiverID->id;
+							mailInfo.itemSubkey = LWOOBJID_EMPTY;
+
+							//clear out the attachementID
+							mailInfo.itemID = 0;
+
+							Database::Get()->InsertNewMail(mailInfo);
+							response.status = eSendResponse::Success;
+							character->SaveXMLToDatabase();
+						} else {
+							response.status = eSendResponse::AttachmentNotFound;
+						}
+					}
+				}
+			} else {
+				response.status = eSendResponse::SenderAccountIsMuted;
+			}
 		}
+
 		LOG("Finished send with status %s", StringifiedEnum::ToString(response.status).data());
 		response.Send(sysAddr);
 	}
@@ -193,7 +198,7 @@ namespace Mail {
 
 		if (mailID > 0 && playerID == player->GetObjectID() && inv) {
 			auto playerMail = Database::Get()->GetMail(mailID);
-			if (!playerMail) {
+			if (!playerMail || playerMail->receiverId != player->GetObjectID()) {
 				response.status = eAttachmentCollectResponse::MailNotFound;
 			} else if (!inv->HasSpaceForLoot({ {playerMail->itemLOT, playerMail->itemCount} })) {
 				response.status = eAttachmentCollectResponse::NoSpaceInInventory;
@@ -225,15 +230,21 @@ namespace Mail {
 		DeleteResponse response;
 		response.mailID = mailID;
 
-		auto mailData = Database::Get()->GetMail(mailID);
-		if (mailData && !(mailData->itemLOT > 0 && mailData->itemCount > 0)) {
-			Database::Get()->DeleteMail(mailID);
-			response.status = eDeleteResponse::Success;
-		} else if (mailData && mailData->itemLOT > 0 && mailData->itemCount > 0) {
-			response.status = eDeleteResponse::HasAttachments;
-		} else {
-			response.status = eDeleteResponse::NotFound;
+		const auto mailData = Database::Get()->GetMail(mailID);
+		response.status = eDeleteResponse::NotFound;
+		if (mailData) {
+			if (mailData->receiverId != playerID) {
+				LOG("Player %llu attempted to delete mail owned by %llu. Possible spoof?", playerID, mailData->receiverId);
+			} else {
+				if (!(mailData->itemLOT > 0 && mailData->itemCount > 0)) {
+					Database::Get()->DeleteMail(mailID);
+					response.status = eDeleteResponse::Success;
+				} else if (mailData->itemLOT > 0 && mailData->itemCount > 0) {
+					response.status = eDeleteResponse::HasAttachments;
+				}
+			}
 		}
+
 		LOG("DeleteRequest status %s", StringifiedEnum::ToString(response.status).data());
 		response.Send(sysAddr);
 	}
@@ -253,11 +264,19 @@ namespace Mail {
 
 	void ReadRequest::Handle() {
 		ReadResponse response;
+		response.status = eReadResponse::UnknownError;
 		response.mailID = mailID;
 
-		if (Database::Get()->GetMail(mailID)) {
-			response.status = eReadResponse::Success;
-			Database::Get()->MarkMailRead(mailID);
+		const auto mail = Database::Get()->GetMail(mailID);
+		if (mail) {
+			if (mail->receiverId == player->GetObjectID()) {
+				response.status = eReadResponse::Success;
+				Database::Get()->MarkMailRead(mailID);
+			} else {
+				LOG("Player %llu tried to mark mail read for player %llu", mail->receiverId, player->GetObjectID());
+			}
+		} else {
+			LOG("No mail by ID %llu found to mark as read.", mailID);
 		}
 
 		LOG("ReadRequest %s", StringifiedEnum::ToString(response.status).data());

dGame/dUtilities/Preconditions.cpp

@@ -114,11 +114,13 @@ bool Precondition::Check(Entity* player, bool evaluateCosts) const {
 
 
 bool Precondition::CheckValue(Entity* player, const uint32_t value, bool evaluateCosts) const {
-	auto* missionComponent = player->GetComponent<MissionComponent>();
-	auto* inventoryComponent = player->GetComponent<InventoryComponent>();
-	auto* destroyableComponent = player->GetComponent<DestroyableComponent>();
-	auto* levelComponent = player->GetComponent<LevelProgressionComponent>();
 	auto* character = player->GetCharacter();
+	auto [missionComponent, inventoryComponent, destroyableComponent, levelComponent] =
+		player->GetComponentsMut<const MissionComponent, /* not const */ InventoryComponent, const DestroyableComponent, const LevelProgressionComponent>();
+	
+	if (!missionComponent || !inventoryComponent || !destroyableComponent || !levelComponent || !character) {
+		return false;
+	}
 
 	Mission* mission;
 

dGame/dUtilities/SlashCommands/DEVGMCommands.cpp

@@ -825,7 +825,7 @@ namespace DEVGMCommands {
 		}
 
 		const auto numberToSpawnOptional = GeneralUtils::TryParse<uint32_t>(splitArgs[1]);
-		if (!numberToSpawnOptional && numberToSpawnOptional.value() > 0) {
+		if (!numberToSpawnOptional) {
 			ChatPackets::SendSystemMessage(sysAddr, u"Invalid number of enemies to spawn.");
 			return;
 		}
@@ -833,7 +833,7 @@ namespace DEVGMCommands {
 
 		// Must spawn within a radius of at least 0.0f
 		const auto radiusToSpawnWithinOptional = GeneralUtils::TryParse<float>(splitArgs[2]);
-		if (!radiusToSpawnWithinOptional && radiusToSpawnWithinOptional.value() < 0.0f) {
+		if (!radiusToSpawnWithinOptional || radiusToSpawnWithinOptional.value() < 0.0f) {
 			ChatPackets::SendSystemMessage(sysAddr, u"Invalid radius to spawn within.");
 			return;
 		}
@@ -1133,6 +1133,10 @@ namespace DEVGMCommands {
 		}
 
 		const auto& password = splitArgs[2];
+		if (password.length() >= 50) {
+			ChatPackets::SendSystemMessage(sysAddr, u"Password is too long.");
+			return;
+		}
 
 		ZoneInstanceManager::Instance()->CreatePrivateZone(Game::server, zone.value(), clone.value(), password);
 

dGame/dUtilities/SlashCommands/GMZeroCommands.cpp

@@ -187,8 +187,13 @@ namespace GMZeroCommands {
 		auto splitArgs = GeneralUtils::SplitString(args, ' ');
 		if (splitArgs.empty()) return;
 
-		ChatPackets::SendSystemMessage(sysAddr, u"Requesting private map...");
 		const auto& password = splitArgs[0];
+		if (password.length() >= 50) {
+			ChatPackets::SendSystemMessage(sysAddr, u"Password is too long.");
+			return;
+		}
+
+		ChatPackets::SendSystemMessage(sysAddr, u"Requesting private map...");
 
 		ZoneInstanceManager::Instance()->RequestPrivateZone(Game::server, false, password, [=](bool mythranShift, uint32_t zoneID, uint32_t zoneInstance, uint32_t zoneClone, std::string serverIP, uint16_t serverPort) {
 			LOG("Transferring %s to Zone %i (Instance %i | Clone %i | Mythran Shift: %s) with IP %s and Port %i", sysAddr.ToString(), zoneID, zoneInstance, zoneClone, mythranShift == true ? "true" : "false", serverIP.c_str(), serverPort);