Pathfinder: Fix out-of-bounds read/write at load/store of games #611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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)
zzam
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix memory issues at save and load.
Saving a too long path stores no path at all (=an empty path). Loading a too long path also sets it to empty (to be compatible to old saved files).
I hope this is good.
It also fixes the value of Length at runtime.
Resolves #610
It might resolve some of the open crash-issues.