Skip to content

Commit

Permalink
Inv deSerialize(): Prevent infinite loop, error on failure (#7711)
Browse files Browse the repository at this point in the history
Throws an error about potentially damaged player inventories but proceeds converting the rest of them
  • Loading branch information
SmallJoker authored Sep 14, 2018
1 parent 81c06de commit 220ec79
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 17 deletions.
41 changes: 25 additions & 16 deletions src/inventory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,7 @@ void InventoryList::deSerialize(std::istream &is)
u32 item_i = 0;
m_width = 0;

for(;;)
{
while (is.good()) {
std::string line;
std::getline(is, line, '\n');

Expand All @@ -447,14 +446,12 @@ void InventoryList::deSerialize(std::istream &is)
std::string name;
std::getline(iss, name, ' ');

if (name == "EndInventoryList") {
break;
}
if (name == "EndInventoryList")
return;

// This is a temporary backwards compatibility fix
if (name == "end") {
break;
}
if (name == "end")
return;

if (name == "Width") {
iss >> m_width;
Expand All @@ -476,6 +473,14 @@ void InventoryList::deSerialize(std::istream &is)
m_items[item_i++].clear();
}
}

// Contents given to deSerialize() were not terminated properly: throw error.

std::ostringstream ss;
ss << "Malformatted inventory list. list="
<< m_name << ", read " << item_i << " of " << getSize()
<< " ItemStacks." << std::endl;
throw SerializationError(ss.str());
}

InventoryList::InventoryList(const InventoryList &other)
Expand Down Expand Up @@ -859,8 +864,7 @@ void Inventory::deSerialize(std::istream &is)
{
clear();

for(;;)
{
while (is.good()) {
std::string line;
std::getline(is, line, '\n');

Expand All @@ -869,14 +873,12 @@ void Inventory::deSerialize(std::istream &is)
std::string name;
std::getline(iss, name, ' ');

if (name == "EndInventory") {
break;
}
if (name == "EndInventory")
return;

// This is a temporary backwards compatibility fix
if (name == "end") {
break;
}
if (name == "end")
return;

if (name == "List") {
std::string listname;
Expand All @@ -895,6 +897,13 @@ void Inventory::deSerialize(std::istream &is)
throw SerializationError("invalid inventory specifier: " + name);
}
}

// Contents given to deSerialize() were not terminated properly: throw error.

std::ostringstream ss;
ss << "Malformatted inventory (damaged?). "
<< m_lists.size() << " lists read." << std::endl;
throw SerializationError(ss.str());
}

InventoryList * Inventory::addList(const std::string &name, u32 size)
Expand Down
7 changes: 6 additions & 1 deletion src/remoteplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,12 @@ void RemotePlayer::deSerialize(std::istream &is, const std::string &playername,
} catch (SettingNotFoundException &e) {}
}

inventory.deSerialize(is);
try {
inventory.deSerialize(is);
} catch (SerializationError &e) {
errorstream << "Failed to deserialize player inventory. player_name="
<< name << " " << e.what() << std::endl;
}

if (!inventory.getList("craftpreview") && inventory.getList("craftresult")) {
// Convert players without craftpreview
Expand Down

0 comments on commit 220ec79

Please sign in to comment.