LandSandBoat / server

:sailboat: LandSandBoat - a server emulator for Final Fantasy XI
https://landsandboat.github.io/server/
GNU General Public License v3.0
289 stars 574 forks source link

Under certain conditions, executing non existent commands will crash map server. #1516

Closed CatsEyeXI closed 2 years ago

CatsEyeXI commented 2 years ago

Additional Information (Steps to reproduce/Expected behavior) : As subject states, under some undetermined conditions, non existent commands can be used to intentionally crash the server.

In this first example, it was msvcrt.dll that crashed, which i realize is outside the project... but in many cases there is no dump generated at all...

[04/06/22 16:11:02:714][map][info][info] parse: 0B5 | 02D7 02D6 08 from user: Shuran (parse:632)
[04/06/22 16:11:02:714][map][error][error] cmdhandler::call: (fillmode): cannot open ./scripts/commands/fillmode.lua: No such file or directory (CCommandHandler::call:64)
=====================================================
!!! CRASH !!!
Exception code: C0000005 (ACCESS_VIOLATION)
Fault address: 00007FFC26E88CA9 01:0000000000027CA9
Process Name: C:\Windows\System32\msvcrt.dll
Full crash report: C:\catseyexi\dmp\topaz_game_64.exe_6-4_16-11-2.log
Memory dump: C:\catseyexi\dmp\topaz_game_64.exe_6-4_16-11-2.dmp
CatsEyeXI commented 2 years ago

in this second example, on dump was automatically generated...

[04/06/22 21:36:23:164][map][info][info] parse: 0B5 | 82B7 82AE 06 from user: Eons (parse:632)
[04/06/22 21:36:23:164][map][error][error] cmdhandler::call: (home): cannot open ./scripts/commands/home.lua: No such file or directory (CCommandHandler::call:64)
[04/06/22 21:36:24:481][map][info][status] do_init: begin server initialization (do_init:165)
[04/06/22 21:36:24:482][map][info][info] Console Silent Setting: 0 (map_config_read:1173)
[04/06/22 21:36:24:482][map][info][status] do_init: map_config is reading (do_init:198)
[04/06/22 21:36:24:482][map][info][status] luautils::init:lua initializing (luautils::init:113)
[04/06/22 21:36:24:632][map][info][lua] Preparing override: xi.zones.Behemoths_Dominion.mobs.Behemoth.onMobDespawn (moduleutils::LoadLuaModules:92)
[04/06/22 21:36:24:632][map][info][lua] Preparing override: xi.zones.Behemoths_Dominion.Zone.onInitialize (moduleutils::LoadLuaModules:92)
CatsEyeXI commented 2 years ago
[04/06/22 21:41:46:361][map][info][info] parse: 0B5 | 0047 0046 06 from user: Eons (parse:632)
[04/06/22 21:41:46:361][map][error][error] cmdhandler::call: (home): cannot open ./scripts/commands/home.lua: No such file or directory (CCommandHandler::call:64)
[04/06/22 21:41:47:692][map][info][status] do_init: begin server initialization (do_init:165)
[04/06/22 21:41:47:692][map][info][info] Console Silent Setting: 0 (map_config_read:1173)
[04/06/22 21:41:47:692][map][info][status] do_init: map_config is reading (do_init:198)
[04/06/22 21:41:47:692][map][info][status] luautils::init:lua initializing (luautils::init:113)
[04/06/22 21:41:47:831][map][info][lua] Preparing override: xi.zones.Behemoths_Dominion.mobs.Behemoth.onMobDespawn (moduleutils::LoadLuaModules:92)
[04/06/22 21:41:47:831][map][info][lua] Preparing override: xi.zones.Behemoths_Dominion.Zone.onInitialize (moduleutils::LoadLuaModules:92)
[04/06/22 21:41:47:831][map][info][lua] Preparing override: xi.zones.Dragons_Aery.mobs.Fafnir.onMobDespawn (moduleutils::LoadLuaModules:92)
CatsEyeXI commented 2 years ago

crash with no explanation:

