Skip to content

Commit

Permalink
Pathfinder: only load and store paths with good length
Browse files Browse the repository at this point in the history
This avoids buffer overruns on loading and saving of games.
This should fix some crashes.

See Wargus#610

It fixes e.g. this valgrind warning (at loading):

==24878== Invalid write of size 1
==24878==    at 0x3DBC63: PathFinderOutput::Load(lua_State*) (script_unit.cpp:261)
==24878==    by 0x3DD5D5: CclUnit(lua_State*) (script_unit.cpp:500)
==24878==    by 0x48A9F4D: luaD_precall (ldo.c:320)
==24878==    by 0x48BBBB3: luaV_execute (lvm.c:591)
==24878==    by 0x48AA5FC: luaD_call (ldo.c:378)
==24878==    by 0x48A98EA: luaD_rawrunprotected (ldo.c:116)
==24878==    by 0x48AA79C: luaD_pcall (ldo.c:464)
==24878==    by 0x48A1E67: lua_pcall (lapi.c:821)
==24878==    by 0x390DDB: LuaCall(lua_State*, int, int, int, bool) (script.cpp:200)
==24878==    by 0x390D69: LuaCall(int, int, bool) (script.cpp:172)
==24878==    by 0x3914BB: LuaLoadFile(std::filesystem::__cxx11::path const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) (script.cpp:271)
==24878==    by 0x2B1946: LoadGame(std::filesystem::__cxx11::path const&) (loadgame.cpp:204)
==24878==  Address 0x10819890 is 0 bytes after a block of size 64 alloc'd
==24878==    at 0x483EEDD: operator new(unsigned long) (vg_replace_malloc.c:485)
==24878==    by 0x1D528D: std::__detail::_MakeUniq<PathFinderData>::__single_object std::make_unique<PathFinderData>() (unique_ptr.h:1070)
==24878==    by 0x1CAC05: CUnit::Init() (unit.cpp:425)
==24878==    by 0x1EA254: CUnit::CUnit() (unit.h:142)
==24878==    by 0x1E9AEE: CUnitManager::Load(lua_State*) (unit_manager.cpp:180)
==24878==    by 0x3E287A: CclSlotUsage(lua_State*) (script_unit.cpp:1486)
==24878==    by 0x48A9F4D: luaD_precall (ldo.c:320)
==24878==    by 0x48BBBB3: luaV_execute (lvm.c:591)
==24878==    by 0x48AA5FC: luaD_call (ldo.c:378)
==24878==    by 0x48A98EA: luaD_rawrunprotected (ldo.c:116)
==24878==    by 0x48AA79C: luaD_pcall (ldo.c:464)
==24878==    by 0x48A1E67: lua_pcall (lapi.c:821)
  • Loading branch information
zzam committed Feb 26, 2024
1 parent 737be8f commit 6565d2d
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
9 changes: 6 additions & 3 deletions src/unit/script_unit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,13 @@ void PathFinderOutput::Load(lua_State *l)
LuaError(l, "incorrect argument _");
}
const int subargs = lua_rawlen(l, -1);
for (int k = 0; k < subargs; ++k) {
this->Path[k] = LuaToNumber(l, -1, k + 1);
if (subargs <= PathFinderOutput::MAX_PATH_LENGTH)
{
for (int k = 0; k < subargs; ++k) {
this->Path[k] = LuaToNumber(l, -1, k + 1);
}
this->Length = subargs;
}
this->Length = subargs;
lua_pop(l, 1);
} else {
LuaError(l, "PathFinderOutput::Load: Unsupported tag: %s", tag.data());
Expand Down
2 changes: 1 addition & 1 deletion src/unit/unit_save.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ void PathFinderOutput::Save(CFile &file) const
if (this->OverflowLength) {
file.printf("\"overflow-length\", %d, ", this->OverflowLength);
}
if (this->Length > 0) {
if (this->Length > 0 && this->Length <= PathFinderOutput::MAX_PATH_LENGTH) {
file.printf("\"path\", {");
for (int i = 0; i < this->Length; ++i) {
file.printf("%d, ", this->Path[i]);
Expand Down

0 comments on commit 6565d2d

Please sign in to comment.