From 4efa3e9dd8e5ea57f250495a52439e7f474920c0 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Sun, 14 May 2023 21:47:58 -0700 Subject: [PATCH 1/4] fix: possible crash on method "getTile" on move item --- src/creatures/combat/combat.cpp | 14 +++++---- src/creatures/creature.cpp | 24 +++++++++++----- src/creatures/creature.h | 6 +++- src/creatures/players/player.cpp | 13 ++++++--- src/creatures/players/player.h | 30 +++++++++++++++++--- src/game/game.cpp | 10 +++++-- src/server/network/protocol/protocolgame.cpp | 30 +++++++++++++++----- 7 files changed, 96 insertions(+), 31 deletions(-) diff --git a/src/creatures/combat/combat.cpp b/src/creatures/combat/combat.cpp index 270cc0170d2..9541e07a074 100644 --- a/src/creatures/combat/combat.cpp +++ b/src/creatures/combat/combat.cpp @@ -276,6 +276,8 @@ ReturnValue Combat::canDoCombat(Creature* attacker, Creature* target) { return RETURNVALUE_YOUMAYNOTATTACKTHISPLAYER; } + const Tile* targetPlayerTile = targetPlayer->getTile(); + if (const Player* attackerPlayer = attacker->getPlayer()) { if (attackerPlayer->hasFlag(PlayerFlags_t::CannotAttackPlayer)) { return RETURNVALUE_YOUMAYNOTATTACKTHISPLAYER; @@ -286,10 +288,10 @@ ReturnValue Combat::canDoCombat(Creature* attacker, Creature* target) { } // nopvp-zone - if (const Tile* targetPlayerTile = targetPlayer->getTile(); - targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE)) { + auto attackerTile = attackerPlayer->getTile(); + if (targetPlayerTile && targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE)) { return RETURNVALUE_ACTIONNOTPERMITTEDINANOPVPZONE; - } else if (attackerPlayer->getTile()->hasFlag(TILESTATE_NOPVPZONE) && !targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE | TILESTATE_PROTECTIONZONE)) { + } else if (attackerTile && attackerTile->hasFlag(TILESTATE_NOPVPZONE) && !targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE | TILESTATE_PROTECTIONZONE)) { return RETURNVALUE_ACTIONNOTPERMITTEDINANOPVPZONE; } @@ -304,7 +306,7 @@ ReturnValue Combat::canDoCombat(Creature* attacker, Creature* target) { return RETURNVALUE_YOUMAYNOTATTACKTHISPLAYER; } - if (targetPlayer->getTile()->hasFlag(TILESTATE_NOPVPZONE)) { + if (targetPlayerTile && targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE)) { return RETURNVALUE_ACTIONNOTPERMITTEDINANOPVPZONE; } @@ -1651,8 +1653,8 @@ void MagicField::onStepInField(Creature &creature) { auto ownerId = getAttribute(ItemAttribute_t::OWNER); if (ownerId) { bool harmfulField = true; - - if (g_game().getWorldType() == WORLD_TYPE_NO_PVP || getTile()->hasFlag(TILESTATE_NOPVPZONE)) { + auto itemTile = getTile(); + if (g_game().getWorldType() == WORLD_TYPE_NO_PVP || itemTile && itemTile->hasFlag(TILESTATE_NOPVPZONE)) { Creature* owner = g_game().getCreatureByID(ownerId); if (owner) { if (owner->getPlayer() || (owner->isSummon() && owner->getMaster()->getPlayer())) { diff --git a/src/creatures/creature.cpp b/src/creatures/creature.cpp index e12f18e681a..ba033abf178 100644 --- a/src/creatures/creature.cpp +++ b/src/creatures/creature.cpp @@ -419,18 +419,24 @@ void Creature::checkSummonMove(const Position &newPos, bool teleportSummon) cons const Position &pos = creature->getPosition(); const Monster* monster = creature->getMonster(); - bool protectionZoneCheck = this->getTile()->hasFlag(TILESTATE_PROTECTIONZONE); + auto creatureTile = getTile(); + bool protectionZoneCheck = creatureTile ? creatureTile->hasFlag(TILESTATE_PROTECTIONZONE) : false; // Check if any of our summons is out of range (+/- 0 floors or 15 tiles away) bool checkSummonDist = Position::getDistanceZ(newPos, pos) > 0 || (std::max(Position::getDistanceX(newPos, pos), Position::getDistanceY(newPos, pos)) > 15); // Check if any of our summons is out of range (+/- 2 floors or 30 tiles away) bool checkRemoveDist = Position::getDistanceZ(newPos, pos) > 2 || (std::max(Position::getDistanceX(newPos, pos), Position::getDistanceY(newPos, pos)) > 30); if (monster && monster->isFamiliar() && checkSummonDist || teleportSummon && !protectionZoneCheck && checkSummonDist) { - if (Tile* masterTile = creature->getMaster()->getTile()) { + auto creatureMaster = creature->getMaster(); + if (!creatureMaster) { + continue; + } + + if (Tile* masterTile = creatureMaster->getTile()) { if (masterTile->hasFlag(TILESTATE_TELEPORT)) { - SPDLOG_WARN("[{}] cannot teleport summon, position has teleport. {}", __FUNCTION__, creature->getMaster()->getPosition().toString()); + SPDLOG_WARN("[{}] cannot teleport summon, position has teleport. {}", __FUNCTION__, creatureMaster->getPosition().toString()); } else { - g_game().internalTeleport(creature, creature->getMaster()->getPosition(), true); + g_game().internalTeleport(creature, creatureMaster->getPosition(), true); continue; } } @@ -880,7 +886,9 @@ BlockType_t Creature::blockHit(Creature* attacker, CombatType_t combatType, int3 bool Creature::setAttackedCreature(Creature* creature) { if (creature) { - if (this->getMonster() && this->getMonster()->isFamiliar() && this->getTile() && this->getTile()->hasFlag(TILESTATE_PROTECTIONZONE)) { + auto monster = getMonster(); + auto tile = getTile(); + if (monster && monster->isFamiliar() && tile && tile->hasFlag(TILESTATE_PROTECTIONZONE)) { return false; } @@ -962,7 +970,8 @@ void Creature::goToFollowCreature() { } bool Creature::canFollowMaster() const { - return !master->getTile()->hasFlag(TILESTATE_PROTECTIONZONE) && (canSeeInvisibility() || !master->isInvisible()); + auto tile = master->getTile(); + return tile && !tile->hasFlag(TILESTATE_PROTECTIONZONE) && (canSeeInvisibility() || !master->isInvisible()); } bool Creature::setFollowCreature(Creature* creature) { @@ -1056,7 +1065,8 @@ void Creature::onEndCondition(ConditionType_t) { } void Creature::onTickCondition(ConditionType_t type, bool &bRemove) { - const MagicField* field = getTile()->getFieldItem(); + auto tile = getTile(); + const MagicField* field = tile ? tile->getFieldItem() : nullptr; if (!field) { return; } diff --git a/src/creatures/creature.h b/src/creatures/creature.h index 3e1c7b43711..4f91cf7b8f1 100644 --- a/src/creatures/creature.h +++ b/src/creatures/creature.h @@ -231,7 +231,11 @@ class Creature : virtual public Thing { } bool isInvisible() const; ZoneType_t getZone() const { - return getTile()->getZone(); + if (auto tile = getTile()) { + return tile->getZone(); + } + + return ZONE_NORMAL; } // walk functions diff --git a/src/creatures/players/player.cpp b/src/creatures/players/player.cpp index 5ad114b7b76..b8c2e98d101 100644 --- a/src/creatures/players/player.cpp +++ b/src/creatures/players/player.cpp @@ -1928,8 +1928,8 @@ void Player::onThink(uint32_t interval) { // Momentum (cooldown resets) triggerMomentum(); - - if (!getTile()->hasFlag(TILESTATE_NOLOGOUT) && !isAccessPlayer() && !isExerciseTraining()) { + auto playerTile = getTile(); + if (playerTile && !playerTile->hasFlag(TILESTATE_NOLOGOUT) && !isAccessPlayer() && !isExerciseTraining()) { idleTime += interval; const int32_t kickAfterMinutes = g_configManager().getNumber(KICK_AFTER_MINUTES); if (idleTime > (kickAfterMinutes * 60000) + 60000) { @@ -4525,11 +4525,16 @@ bool Player::canLogout() { return false; } - if (getTile()->hasFlag(TILESTATE_NOLOGOUT)) { + auto tile = getTile(); + if (!tile) { + return false; + } + + if (tile->hasFlag(TILESTATE_NOLOGOUT)) { return false; } - if (getTile()->hasFlag(TILESTATE_PROTECTIONZONE)) { + if (tile->hasFlag(TILESTATE_PROTECTIONZONE)) { return true; } diff --git a/src/creatures/players/player.h b/src/creatures/players/player.h index 24061bc9c81..ee9823b95a5 100644 --- a/src/creatures/players/player.h +++ b/src/creatures/players/player.h @@ -937,8 +937,17 @@ class Player final : public Creature, public Cylinder { } } void sendCreatureAppear(const Creature* creature, const Position &pos, bool isLogin) { + if (!creature) { + return; + } + + auto tile = creature->getTile(); + if (!tile) { + return; + } + if (client) { - client->sendAddCreature(creature, pos, creature->getTile()->getStackposOfCreature(this, creature), isLogin); + client->sendAddCreature(creature, pos, tile->getStackposOfCreature(this, creature), isLogin); } } void sendCreatureMove(const Creature* creature, const Position &newPos, int32_t newStackPos, const Position &oldPos, int32_t oldStackPos, bool teleport) { @@ -947,8 +956,17 @@ class Player final : public Creature, public Cylinder { } } void sendCreatureTurn(const Creature* creature) { + if (!creature) { + return; + } + + auto tile = creature->getTile(); + if (!tile) { + return; + } + if (client && canSeeCreature(creature)) { - int32_t stackpos = creature->getTile()->getStackposOfCreature(this, creature); + int32_t stackpos = tile->getStackposOfCreature(this, creature); if (stackpos != -1) { client->sendCreatureTurn(creature, stackpos); } @@ -980,7 +998,7 @@ class Player final : public Creature, public Cylinder { } } void sendCreatureChangeVisible(const Creature* creature, bool visible) { - if (!client) { + if (!client || !creature) { return; } @@ -994,7 +1012,11 @@ class Player final : public Creature, public Cylinder { } else if (canSeeInvisibility()) { client->sendCreatureOutfit(creature, creature->getCurrentOutfit()); } else { - int32_t stackpos = creature->getTile()->getStackposOfCreature(this, creature); + auto tile = creature->getTile(); + if (!tile) { + return; + } + int32_t stackpos = tile->getStackposOfCreature(this, creature); if (stackpos == -1) { return; } diff --git a/src/game/game.cpp b/src/game/game.cpp index 14826cacd64..29dbc887c70 100644 --- a/src/game/game.cpp +++ b/src/game/game.cpp @@ -1095,6 +1095,10 @@ void Game::playerMoveCreature(Player* player, Creature* movingCreature, const Po } ReturnValue Game::internalMoveCreature(Creature* creature, Direction direction, uint32_t flags /*= 0*/) { + if (!creature) { + return RETURNVALUE_NOTPOSSIBLE; + } + creature->setLastPosition(creature->getPosition()); const Position ¤tPos = creature->getPosition(); Position destPos = getNextPosition(direction, currentPos); @@ -1103,7 +1107,8 @@ ReturnValue Game::internalMoveCreature(Creature* creature, Direction direction, bool diagonalMovement = (direction & DIRECTION_DIAGONAL_MASK) != 0; if (player && !diagonalMovement) { // try go up - if (currentPos.z != 8 && creature->getTile()->hasHeight(3)) { + auto tile = creature->getTile(); + if (currentPos.z != 8 && tile && tile->hasHeight(3)) { Tile* tmpTile = map.getTile(currentPos.x, currentPos.y, currentPos.getZ() - 1); if (tmpTile == nullptr || (tmpTile->getGround() == nullptr && !tmpTile->hasFlag(TILESTATE_BLOCKSOLID))) { tmpTile = map.getTile(destPos.x, destPos.y, destPos.getZ() - 1); @@ -1290,7 +1295,8 @@ void Game::playerMoveItem(Player* player, const Position &fromPos, uint16_t item } const Position &playerPos = player->getPosition(); - const Position &mapFromPos = fromCylinder->getTile()->getPosition(); + auto cylinderTile = fromCylinder->getTile(); + const Position &mapFromPos = cylinderTile ? cylinderTile->getPosition() : item->getPosition(); if (playerPos.z != mapFromPos.z) { player->sendCancelMessage(playerPos.z > mapFromPos.z ? RETURNVALUE_FIRSTGOUPSTAIRS : RETURNVALUE_FIRSTGODOWNSTAIRS); return; diff --git a/src/server/network/protocol/protocolgame.cpp b/src/server/network/protocol/protocolgame.cpp index bf30e146d8f..2ac7121ed21 100644 --- a/src/server/network/protocol/protocolgame.cpp +++ b/src/server/network/protocol/protocolgame.cpp @@ -452,13 +452,14 @@ void ProtocolGame::logout(bool displayEffect, bool forced) { } bool removePlayer = !player->isRemoved() && !forced; + auto tile = player->getTile(); if (removePlayer && !player->isAccessPlayer()) { - if (player->getTile()->hasFlag(TILESTATE_NOLOGOUT)) { + if (tile && tile->hasFlag(TILESTATE_NOLOGOUT)) { player->sendCancelMessage(RETURNVALUE_YOUCANNOTLOGOUTHERE); return; } - if (!player->getTile()->hasFlag(TILESTATE_PROTECTIONZONE) && player->hasCondition(CONDITION_INFIGHT)) { + if (tile && !tile->hasFlag(TILESTATE_PROTECTIONZONE) && player->hasCondition(CONDITION_INFIGHT)) { player->sendCancelMessage(RETURNVALUE_YOUMAYNOTLOGOUTDURINGAFIGHT); return; } @@ -2931,13 +2932,18 @@ void ProtocolGame::sendCreatureShield(const Creature* creature) { } void ProtocolGame::sendCreatureEmblem(const Creature* creature) { - if (!canSee(creature) || oldProtocol) { + if (!creature || !canSee(creature) || oldProtocol) { + return; + } + + auto tile = creature->getTile(); + if (!tile) { return; } // Remove creature from client and re-add to update Position pos = creature->getPosition(); - int32_t stackpos = creature->getTile()->getClientIndexOfCreature(player, creature); + int32_t stackpos = tile->getClientIndexOfCreature(player, creature); sendRemoveTileThing(pos, stackpos); NetworkMessage msg; msg.addByte(0x6A); @@ -7239,10 +7245,15 @@ void ProtocolGame::sendItemsPrice() { } void ProtocolGame::reloadCreature(const Creature* creature) { - if (!canSee(creature)) + if (!creature || !canSee(creature)) + return; + + auto tile = creature->getTile(); + if (!tile) { return; + } - uint32_t stackpos = creature->getTile()->getClientIndexOfCreature(player, creature); + uint32_t stackpos = tile->getClientIndexOfCreature(player, creature); if (stackpos >= 10) return; @@ -7469,10 +7480,15 @@ void ProtocolGame::sendUpdateCreature(const Creature* creature) { return; } + auto tile = creature->getTile(); + if (!tile) { + return; + } + if (!canSee(creature)) return; - int32_t stackPos = creature->getTile()->getClientIndexOfCreature(player, creature); + int32_t stackPos = tile->getClientIndexOfCreature(player, creature); if (stackPos == -1 || stackPos >= 10) { return; } From 9b8ff11ac88b83563ee08dba93b007178f5d6d29 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Mon, 15 May 2023 15:17:23 -0700 Subject: [PATCH 2/4] fix bug --- src/creatures/combat/combat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/creatures/combat/combat.cpp b/src/creatures/combat/combat.cpp index 9541e07a074..950f0e87661 100644 --- a/src/creatures/combat/combat.cpp +++ b/src/creatures/combat/combat.cpp @@ -291,7 +291,7 @@ ReturnValue Combat::canDoCombat(Creature* attacker, Creature* target) { auto attackerTile = attackerPlayer->getTile(); if (targetPlayerTile && targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE)) { return RETURNVALUE_ACTIONNOTPERMITTEDINANOPVPZONE; - } else if (attackerTile && attackerTile->hasFlag(TILESTATE_NOPVPZONE) && !targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE | TILESTATE_PROTECTIONZONE)) { + } else if (attackerTile && attackerTile->hasFlag(TILESTATE_NOPVPZONE) && targetPlayerTile && !targetPlayerTile->hasFlag(TILESTATE_NOPVPZONE | TILESTATE_PROTECTIONZONE)) { return RETURNVALUE_ACTIONNOTPERMITTEDINANOPVPZONE; } From 30804c68515af8272ad3b671f306186dd7fcb98e Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Mon, 15 May 2023 15:19:55 -0700 Subject: [PATCH 3/4] fix: use member variable instead of create new --- src/creatures/creature.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/creatures/creature.cpp b/src/creatures/creature.cpp index ba033abf178..aedec0d031d 100644 --- a/src/creatures/creature.cpp +++ b/src/creatures/creature.cpp @@ -419,8 +419,7 @@ void Creature::checkSummonMove(const Position &newPos, bool teleportSummon) cons const Position &pos = creature->getPosition(); const Monster* monster = creature->getMonster(); - auto creatureTile = getTile(); - bool protectionZoneCheck = creatureTile ? creatureTile->hasFlag(TILESTATE_PROTECTIONZONE) : false; + bool protectionZoneCheck = tile ? tile->hasFlag(TILESTATE_PROTECTIONZONE) : false; // Check if any of our summons is out of range (+/- 0 floors or 15 tiles away) bool checkSummonDist = Position::getDistanceZ(newPos, pos) > 0 || (std::max(Position::getDistanceX(newPos, pos), Position::getDistanceY(newPos, pos)) > 15); // Check if any of our summons is out of range (+/- 2 floors or 30 tiles away) @@ -887,7 +886,6 @@ BlockType_t Creature::blockHit(Creature* attacker, CombatType_t combatType, int3 bool Creature::setAttackedCreature(Creature* creature) { if (creature) { auto monster = getMonster(); - auto tile = getTile(); if (monster && monster->isFamiliar() && tile && tile->hasFlag(TILESTATE_PROTECTIONZONE)) { return false; } @@ -1065,7 +1063,6 @@ void Creature::onEndCondition(ConditionType_t) { } void Creature::onTickCondition(ConditionType_t type, bool &bRemove) { - auto tile = getTile(); const MagicField* field = tile ? tile->getFieldItem() : nullptr; if (!field) { return; From c22ef3ec7994f5fac4c1ff7c8304d5a9d2598f87 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Mon, 15 May 2023 15:20:32 -0700 Subject: [PATCH 4/4] fix: declaration duplicated --- src/creatures/creature.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/creatures/creature.h b/src/creatures/creature.h index 4f91cf7b8f1..ab6a556ee60 100644 --- a/src/creatures/creature.h +++ b/src/creatures/creature.h @@ -231,7 +231,7 @@ class Creature : virtual public Thing { } bool isInvisible() const; ZoneType_t getZone() const { - if (auto tile = getTile()) { + if (getTile()) { return tile->getZone(); }