[04/08/22 00:09:41:212][map][info][info] parse: 0C0 | 2C13 2C11 02 from user: Kabuza (parse:632)
[04/08/22 00:09:41:447][map][info][info] parse: 01A | 0329 0328 0E from user: Tamami (parse:632)
[04/08/22 00:09:41:450][map][info][action] CLIENT Tamami PERFORMING ACTION 00 (SmallPacket0x01A:1061)
[04/08/22 00:09:41:485][map][info][info] parse: 01A | 8763 873C 0E from user: Supersmol (parse:632)
[04/08/22 00:09:41:485][map][info][action] CLIENT Supersmol PERFORMING ACTION 03 (SmallPacket0x01A:1061)
[04/08/22 08:25:33:527][map][info][status] do_init: begin server initialization (do_init:165)
[04/08/22 08:25:33:537][map][info][info] Console Silent Setting: 0 (map_config_read:1173)
[04/08/22 08:25:33:542][map][info][status] do_init: map_config is reading (do_init:198)
[04/08/22 08:25:33:542][map][info][status] luautils::init:lua initializing (luautils::init:113)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Behemoths_Dominion.mobs.Behemoth.onMobDespawn (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Behemoths_Dominion.Zone.onInitialize (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Dragons_Aery.mobs.Fafnir.onMobDespawn (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Dragons_Aery.Zone.onInitialize (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Valley_of_Sorrows.mobs.Adamantoise.onMobDespawn (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Valley_of_Sorrows.Zone.onInitialize (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Caedarva_Mire.mobs.Khimaira.onMobDespawn (moduleutils::LoadLuaModules:92)
[04/08/22 08:25:34:348][map][info][lua] Preparing override: xi.zones.Caedarva_Mire.Zone.onInitialize (moduleutils::LoadLuaModules:92)
zach2good commented 2 years ago

Can you post your entries for the TOD module? Might be helpful for repro

CatsEyeXI commented 2 years ago

Can you post your entries for the TOD module? Might be helpful for repro

of course...

local nms_to_persist =
{
    { "Behemoths_Dominion", "Behemoth", function() return 75600 + math.random(0, 6) * 1800 end }, -- 21 - 24 hours with half hour windows
    { "Dragons_Aery", "Fafnir", function() return 75600 + math.random(0, 6) * 1800 end }, -- 21 - 24 hours with half hour windows
    { "Valley_of_Sorrows", "Adamantoise", function() return 75600 + math.random(0, 6) * 1800 end }, -- 21 - 24 hours with half hour windows
    { "Caedarva_Mire", "Khimaira", function() return 75600 + math.random(0, 6) * 1800 end }, -- 21 - 24 hours with half hour windows
}
CatsEyeXI commented 2 years ago

I wanted to do my due diligence, so I rebuilt my entire repo to eliminate the possibility of any merge conflicts, updated Visual Studio 2022, installed Windows Updates, reset my database and imported my char tables... Unfortunately the issue persists.

topaz_game_64.exe_12-4_6-2-52.log

zach2good commented 2 years ago

I think that log is the most useful one so far:

00007FF6F648537B  000000D6C5CFC6B0  sol::stack::remove+7B (C:\catseyexi\ext\sol\include\sol\sol.hpp, line 8955)
00007FF6F64878F1  000000D6C5CFCC00  CCommandHandler::call+2341 (C:\catseyexi\src\map\commandhandler.cpp, line 232)

    lua_settop(lua.lua_state(), top);
    return 0;

The command handler is one of the areas where we do a lot of manual lua stack prodding and poking, I'm not surprised that it caused a crash that totally wipes everything out. Needs a rewrite ;_;

Luckily, I think we're all now versed enough in manipulating the Lua stack and using Sol properly that it should be doable

zach2good commented 2 years ago

For my own reference during rewrites:

Can't use std::any, std::variant is a better shout: https://github.com/ThePhD/sol2/issues/1035

https://github.com/ThePhD/sol2/blob/f56b3c698c2116f41aad3bf731ba8400e68c498b/documentation/source/tutorial/functions.rst#any-return-to-and-from-lua

https://github.com/ThePhD/sol2/blob/7aae1aaaaa1bbcc03fd059fe38075cfe3f4b2e90/tests/run_time/source/lua_value.cpp

https://github.com/ThePhD/sol2/blob/f56b3c698c2116f41aad3bf731ba8400e68c498b/tests/runtime_tests/source/utility.cpp

https://sol2.readthedocs.io/en/latest/api/as_args.html

CatsEyeXI commented 2 years ago

I think that log is the most useful one so far:

00007FF6F648537B  000000D6C5CFC6B0  sol::stack::remove+7B (C:\catseyexi\ext\sol\include\sol\sol.hpp, line 8955)
00007FF6F64878F1  000000D6C5CFCC00  CCommandHandler::call+2341 (C:\catseyexi\src\map\commandhandler.cpp, line 232)
    lua_settop(lua.lua_state(), top);
    return 0;

The command handler is one of the areas where we do a lot of manual lua stack prodding and poking, I'm not surprised that it caused a crash that totally wipes everything out. Needs a rewrite ;_;

Luckily, I think we're all now versed enough in manipulating the Lua stack and using Sol properly that it should be doable

I'm relieved that there's at least something useful here... thanks for looking

zach2good commented 2 years ago

Reopening until it has been running clean for a while

CatsEyeXI commented 2 years ago

initial testing results are positive... will update as the day goes on...

CatsEyeXI commented 2 years ago

thank you zach for the quick fix... appreciate you!