Restrict Lua controller interrupt IDs (#534)

* Deprecate non-string IIDs
* Restrict tabular IIDs to proper trees
Fixes crash on recursive interrupt ID (#473)
This commit is contained in:
Vitaliy 2020-09-21 22:32:25 +03:00 committed by GitHub
parent 3202bf6786
commit 6921909100
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 14 deletions

View File

@ -193,14 +193,23 @@ function mesecon.tablecopy(obj) -- deep copy
return obj return obj
end end
-- Returns whether two values are equal.
-- In tables, keys are compared for identity but values are compared recursively.
-- There is no protection from infinite recursion.
function mesecon.cmpAny(t1, t2) function mesecon.cmpAny(t1, t2)
if type(t1) ~= type(t2) then return false end if type(t1) ~= type(t2) then return false end
if type(t1) ~= "table" and type(t2) ~= "table" then return t1 == t2 end if type(t1) ~= "table" then return t1 == t2 end
-- Check that for each key of `t1` both tables have the same value
for i, e in pairs(t1) do for i, e in pairs(t1) do
if not mesecon.cmpAny(e, t2[i]) then return false end if not mesecon.cmpAny(e, t2[i]) then return false end
end end
-- Check that all keys of `t2` are also keys of `t1` so were checked in the previous loop
for i, _ in pairs(t2) do
if t1[i] == nil then return false end
end
return true return true
end end

View File

@ -266,6 +266,46 @@ local function remove_functions(x)
return x return x
end end
local function validate_iid(iid)
if not iid then return true end -- nil is OK
local limit = mesecon.setting("luacontroller_interruptid_maxlen", 256)
if type(iid) == "string" then
if #iid <= limit then return true end -- string is OK unless too long
return false, "An interrupt ID was too large!"
end
if type(iid) == "number" or type(iid) == "boolean" then return true, "Non-string interrupt IDs are deprecated" end
local warn
local seen = {}
local function check(t)
if type(t) == "function" then
warn = "Functions cannot be used in interrupt IDs"
return false
end
if type(t) ~= "table" then
return true
end
if seen[t] then
warn = "Non-tree-like tables are forbidden as interrupt IDs"
return false
end
seen[t] = true
for k, v in pairs(t) do
if not check(k) then return false end
if not check(v) then return false end
end
return true
end
if not check(iid) then return false, warn end
if #minetest.serialize(iid) > limit then
return false, "An interrupt ID was too large!"
end
return true, "Table interrupt IDs are deprecated and are unreliable; use strings instead"
end
-- The setting affects API so is not intended to be changeable at runtime -- The setting affects API so is not intended to be changeable at runtime
local get_interrupt local get_interrupt
if mesecon.setting("luacontroller_lightweight_interrupts", false) then if mesecon.setting("luacontroller_lightweight_interrupts", false) then
@ -282,26 +322,18 @@ else
-- itbl: Flat table of functions to run after sandbox cleanup, used to prevent various security hazards -- itbl: Flat table of functions to run after sandbox cleanup, used to prevent various security hazards
get_interrupt = function(pos, itbl, send_warning) get_interrupt = function(pos, itbl, send_warning)
-- iid = interrupt id -- iid = interrupt id
local function interrupt(time, iid) return function (time, iid)
-- NOTE: This runs within string metatable sandbox, so don't *rely* on anything of the form (""):y -- NOTE: This runs within string metatable sandbox, so don't *rely* on anything of the form (""):y
-- Hence the values get moved out. Should take less time than original, so totally compatible -- Hence the values get moved out. Should take less time than original, so totally compatible
if type(time) ~= "number" then error("Delay must be a number") end if type(time) ~= "number" then error("Delay must be a number") end
table.insert(itbl, function () table.insert(itbl, function ()
-- Outside string metatable sandbox, can safely run this now -- Outside string metatable sandbox, can safely run this now
local luac_id = minetest.get_meta(pos):get_int("luac_id") local luac_id = minetest.get_meta(pos):get_int("luac_id")
-- Check if IID is dodgy, so you can't use interrupts to store an infinite amount of data. local ok, warn = validate_iid(iid)
-- Note that this is safe from alter-after-free because this code gets run after the sandbox has ended. if ok then mesecon.queue:add_action(pos, "lc_interrupt", {luac_id, iid}, time, iid, 1) end
-- This runs outside of the timer and *shouldn't* harm perf. unless dodgy data is being sent in the first place if warn then send_warning(warn) end
iid = remove_functions(iid)
local msg_ser = minetest.serialize(iid)
if #msg_ser <= mesecon.setting("luacontroller_interruptid_maxlen", 256) then
mesecon.queue:add_action(pos, "lc_interrupt", {luac_id, iid}, time, iid, 1)
else
send_warning("An interrupt ID was too large!")
end
end) end)
end end
return interrupt
end end
end end
@ -901,4 +933,3 @@ minetest.register_craft({
{'group:mesecon_conductor_craftable', 'group:mesecon_conductor_craftable', ''}, {'group:mesecon_conductor_craftable', 'group:mesecon_conductor_craftable', ''},
} }
}) })