1
0
mirror of https://github.com/luanti-org/luanti.git synced 2026-01-12 20:25:26 +01:00

Serialize: Throw exception on incomplete reads (#16796)

Several mistakes were made past where the stream was expected to raise
the EOF flag when reaching the end of stream. That is incorrect. The
flag is only raised if the current read operation fails.

This commit unifies all istream compatibility code to use 'canRead'
for reliable EOF detection. An exception is now thrown when a
value cannot be read completely (e.g. missing bytes).
Version comments are added for easier backtracing.
This commit is contained in:
SmallJoker
2026-01-03 11:13:14 +01:00
committed by GitHub
parent 6079d762ce
commit 3d10d4e859
13 changed files with 207 additions and 179 deletions

View File

@@ -979,6 +979,20 @@ void Client::ReceiveAll()
if (!m_con->TryReceive(&pkt))
break;
ProcessData(&pkt);
#ifdef NDEBUG
} catch (SerializationError &e) {
// Add more information to find the bad packet
const u16 cmd = pkt.getCommand();
std::ostringstream oss;
oss << e.what()
<< "\n @ command=" << cmd << " size=" << pkt.getSize();
if (cmd < TOCLIENT_NUM_MSG_TYPES) {
auto def = toClientCommandTable[pkt.getCommand()];
oss << " name=" << def.name;
}
throw SerializationError(oss.str());
#endif
} catch (const con::InvalidIncomingDataException &e) {
infostream << "Client::ReceiveAll(): "
"InvalidIncomingDataException: what()="

View File

@@ -2,7 +2,7 @@
// SPDX-License-Identifier: LGPL-2.1-or-later
// Copyright (C) 2010-2017 celeron55, Perttu Ahola <celeron55@gmail.com>
#include "util/serialize.h"
#include "util/serialize.h" // serializeJsonString
#include "util/pointedthing.h"
#include "client.h"
#include "clientenvironment.h"
@@ -319,9 +319,14 @@ void ClientEnvironment::addActiveObject(u16 id, u8 type,
obj->setId(id);
try {
#ifdef NDEBUG
try
#endif
{
obj->initialize(init_data);
} catch(SerializationError &e) {
}
#ifdef NDEBUG
catch (SerializationError &e) {
errorstream<<"ClientEnvironment::addActiveObject():"
<<" id="<<id<<" type="<<type
<<": SerializationError in initialize(): "
@@ -329,6 +334,7 @@ void ClientEnvironment::addActiveObject(u16 id, u8 type,
<<": init_data="<<serializeJsonString(init_data)
<<std::endl;
}
#endif
u16 new_id = addActiveObject(std::move(obj));
// Object initialized:
@@ -370,14 +376,20 @@ void ClientEnvironment::processActiveObjectMessage(u16 id, const std::string &da
return;
}
try {
#ifdef NDEBUG
try
#endif
{
obj->processMessage(data);
} catch (SerializationError &e) {
}
#ifdef NDEBUG
catch (SerializationError &e) {
errorstream<<"ClientEnvironment::processActiveObjectMessage():"
<< " id=" << id << " type=" << obj->getType()
<< " SerializationError in processMessage(): " << e.what()
<< std::endl;
}
#endif
}
/*

View File

@@ -1586,60 +1586,37 @@ void GenericCAO::processMessage(const std::string &data)
updateTextureAnim();
} else if (cmd == AO_CMD_SET_PHYSICS_OVERRIDE) {
float override_speed = readF32(is);
float override_jump = readF32(is);
float override_gravity = readF32(is);
PlayerPhysicsOverride phys; // defaults defined by ctor
phys.speed = readF32(is);
phys.jump = readF32(is);
phys.gravity = readF32(is);
// MT 0.4.10 legacy: send inverted for detault `true` if the server sends nothing
bool sneak = !readU8(is);
bool sneak_glitch = !readU8(is);
bool new_move = !readU8(is);
phys.sneak = !readU8(is);
phys.sneak_glitch = !readU8(is);
phys.new_move = !readU8(is);
// new overrides since 5.8.0
float override_speed_climb = readF32(is);
float override_speed_crouch = readF32(is);
float override_liquid_fluidity = readF32(is);
float override_liquid_fluidity_smooth = readF32(is);
float override_liquid_sink = readF32(is);
float override_acceleration_default = readF32(is);
float override_acceleration_air = readF32(is);
if (is.eof()) {
override_speed_climb = 1.0f;
override_speed_crouch = 1.0f;
override_liquid_fluidity = 1.0f;
override_liquid_fluidity_smooth = 1.0f;
override_liquid_sink = 1.0f;
override_acceleration_default = 1.0f;
override_acceleration_air = 1.0f;
if (canRead(is)) {
phys.speed_climb = readF32(is);
phys.speed_crouch = readF32(is);
phys.liquid_fluidity = readF32(is);
phys.liquid_fluidity_smooth = readF32(is);
phys.liquid_sink = readF32(is);
phys.acceleration_default = readF32(is);
phys.acceleration_air = readF32(is);
}
// new overrides since 5.9.0
float override_speed_fast = readF32(is);
float override_acceleration_fast = readF32(is);
float override_speed_walk = readF32(is);
if (is.eof()) {
override_speed_fast = 1.0f;
override_acceleration_fast = 1.0f;
override_speed_walk = 1.0f;
if (canRead(is)) {
phys.speed_fast = readF32(is);
phys.acceleration_fast = readF32(is);
phys.speed_walk = readF32(is);
}
if (m_is_local_player) {
auto &phys = m_env->getLocalPlayer()->physics_override;
phys.speed = override_speed;
phys.jump = override_jump;
phys.gravity = override_gravity;
phys.sneak = sneak;
phys.sneak_glitch = sneak_glitch;
phys.new_move = new_move;
phys.speed_climb = override_speed_climb;
phys.speed_crouch = override_speed_crouch;
phys.liquid_fluidity = override_liquid_fluidity;
phys.liquid_fluidity_smooth = override_liquid_fluidity_smooth;
phys.liquid_sink = override_liquid_sink;
phys.acceleration_default = override_acceleration_default;
phys.acceleration_air = override_acceleration_air;
phys.speed_fast = override_speed_fast;
phys.acceleration_fast = override_acceleration_fast;
phys.speed_walk = override_speed_walk;
m_env->getLocalPlayer()->physics_override = phys;
}
} else if (cmd == AO_CMD_SET_ANIMATION) {
v2f range = readV2F32(is);
@@ -1699,13 +1676,15 @@ void GenericCAO::processMessage(const std::string &data)
// Read new values
props.position.vector = readV3F32(is);
props.rotation.next = core::quaternion(readV3F32(is) * core::DEGTORAD);
props.scale.vector = readV3F32(is); // reads past end of string on older cmds
if (is.eof()) {
// Backwards compatibility
props.scale.vector = v3f(1, 1, 1); // restore the scale which was not sent
if (!canRead(is)) {
// For PROTOCOL_VERSION < 44
// scale.vector : default
props.position.absolute = true;
props.rotation.absolute = true;
} else {
// For PROTOCOL_VERSION >= 44
props.scale.vector = readV3F32(is);
props.position.interp_duration = readF32(is);
props.rotation.interp_duration = readF32(is);
props.scale.interp_duration = readF32(is);
@@ -1720,7 +1699,11 @@ void GenericCAO::processMessage(const std::string &data)
std::string bone = deSerializeString16(is);
v3f position = readV3F32(is);
v3f rotation = readV3F32(is);
bool force_visible = readU8(is); // Returns false for EOF
bool force_visible = false;
if (canRead(is)) {
// >= 5.4.0-dev
force_visible = readU8(is);
}
setAttachment(parent_id, bone, position, rotation, force_visible);
} else if (cmd == AO_CMD_PUNCHED) {

View File

@@ -62,20 +62,14 @@ void TouchInteraction::serialize(std::ostream &os) const
void TouchInteraction::deSerialize(std::istream &is)
{
u8 tmp = readU8(is);
if (is.eof())
throw SerializationError("");
if (tmp < TouchInteractionMode_END)
pointed_nothing = (TouchInteractionMode)tmp;
tmp = readU8(is);
if (is.eof())
throw SerializationError("");
if (tmp < TouchInteractionMode_END)
pointed_node = (TouchInteractionMode)tmp;
tmp = readU8(is);
if (is.eof())
throw SerializationError("");
if (tmp < TouchInteractionMode_END)
pointed_object = (TouchInteractionMode)tmp;
}
@@ -325,9 +319,11 @@ void ItemDefinition::deSerialize(std::istream &is, u16 protocol_version)
inventory_overlay .deSerialize(is, protocol_version);
wield_overlay.deSerialize(is, protocol_version);
// If you add anything here, insert it inside the try-catch
// block to not need to increase the version.
try {
do {
if (!canRead(is))
break;
// >= 5.4.0-dev
short_description = deSerializeString16(is);
if (protocol_version <= 43) {
@@ -340,13 +336,18 @@ void ItemDefinition::deSerialize(std::istream &is, u16 protocol_version)
sound_use.deSerializeSimple(is, protocol_version);
sound_use_air.deSerializeSimple(is, protocol_version);
if (is.eof())
throw SerializationError("");
if (!canRead(is))
break;
// >= 5.8.0-dev
if (readU8(is)) // protocol_version >= 43
if (readU8(is)) // "have param2"
place_param2 = readU8(is);
wallmounted_rotate_vertical = readU8(is); // 0 if missing
if (!canRead(is))
break;
// >= 5.9.0-dev
wallmounted_rotate_vertical = readU8(is);
touch_interaction.deSerialize(is);
std::string pointabilities_s = deSerializeString16(is);
@@ -356,10 +357,13 @@ void ItemDefinition::deSerialize(std::istream &is, u16 protocol_version)
pointabilities->deSerialize(tmp_is);
}
if (readU8(is)) {
if (readU8(is)) // "have wear bar params"
wear_bar_params = WearBarParams::deserialize(is);
}
} catch(SerializationError &e) {};
//if (!canRead(is))
// break;
// Add new code here
} while (0);
}

View File

@@ -651,16 +651,11 @@ void MapBlock::deSerialize(std::istream &in_compressed, u8 version, bool disk)
void MapBlock::deSerializeNetworkSpecific(std::istream &is)
{
try {
readU8(is);
//const u8 version = readU8(is);
//if (version != 1)
//throw SerializationError("unsupported MapBlock version");
(void)readU8(is); // version
} catch(SerializationError &e) {
warningstream<<"MapBlock::deSerializeNetworkSpecific(): Ignoring an error"
<<": "<<e.what()<<std::endl;
}
//if (version < 3)
// return;
// Add new code here
}
bool MapBlock::storeActiveObject(u16 id)

View File

@@ -465,20 +465,12 @@ void Client::handleCommand_ActiveObjectMessages(NetworkPacket* pkt)
std::string datastring(pkt->getString(0), pkt->getSize());
std::istringstream is(datastring, std::ios_base::binary);
try {
while (is.good()) {
u16 id = readU16(is);
if (!is.good())
break;
while (canRead(is)) {
u16 id = readU16(is);
std::string message = deSerializeString16(is);
std::string message = deSerializeString16(is);
// Pass on to the environment
m_env.processActiveObjectMessage(id, message);
}
} catch (SerializationError &e) {
errorstream << "Client::handleCommand_ActiveObjectMessages: "
<< "caught SerializationError: " << e.what() << std::endl;
// Pass on to the environment
m_env.processActiveObjectMessage(id, message);
}
}
@@ -989,7 +981,7 @@ void Client::handleCommand_SpawnParticleBatch(NetworkPacket *pkt)
decompressZstd(compressed, particle_batch_data);
}
while (particle_batch_data.peek() != EOF) {
while (canRead(particle_batch_data)) {
auto p = std::make_unique<ParticleParameters>();
{
std::istringstream particle_data(deSerializeString32(particle_batch_data), std::ios::binary);
@@ -1049,25 +1041,24 @@ void Client::handleCommand_AddParticleSpawner(NetworkPacket* pkt)
p.glow = readU8(is);
p.object_collision = readU8(is);
// This is kinda awful
do {
u16 tmp_param0 = readU16(is);
if (is.eof())
if (!canRead(is))
break;
p.node.param0 = tmp_param0;
// >= 5.3.0-dev
p.node.param0 = readU16(is);;
p.node.param2 = readU8(is);
p.node_tile = readU8(is);
if (m_proto_ver < 42) {
// v >= 5.6.0
f32 tmp_sbias = readF32(is);
if (is.eof())
if (!canRead(is))
break;
// initial bias must be stored separately in the stream to preserve
// backwards compatibility with older clients, which do not support
// a bias field in their range "format"
p.pos.start.bias = tmp_sbias;
p.pos.start.bias = readF32(is);
p.vel.start.bias = readF32(is);
p.acc.start.bias = readF32(is);
p.exptime.start.bias = readF32(is);
@@ -1112,6 +1103,10 @@ void Client::handleCommand_AddParticleSpawner(NetworkPacket* pkt)
newtex.deSerialize(is, m_proto_ver);
p.texpool.push_back(newtex);
}
//if (!canRead(is))
// break;
// Add new code here
} while(0);
if (missing_end_values) {

View File

@@ -563,10 +563,12 @@ void ContentFeatures::deSerialize(std::istream &is, u16 protocol_version)
palette_name = deSerializeString16(is);
waving = readU8(is);
connect_sides = readU8(is);
u16 connects_to_size = readU16(is);
connects_to_ids.clear();
for (u16 i = 0; i < connects_to_size; i++)
connects_to_ids.push_back(readU16(is));
{
connects_to_ids.clear();
u16 connects_to_size = readU16(is);
for (u16 i = 0; i < connects_to_size; i++)
connects_to_ids.push_back(readU16(is));
}
post_effect_color = readARGB8(is);
leveled = readU8(is);
@@ -616,36 +618,40 @@ void ContentFeatures::deSerialize(std::istream &is, u16 protocol_version)
legacy_facedir_simple = readU8(is);
legacy_wallmounted = readU8(is);
try {
do {
node_dig_prediction = deSerializeString16(is);
u8 tmp = readU8(is);
if (is.eof()) /* readU8 doesn't throw exceptions so we have to do this */
throw SerializationError("");
leveled_max = tmp;
if (!canRead(is))
break;
// >= 5.3.0-dev
tmp = readU8(is);
if (is.eof())
throw SerializationError("");
alpha = static_cast<enum AlphaMode>(tmp);
leveled_max = readU8(is);
if (!canRead(is))
break;
// >= 5.4.0-dev
alpha = static_cast<enum AlphaMode>(readU8(is));
if (alpha >= AlphaMode_END || alpha == ALPHAMODE_LEGACY_COMPAT)
alpha = ALPHAMODE_OPAQUE;
tmp = readU8(is);
if (is.eof())
throw SerializationError("");
move_resistance = tmp;
if (!canRead(is))
break;
// >= 5.5.0-dev
tmp = readU8(is);
if (is.eof())
throw SerializationError("");
liquid_move_physics = tmp;
move_resistance = readU8(is);
liquid_move_physics = readU8(is);
tmp = readU8(is);
if (is.eof())
throw SerializationError("");
post_effect_color_shaded = tmp;
} catch (SerializationError &e) {};
if (!canRead(is))
break;
// >= 5.8.0-dev
post_effect_color_shaded = readU8(is);
//if (!canRead(is))
// break;
// Add new code here
} while (0);
}
/*

View File

@@ -221,25 +221,6 @@ void ObjectProperties::serialize(std::ostream &os) const
// Never remove anything, because we don't want new versions of this!
}
namespace {
// Type-safe wrapper for bools as u8
inline bool readBool(std::istream &is)
{
return readU8(is) != 0;
}
// Wrapper for primitive reading functions that don't throw (awful)
template <typename T, T (reader)(std::istream& is)>
bool tryRead(T& val, std::istream& is)
{
T tmp = reader(is);
if (is.eof())
return false;
val = tmp;
return true;
}
}
void ObjectProperties::deSerialize(std::istream &is)
{
int version = readU8(is);
@@ -295,40 +276,50 @@ void ObjectProperties::deSerialize(std::istream &is)
zoom_fov = readF32(is);
use_texture_alpha = readU8(is);
try {
damage_texture_modifier = deSerializeString16(is);
} catch (SerializationError &e) {
if (!canRead(is))
return;
}
// >= 5.3.0-dev
if (!tryRead<bool, readBool>(shaded, is))
return;
if (!tryRead<bool, readBool>(show_on_minimap, is))
damage_texture_modifier = deSerializeString16(is);
shaded = readU8(is);
if (!canRead(is))
return;
// >= 5.4.0-dev
show_on_minimap = readU8(is);
auto bgcolor = readARGB8(is);
if (bgcolor != NULL_BGCOLOR)
nametag_bgcolor = bgcolor;
else
nametag_bgcolor = std::nullopt;
if (!tryRead<bool, readBool>(rotate_selectionbox, is))
if (!canRead(is))
return;
// >= 5.7.0-dev
if (!tryRead<content_t, readU16>(node.param0, is))
rotate_selectionbox = readU8(is);
if (!canRead(is))
return;
// >= 5.12.0-dev
node.param0 = readU16(is);
node.param1 = readU8(is);
node.param2 = readU8(is);
u32 fontsize;
if (!tryRead<u32, readU32>(fontsize, is))
if (!canRead(is))
return;
// >= 5.14.0-dev
const u32 fontsize = readU32(is);
if (fontsize != U32_MAX)
nametag_fontsize = fontsize;
else
nametag_fontsize = std::nullopt;
nametag_scale_z = readU8(is);
// Add new properties down here and remember to use either tryRead<> or a try-catch.
//if (!canRead(is))
// return;
// Add new code here
}

View File

@@ -212,10 +212,6 @@ void ServerParticleTexture::deSerialize(std::istream &is, u16 protocol_ver,
{
FlagT flags = 0;
deSerializeParameterValue(is, flags);
// new texture properties were missing in ParticleParameters::serialize
// before Minetest 5.9.0
if (is.eof())
return;
animated = !!(flags & FlagT(ParticleTextureFlags::animated));
blendmode = BlendMode((flags & FlagT(ParticleTextureFlags::blend)) >> 1);
@@ -254,17 +250,6 @@ void ParticleParameters::serialize(std::ostream &os, u16 protocol_ver) const
texture.serialize(os, protocol_ver, true, true);
}
template <typename T, T (reader)(std::istream& is)>
inline bool streamEndsBeforeParam(T& val, std::istream& is)
{
// This is kinda awful
T tmp = reader(is);
if (is.eof())
return true;
val = tmp;
return false;
}
void ParticleParameters::deSerialize(std::istream &is, u16 protocol_ver)
{
pos = readV3F32(is);
@@ -280,14 +265,25 @@ void ParticleParameters::deSerialize(std::istream &is, u16 protocol_ver)
glow = readU8(is);
object_collision = readU8(is);
if (streamEndsBeforeParam<u16, readU16>(node.param0, is))
if (!canRead(is))
return;
// >= 5.3.0-dev
node.param0 = readU16(is);
node.param2 = readU8(is);
node_tile = readU8(is);
if (streamEndsBeforeParam<v3f, readV3F32>(drag, is))
if (!canRead(is))
return;
// >= 5.6.0-dev
drag = readV3F32(is);
jitter.deSerialize(is);
bounce.deSerialize(is);
if (!canRead(is))
return;
// >= 5.9.0-dev
texture.deSerialize(is, protocol_ver, true, true);
}

View File

@@ -25,7 +25,8 @@ LuaEntitySAO::LuaEntitySAO(ServerEnvironment *env, v3f pos, const std::string &d
while (!data.empty()) { // breakable, run for one iteration
std::istringstream is(data, std::ios::binary);
// 'version' does not allow to incrementally extend the parameter list thus
// Servers < 5.0.0-dev (PROTOCOL_VERSION < 37) had improper compatibility code,
// only handling exactly 'version=0' and 'version=1'. See commit 67049eba. Thus,
// we need another variable to build on top of 'version=1'. Ugly hack but works™
u8 version2 = 0;
u8 version = readU8(is);
@@ -41,7 +42,7 @@ LuaEntitySAO::LuaEntitySAO(ServerEnvironment *env, v3f pos, const std::string &d
// yaw must be yaw to be backwards-compatible
rotation.Y = readF1000(is);
if (is.good()) // EOF for old formats
if (canRead(is))
version2 = readU8(is);
if (version2 < 1) // PROTOCOL_VERSION < 37

View File

@@ -96,6 +96,23 @@ void TestSerialization::testDeSerializeString()
UASSERT(is.eof());
}
// Exception test of primitive reader functions
{
std::istringstream is(mkstr("\x00\x01\x02\x03"), std::ios::binary);
UASSERT(readU16(is));
UASSERT(canRead(is));
UASSERT(readU16(is));
// Yet no EOF set, thus use `canRead`.
UASSERT(is.good());
UASSERT(!is.eof());
UASSERT(!canRead(is));
// Out of bounds
EXCEPTION_CHECK(SerializationError, readU8(is));
UASSERT(is.eof());
UASSERT(!canRead(is));
}
// Test deserialize an incomplete length specifier
{
std::istringstream is(mkstr("\x53"), std::ios::binary);

View File

@@ -92,7 +92,7 @@ void AreaStore::deserialize(std::istream &is)
areas.emplace_back(std::move(a));
}
bool read_ids = is.good(); // EOF for old formats
const bool read_ids = canRead(is); // since 5.1.0-dev
for (auto &area : areas) {
if (read_ids)

View File

@@ -377,11 +377,25 @@ inline void writeV3F32(u8 *data, v3f p)
//// Iostream wrapper for data read/write
////
inline bool canRead(std::istream &is)
{
return is.peek() != EOF;
}
// Assuming -O3 on GCC 14.2.0, this function results in 55% less machine code
// generated for the `is.eof()` branch in `MAKE_STREAM_READ_FXN`.
static void serialize_throw_eof()
{
throw SerializationError("EOF");
}
#define MAKE_STREAM_READ_FXN(T, N, S) \
inline T read ## N(std::istream &is) \
{ \
char buf[S] = {0}; \
is.read(buf, sizeof(buf)); \
if (is.eof()) \
serialize_throw_eof(); \
return read ## N((u8 *)buf); \
}