diff options
| -rw-r--r-- | src/network/serverpackethandler.cpp | 79 |
1 files changed, 35 insertions, 44 deletions
diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index fe70d376e..0bd09e637 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -600,7 +600,7 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) << std::endl; std::istringstream is(datastring, std::ios_base::binary); // Create an action - InventoryAction *a = InventoryAction::deSerialize(is); + std::unique_ptr<InventoryAction> a(InventoryAction::deSerialize(is)); if (!a) { infostream << "TOSERVER_INVENTORY_ACTION: " << "InventoryAction::deSerialize() returned NULL" @@ -617,11 +617,30 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) where the client made a bad prediction. */ + const bool player_has_interact = checkPriv(player->getName(), "interact"); + + auto check_inv_access = [player, player_has_interact] ( + const InventoryLocation &loc) -> bool { + if (loc.type == InventoryLocation::CURRENT_PLAYER) + return false; // Only used internally on the client, never sent + if (loc.type == InventoryLocation::PLAYER) { + // Allow access to own inventory in all cases + return loc.name == player->getName(); + } + + if (!player_has_interact) { + infostream << "Cannot modify foreign inventory: " + << "No interact privilege" << std::endl; + return false; + } + return true; + }; + /* Handle restrictions and special cases of the move action */ if (a->getType() == IAction::Move) { - IMoveAction *ma = (IMoveAction*)a; + IMoveAction *ma = (IMoveAction*)a.get(); ma->from_inv.applyCurrentPlayer(player->getName()); ma->to_inv.applyCurrentPlayer(player->getName()); @@ -630,21 +649,11 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) if (ma->from_inv != ma->to_inv) m_inventory_mgr->setInventoryModified(ma->to_inv); - bool from_inv_is_current_player = false; - if (ma->from_inv.type == InventoryLocation::PLAYER) { - if (ma->from_inv.name != player->getName()) - return; - from_inv_is_current_player = true; - } - - bool to_inv_is_current_player = false; - if (ma->to_inv.type == InventoryLocation::PLAYER) { - if (ma->to_inv.name != player->getName()) - return; - to_inv_is_current_player = true; - } + if (!check_inv_access(ma->from_inv) || + !check_inv_access(ma->to_inv)) + return; - InventoryLocation *remote = from_inv_is_current_player ? + InventoryLocation *remote = ma->from_inv.type == InventoryLocation::PLAYER ? &ma->to_inv : &ma->from_inv; // Check for out-of-range interaction @@ -664,7 +673,6 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) << (ma->from_inv.dump()) << ":" << ma->from_list << " to " << (ma->to_inv.dump()) << ":" << ma->to_list << " because src is " << ma->from_list << std::endl; - delete a; return; } @@ -676,18 +684,6 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) << (ma->from_inv.dump()) << ":" << ma->from_list << " to " << (ma->to_inv.dump()) << ":" << ma->to_list << " because dst is " << ma->to_list << std::endl; - delete a; - return; - } - - // Disallow moving items in elsewhere than player's inventory - // if not allowed to interact - if (!checkPriv(player->getName(), "interact") && - (!from_inv_is_current_player || - !to_inv_is_current_player)) { - infostream << "Cannot move outside of player's inventory: " - << "No interact privilege" << std::endl; - delete a; return; } } @@ -695,7 +691,7 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) Handle restrictions and special cases of the drop action */ else if (a->getType() == IAction::Drop) { - IDropAction *da = (IDropAction*)a; + IDropAction *da = (IDropAction*)a.get(); da->from_inv.applyCurrentPlayer(player->getName()); @@ -708,22 +704,18 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) infostream << "Ignoring IDropAction from " << (da->from_inv.dump()) << ":" << da->from_list << " because src is " << da->from_list << std::endl; - delete a; return; } // Disallow dropping items if not allowed to interact - if (!checkPriv(player->getName(), "interact")) { - delete a; + if (!player_has_interact || !check_inv_access(da->from_inv)) return; - } // Disallow dropping items if dead if (playersao->isDead()) { infostream << "Ignoring IDropAction from " << (da->from_inv.dump()) << ":" << da->from_list << " because player is dead." << std::endl; - delete a; return; } } @@ -731,29 +723,28 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) Handle restrictions and special cases of the craft action */ else if (a->getType() == IAction::Craft) { - ICraftAction *ca = (ICraftAction*)a; + ICraftAction *ca = (ICraftAction*)a.get(); ca->craft_inv.applyCurrentPlayer(player->getName()); m_inventory_mgr->setInventoryModified(ca->craft_inv); - //bool craft_inv_is_current_player = - // (ca->craft_inv.type == InventoryLocation::PLAYER) && - // (ca->craft_inv.name == player->getName()); - // Disallow crafting if not allowed to interact - if (!checkPriv(player->getName(), "interact")) { + if (!player_has_interact) { infostream << "Cannot craft: " << "No interact privilege" << std::endl; - delete a; return; } + + if (!check_inv_access(ca->craft_inv)) + return; + } else { + // Unknown action. Ignored. + return; } // Do the action a->apply(m_inventory_mgr.get(), playersao, this); - // Eat the action - delete a; } void Server::handleCommand_ChatMessage(NetworkPacket* pkt) |