Skip to content
Permalink
Browse files

Inventory: Protect Craft and Drop actions (#10353)

Change dangerous pointer to unique_ptr for automated deletion.
  • Loading branch information
SmallJoker committed Sep 7, 2020
1 parent 6dcc9e6 commit 0d128ab344e3d04d2b30dbd5e047f4ac700013b7
Showing with 35 additions and 44 deletions.
  1. +35 −44 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,26 +684,14 @@ 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;
}
}
/*
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,52 +704,47 @@ 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;
}
}
/*
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)

0 comments on commit 0d128ab

Please sign in to comment.
You can’t perform that action at this time.