minetest-mods / mesecons

Mod for Minetest that adds digital circuitry [=Minecraft redstone]
https://mesecons.net
Other
211 stars 121 forks source link

LuaController serializer abuse. #415

Open beyondlimits opened 6 years ago

beyondlimits commented 6 years ago

It is possible to extremely slow down server via forcing serializer to serialize a large fractal of nested tables, either in steps (my computer slows down after 24th):

interrupt(1)
mem.x = {mem.x, mem.x}

or bursting everything at once

for x = 1, 30 do
  o = {o, o}
end
mem.x = o
numberZero commented 6 years ago

@beyondlimits That particular bug can and should be fixed, but the core problem is unfixable. See https://github.com/minetest-mods/mesecons/issues/380#issuecomment-339689832 for some details; Lua just doesn’t offer enough sandboxing features.

numberZero commented 6 years ago

Possible solution: run the serializer under timeout.

TheEt1234 commented 8 months ago

not allowing nested tables would solve this most likely

edit: past me meant to say those t = {} t.t = t tables which i have no idea how to detect not allowing nested tables (as in t = {} t.x = {} would be a super breaking change)

fluxionary commented 7 months ago

actually, improvements in minetest's serializer mean this isn't so easy to exploit currently. the following code gets up to 200 and beyond without any noticeable slowdown:

if event.type == "program" or event.type == "interrupt" then
 if pin.a then
  mem.i = (mem.i or 0) + 1
  digiline_send("LCD", tostring(mem.i))
  local o = mem.o or {}
  mem.o = {o, o}
 end

 interrupt(1)
end

trying to double the size of string instead of a table results in overheating after 16 iterations.

TheEt1234 commented 7 months ago

oh cool, so it's basically fixed in the latest minetest version (?)

fluxionary commented 7 months ago

oh cool, so it's basically fixed in the latest minetest version (?)

seems to be. possibly there's some way to exploit this, but i everything i can think of leads to overheating before major problems.

Niklp09 commented 7 months ago

No, this is not fixed. It's still possible to create major lag spikes and a friend and I were even able to trigger an OOM server process kill on a 128GB RAM machine. (I won't share the code here for obvious reasons)

TheEt1234 commented 7 months ago

not allowing nested tables would solve this most likely

edit: past me meant to say those t = {} t.t = t tables which i have no idea how to detect not allowing nested tables (as in t = {} t.x = {} would be a super breaking change)

How could this be fixed without it being a breaking change

TheEt1234 commented 7 months ago

(ok i did not expect github to mention this across those issues.. why does it do that, well the above is somewhat unrelated to the issue)