BRNSystems/minetest
1
0
Fork 0
forked from Mirrorlandia_minetest/minetest
Code Pull Requests 1 Activity

Inventory: Protect Craft and Drop actions (#10353)

Browse Source 
Change dangerous pointer to unique_ptr for automated deletion.
This commit is contained in:
SmallJoker 2020-09-07 21:19:38 +02:00 committed by GitHub
parent 6dcc9e6331
commit 0d128ab344
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 35 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
src/network/serverpackethandler.cpp
View File

@ -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())
if (!check_inv_access(ma->from_inv) ||
!check_inv_access(ma->to_inv))
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;
}
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)