-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pathfinder: Bad handling of failure results #610
Comments
zzam
added a commit
to zzam/stratagus
that referenced
this issue
Feb 25, 2024
This avoids buffer overruns on load and save of games. This should fix some crashes. See Wargus#610
The load/store is fixed with #611 |
zzam
added a commit
to zzam/stratagus
that referenced
this issue
Feb 26, 2024
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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Negative results of AStarFindPath
lead to function NewPath storing negative values be stored in PathFinderOutput::Length
output.Length = std::min<int>(i, PathFinderOutput::MAX_PATH_LENGTH);
but PathFinderOutput::Length being uint8_t. So 253, 254 or 255 are stored.
This itself is only harmful where the values are used:
Saved games contain e.g. these bad pathes:
"pathfinder-output", {"path", {6, 7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 88, -12, -4, -1, 56, -76, -4, -1, 24, 96, -4, -1, 0, 0, -4, -1, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 0, 0, 0, -63, 0, 0, 0, 0, 0, 0, 0, 80, -1, 77, 16, 0, 0, 0, 0, 32, -32, -12, -1, 0, 0, -4, -1, 0, 0, -80, -1, -4, -108, 0, -1, 97, 45, 102, 97, 115, 116, 45, 102, 97, 115, 116, 45, 102, 101, 114, 116, 105, 103, 46, 115, 97, 118, 46, 103, 122, 0, 92, -1, 0, 4, 68, -1, -52, 72, 12, -1, -96, 40, 4, -1, 116, 20, 0, -1, 76, 4, 0, -1, 0, 0, 0, 0, 0, 0, 0, 0, 97, 0, 0, 0, 0, 0, 0, 0, 97, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 20, -116, -8, -1, 16, 96, -56, -1, 16, 60, -104, -1, 12, 32, 108, -1, 12, 72, -52, -1, 4, 40, -96, -1, 0, 20, 116, -1, 0, 4, 76, -1, -64, 62, -92, 16, 0, 0, 0, 0, 16, -81, -103, 15, 0, 0, 0, 0, 20, 64, 104, -1, 32, 88, 124, -1, 48, 112, -112, -1, 0, 0, 0, -1, -80, -59, -24, 15, 0, 0, 0, 0, -63, 0, 0, 0, 0, 0, 0, 0, 80, 0, 0, },"cycles", 32},
The text was updated successfully, but these errors were encountered: