search

mt-mods / beerchat

beerchat mod for minetest
GNU Lesser General Public License v3.0
9 stars 7 forks source link

command block can crash server #121

Open SwissalpS opened 3 months ago

SwissalpS commented 3 months ago

< FeXoR@pandorabox> Triggering "lc [channel]" with a command block owned by an offline player crashes the server if triggered by another player (not verbatum quote)

Not something that should happen often. Maybe checking if player object actually exists (player is online) on chatcommands would be a straightforward way to harden against such situations.

S-S-X commented 3 months ago

Was probably this one:

2024-07-31 02:53:05: ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod '??' in callback environment_Step(): /data/world//worldmods/beerchat/chatcommands.lua:184: attempt to index a nil value
2024-07-31 02:53:05: ERROR[Main]: stack traceback:
2024-07-31 02:53:05: ERROR[Main]:   /data/world//worldmods/beerchat/chatcommands.lua:184: in function 'func'
2024-07-31 02:53:05: ERROR[Main]:   ...world//worldmods/mesecons/mesecons_commandblock/init.lua:167: in function 'action_on'
2024-07-31 02:53:05: ERROR[Main]:   /data/world//worldmods/mesecons/mesecons/internal.lua:184: in function </data/world//worldmods/mesecons/mesecons/internal.lua:177>
2024-07-31 02:53:05: ERROR[Main]:   /data/world//worldmods/mesecons/mesecons/actionqueue.lua:137: in function 'f'
2024-07-31 02:53:05: ERROR[Main]:   /data/world//worldmods/monitoring/metrictypes/counter.lua:43: in function 'old_execute'
2024-07-31 02:53:05: ERROR[Main]:   ...d//worldmods/mesecons_debug/overrides/mesecons_queue.lua:27: in function 'execute'
2024-07-31 02:53:05: ERROR[Main]:   /data/world//worldmods/mesecons/mesecons/actionqueue.lua:111: in function 'globalstep'
2024-07-31 02:53:05: ERROR[Main]:   /data/world//worldmods/monitoring/builtin/globalstep.lua:73: in function </data/world//worldmods/monitoring/builtin/globalstep.lua:55>
2024-07-31 02:53:05: ERROR[Main]:   /usr/local/share/minetest/builtin/common/register.lua:26: in function </usr/local/share/minetest/builtin/common/register.lua:12>
S-S-X commented 3 months ago

Line 184 seems like straightforward easy fix https://github.com/mt-mods/beerchat/blob/4b146ccc02eb3d7ea8a100722163d45312b14dbd/chatcommands.lua#L184-L186

However I'm pretty sure there's a lot of similar stuff where it is simply just assumed that player who executed the command is online.

I think long ago it was already determined that many commands would crash the server with similar reason if executed remotely, back then remote execution was just disabled. Could extend Beerchat API a bit and add these common internal lookups there and fix these corner cases while doing it.