fix: login crash and memory corruption (#2951)

Fixed memory corruption using debug build in ~Container
Fixed memory corruption using debug build in Argon2::parseBitShift
Fixed crash on login

Resolves #2947

src/game/game.cpp

@@ -1012,7 +1012,6 @@ std::shared_ptr<Player> Game::getPlayerByGUID(const uint32_t &guid, bool allowOf
 	if (!allowOffline) {
 		return nullptr;
 	}
-
 	std::shared_ptr<Player> tmpPlayer = std::make_shared<Player>(nullptr);
 	if (!IOLoginData::loadPlayerById(tmpPlayer, guid)) {
 		return nullptr;
@@ -1028,8 +1027,6 @@ std::string Game::getPlayerNameByGUID(const uint32_t &guid) {
 	if (m_playerNameCache.contains(guid)) {
 		return m_playerNameCache.at(guid);
 	}
-
-	// This player need read-only purposes and never saved
 	const auto &player = getPlayerByGUID(guid, true);
 	auto name = player ? player->getName() : "";
 	if (!name.empty()) {
@@ -1069,7 +1066,6 @@ std::vector<std::shared_ptr<Player>> Game::getPlayersByAccount(std::shared_ptr<A
 	if (error != enumToValue(AccountErrors_t::Ok)) {
 		return {};
 	}
-
 	std::vector<std::shared_ptr<Player>> ret;
 	for (const auto &[name, _] : accountPlayers) {
 		const auto &player = getPlayerByName(name, allowOffline);

src/io/functions/iologindata_load_player.cpp

@@ -182,9 +182,6 @@ bool IOLoginDataLoad::loadPlayerBasicInfo(std::shared_ptr<Player> player, DBResu
 	player->setMaxManaShield(result->getNumber<uint32_t>("max_manashield"));
 
 	player->setMarriageSpouse(result->getNumber<int32_t>("marriage_spouse"));
-
-	// Experience load
-	IOLoginDataLoad::loadPlayerExperience(player, result);
 	return true;
 }
 

src/io/iologindata.cpp

@@ -92,21 +92,22 @@ void IOLoginData::updateOnlineStatus(uint32_t guid, bool login) {
 	Database::getInstance().executeQuery(query.str());
 }
 
-bool IOLoginData::loadPlayerById(std::shared_ptr<Player> player, uint32_t id) {
+// The boolean "disableIrrelevantInfo" will deactivate the loading of information that is not relevant to the preload, for example, forge, bosstiary, etc. None of this we need to access if the player is offline
+bool IOLoginData::loadPlayerById(std::shared_ptr<Player> player, uint32_t id, bool disableIrrelevantInfo /* = true*/) {
 	Database &db = Database::getInstance();
 	std::ostringstream query;
 	query << "SELECT * FROM `players` WHERE `id` = " << id;
-	return loadPlayer(player, db.storeQuery(query.str()));
+	return loadPlayer(player, db.storeQuery(query.str()), disableIrrelevantInfo);
 }
 
-bool IOLoginData::loadPlayerByName(std::shared_ptr<Player> player, const std::string &name) {
+bool IOLoginData::loadPlayerByName(std::shared_ptr<Player> player, const std::string &name, bool disableIrrelevantInfo /* = true*/) {
 	Database &db = Database::getInstance();
 	std::ostringstream query;
 	query << "SELECT * FROM `players` WHERE `name` = " << db.escapeString(name);
-	return loadPlayer(player, db.storeQuery(query.str()));
+	return loadPlayer(player, db.storeQuery(query.str()), disableIrrelevantInfo);
 }
 
-bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result) {
+bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result, bool disableIrrelevantInfo /* = false*/) {
 	if (!result || !player) {
 		std::string nullptrType = !result ? "Result" : "Player";
 		g_logger().warn("[{}] - {} is nullptr", __FUNCTION__, nullptrType);
@@ -117,6 +118,9 @@ bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result
 		// First
 		IOLoginDataLoad::loadPlayerBasicInfo(player, result);
 
+		// Experience load
+		IOLoginDataLoad::loadPlayerExperience(player, result);
+
 		// Blessings load
 		IOLoginDataLoad::loadPlayerBlessings(player, result);
 
@@ -174,6 +178,10 @@ bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result
 		// Load instant spells list
 		IOLoginDataLoad::loadPlayerInstantSpellList(player, result);
 
+		if (disableIrrelevantInfo) {
+			return true;
+		}
+
 		// load forge history
 		IOLoginDataLoad::loadPlayerForgeHistory(player, result);
 

src/io/iologindata.hpp

@@ -20,9 +20,9 @@ class IOLoginData {
 	static bool gameWorldAuthentication(const std::string &accountDescriptor, const std::string &sessionOrPassword, std::string &characterName, uint32_t &accountId, bool oldProcotol, const uint32_t ip);
 	static uint8_t getAccountType(uint32_t accountId);
 	static void updateOnlineStatus(uint32_t guid, bool login);
-	static bool loadPlayerById(std::shared_ptr<Player> player, uint32_t id);
-	static bool loadPlayerByName(std::shared_ptr<Player> player, const std::string &name);
-	static bool loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result);
+	static bool loadPlayerById(std::shared_ptr<Player> player, uint32_t id, bool disableIrrelevantInfo = true);
+	static bool loadPlayerByName(std::shared_ptr<Player> player, const std::string &name, bool disableIrrelevantInfo = true);
+	static bool loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result, bool disableIrrelevantInfo = false);
 	static bool savePlayer(std::shared_ptr<Player> player);
 	static uint32_t getGuidByName(const std::string &name);
 	static bool getGuidByNameEx(uint32_t &guid, bool &specialVip, std::string &name);

src/items/containers/container.cpp

@@ -61,12 +61,21 @@ std::shared_ptr<Container> Container::create(std::shared_ptr<Tile> tile) {
 
 Container::~Container() {
 	if (getID() == ITEM_BROWSEFIELD) {
-		if (getParent() && getParent()->getTile()) {
-			g_game().browseFields.erase(getParent()->getTile());
-		}
+		auto parent = getParent();
+		if (parent) {
+			auto tile = parent->getTile();
+			if (tile) {
+				auto browseField = g_game().browseFields.find(tile);
+				if (browseField != g_game().browseFields.end()) {
+					g_game().browseFields.erase(browseField);
+				}
+			}
 
-		for (std::shared_ptr<Item> item : itemlist) {
-			item->setParent(getParent());
+			for (auto &item : itemlist) {
+				if (item) {
+					item->setParent(parent);
+				}
+			}
 		}
 	}
 }

src/security/argon.cpp

@@ -25,17 +25,30 @@ void Argon2::updateConstants() {
 }
 
 uint32_t Argon2::parseBitShift(const std::string &bitShiftStr) const {
-	std::stringstream ss(bitShiftStr);
-	int base;
-	int shift;
-	char op1;
-	char op2;
-
-	if (!(ss >> base >> op1 >> op2 >> shift) || op1 != '<' || op2 != '<') {
-		g_logger().warn("Invalid bit shift string");
+	static const std::regex pattern(R"(^\s*(\d+)\s*<<\s*(\d+)\s*$)");
+	std::smatch match;
+
+	if (!std::regex_match(bitShiftStr, match, pattern)) {
+		g_logger().warn("Invalid bit shift string format: '{}'", bitShiftStr);
+		return 0;
+	}
+
+	int base = 0;
+	int shift = 0;
+	try {
+		base = std::stoi(match[1].str());
+		shift = std::stoi(match[2].str());
+	} catch (const std::exception &e) {
+		g_logger().warn("Error parsing bit shift string: '{}'", e.what());
+		return 0;
+	}
+
+	if (shift < 0 || shift >= 32) {
+		g_logger().warn("Shift value out of bounds: '{}'", shift);
+		return 0;
 	}
 
-	return base << shift;
+	return static_cast<uint32_t>(base) << shift;
 }
 
 bool Argon2::verifyPassword(const std::string &password, const std::string &phash) const {

src/server/network/message/networkmessage.cpp

@@ -44,24 +44,24 @@ int32_t NetworkMessage::decodeHeader() {
 }
 
 // Simply read functions for incoming message
-uint8_t NetworkMessage::getByte() {
+uint8_t NetworkMessage::getByte(const std::source_location &location /*= std::source_location::current()*/) {
 	// Check if there is at least 1 byte to read
 	if (!canRead(1)) {
-		g_logger().error("[{}] Not enough data to read a byte. Current position: {}, Length: {}", __FUNCTION__, info.position, info.length);
+		g_logger().error("[{}] Not enough data to read a byte. Current position: {}, Length: {}. Called line {}:{} in {}", __FUNCTION__, info.position, info.length, location.line(), location.column(), location.function_name());
 		return {};
 	}
 
 	// Ensure that position is within bounds before decrementing
 	if (info.position == 0) {
-		g_logger().error("[{}] Position is at the beginning of the buffer. Cannot decrement.", __FUNCTION__);
+		g_logger().error("[{}] Position is at the beginning of the buffer. Cannot decrement. Called line {}:{} in {}", __FUNCTION__, location.line(), location.column(), location.function_name());
 		return {};
 	}
 
 	try {
 		// Decrement position safely and return the byte
 		return buffer.at(info.position++);
 	} catch (const std::out_of_range &e) {
-		g_logger().error("[{}] Out of range error: {}. Position: {}, Buffer size: {}", __FUNCTION__, e.what(), info.position, buffer.size());
+		g_logger().error("[{}] Out of range error: {}. Position: {}, Buffer size: {}. Called line {}:{} in {}", __FUNCTION__, e.what(), info.position, buffer.size(), location.line(), location.column(), location.function_name());
 	}
 
 	return {};

src/server/network/message/networkmessage.hpp

@@ -34,7 +34,7 @@ class NetworkMessage {
 	}
 
 	// simply read functions for incoming message
-	uint8_t getByte();
+	uint8_t getByte(const std::source_location &location = std::source_location::current());
 
 	uint8_t getPreviousByte();
 

src/server/network/protocol/protocolgame.cpp

@@ -574,7 +574,7 @@ void ProtocolGame::login(const std::string &name, uint32_t accountId, OperatingS
 			return;
 		}
 
-		if (!IOLoginData::loadPlayerById(player, player->getGUID())) {
+		if (!IOLoginData::loadPlayerById(player, player->getGUID(), false)) {
 			g_game().removePlayerUniqueLogin(player);
 			disconnectClient("Your character could not be loaded.");
 			g_logger().warn("Player {} could not be loaded", player->getName());