From dfd5726eb097a6831710e643c2276aaeb3cd62ff Mon Sep 17 00:00:00 2001 From: "alexey.lysiuk" Date: Sat, 22 Aug 2020 11:13:38 +0300 Subject: [PATCH] - added bounds checks for serialization of pointers Bogus pointers and offsets were serialized silently This will to find other cases of dangling pointers, and protect from loading of broken saved games https://forum.zdoom.org/viewtopic.php?t=67494 --- src/common/engine/serializer_internal.h | 26 ++++++++++++++++++++--- src/serializer_doom.cpp | 28 +++++++------------------ 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/common/engine/serializer_internal.h b/src/common/engine/serializer_internal.h index 7323f23f7..fb1402ab7 100644 --- a/src/common/engine/serializer_internal.h +++ b/src/common/engine/serializer_internal.h @@ -226,15 +226,35 @@ struct FReader //========================================================================== template -FSerializer &SerializePointer(FSerializer &arc, const char *key, T *&value, T **defval, T *base) +FSerializer &SerializePointer(FSerializer &arc, const char *key, T *&value, T **defval, T *base, const int64_t count) { assert(base != nullptr); + assert(count > 0); if (arc.isReading() || !arc.w->inObject() || defval == nullptr || value != *defval) { - int64_t vv = value == nullptr ? -1 : value - base; + int64_t vv = -1; + if (value != nullptr) + { + vv = value - base; + if (vv < 0 || vv >= count) + I_Error("Trying to serialize out-of-bounds array value"); + } Serialize(arc, key, vv, nullptr); - value = vv < 0 ? nullptr : base + vv; + if (vv == -1) + value = nullptr; + else if (vv < 0 || vv >= count) + I_Error("Trying to serialize out-of-bounds array value"); + else + value = base + vv; } return arc; } +template +FSerializer &SerializePointer(FSerializer &arc, const char *key, T *&value, T **defval, TArray &array) +{ + if (array.Size() == 0) + I_Error("Trying to serialize a value from empty array"); + return SerializePointer(arc, key, value, defval, array.Data(), array.Size()); +} + diff --git a/src/serializer_doom.cpp b/src/serializer_doom.cpp index 27ab87d60..d94eb5d22 100644 --- a/src/serializer_doom.cpp +++ b/src/serializer_doom.cpp @@ -231,54 +231,40 @@ template<> FSerializer &Serialize(FSerializer &ar, const char *key, FPolyObj *&v { auto arc = dynamic_cast(&ar); if (!arc || arc->Level == nullptr) I_Error("Trying to serialize polyobject without a valid level"); - return SerializePointer(*arc, key, value, defval, arc->Level->Polyobjects.Data()); -} - -template<> FSerializer &Serialize(FSerializer &ar, const char *key, const FPolyObj *&value, const FPolyObj **defval) -{ - auto arc = dynamic_cast(&ar); - if (!arc || arc->Level == nullptr) I_Error("Trying to serialize polyobject without a valid level"); - return SerializePointer(*arc, key, value, defval, arc->Level->Polyobjects.Data()); + return SerializePointer(*arc, key, value, defval, arc->Level->Polyobjects); } template<> FSerializer &Serialize(FSerializer &ar, const char *key, side_t *&value, side_t **defval) { auto arc = dynamic_cast(&ar); if (!arc || arc->Level == nullptr) I_Error("Trying to serialize SIDEDEF without a valid level"); - return SerializePointer(ar, key, value, defval, arc->Level->sides.Data()); + return SerializePointer(*arc, key, value, defval, arc->Level->sides); } template<> FSerializer &Serialize(FSerializer &ar, const char *key, sector_t *&value, sector_t **defval) { auto arc = dynamic_cast(&ar); if (!arc || arc->Level == nullptr) I_Error("Trying to serialize sector without a valid level"); - return SerializePointer(*arc, key, value, defval, arc->Level->sectors.Data()); -} - -template<> FSerializer &Serialize(FSerializer &ar, const char *key, const sector_t *&value, const sector_t **defval) -{ - auto arc = dynamic_cast(&ar); - if (!arc || arc->Level == nullptr) I_Error("Trying to serialize sector without a valid level"); - return SerializePointer(*arc, key, value, defval, arc->Level->sectors.Data()); + return SerializePointer(*arc, key, value, defval, arc->Level->sectors); } template<> FSerializer &Serialize(FSerializer &arc, const char *key, player_t *&value, player_t **defval) { - return SerializePointer(arc, key, value, defval, players); + return SerializePointer(arc, key, value, defval, players, MAXPLAYERS); } template<> FSerializer &Serialize(FSerializer &ar, const char *key, line_t *&value, line_t **defval) { auto arc = dynamic_cast(&ar); if (!arc || arc->Level == nullptr) I_Error("Trying to serialize linedef without a valid level"); - return SerializePointer(*arc, key, value, defval, arc->Level->lines.Data()); + return SerializePointer(*arc, key, value, defval, arc->Level->lines); } template<> FSerializer &Serialize(FSerializer &ar, const char *key, vertex_t *&value, vertex_t **defval) { auto arc = dynamic_cast(&ar); - if (!arc || arc->Level == nullptr) I_Error("Trying to serialize vertex without a valid level"); - return SerializePointer(*arc, key, value, defval, arc->Level->vertexes.Data()); + if (!arc || arc->Level == nullptr) I_Error("Trying to serialize verte without a valid level"); + return SerializePointer(*arc, key, value, defval, arc->Level->vertexes); } //==========================================================================