Limit and optimize digiline_send (#379)

* Close vulnerability and optimize digiline_send

`digiline_send` as it previously existed was vulnerable to a
time-of-check-to-time-of-use vulnerability in which a table could be
sent, size-checked, and then modified after the send but before
delivery. This would allow larger tables to be sent. It was also slow
because it called `minetest.serialize`. Fix both of these by
implementing custom message cleanup logic which simultaneously computes
the message’s cost.

* Clean up interaction with Digilines

Use `minetest.global_exists` to avoid an undefined global variable
warning when operating a Luacontroller with Digilines not available. Use
the new `digilines` table in preference to the old `digiline` table.

* Copy received messages

When a Digiline message is received at a Luacontroller, copy it so that
local modifications made by the Luacontroller code will not modify
copies of the table that are being passed to other nodes on the Digiline
network.
This commit is contained in:
Christopher Head 2018-01-13 11:27:01 -08:00 committed by Vitaliy
parent 993fdedd8c
commit 2b096f050d

View File

@ -270,27 +270,108 @@ local function get_interrupt(pos)
end end
-- Given a message object passed to digiline_send, clean it up into a form
-- which is safe to transmit over the network and compute its "cost" (a very
-- rough estimate of its memory usage).
--
-- The cleaning comprises the following:
-- 1. Functions (and userdata, though user scripts ought not to get hold of
-- those in the first place) are removed, because they break the model of
-- Digilines as a network that carries basic data, and they could exfiltrate
-- references to mutable objects from one Luacontroller to another, allowing
-- inappropriate high-bandwidth, no-wires communication.
-- 2. Tables are duplicated because, being mutable, they could otherwise be
-- modified after the send is complete in order to change what data arrives
-- at the recipient, perhaps in violation of the previous cleaning rule or
-- in violation of the message size limit.
--
-- The cost indication is only approximate; its not a perfect measurement of
-- the number of bytes of memory used by the message object.
--
-- Parameters:
-- msg -- the message to clean
-- back_references -- for internal use only; do not provide
--
-- Returns:
-- 1. The cleaned object.
-- 2. The approximate cost of the object.
local function clean_and_weigh_digiline_message(msg, back_references)
local t = type(msg)
if t == "string" then
-- Strings are immutable so can be passed by reference, and cost their
-- length plus the size of the Lua object header (24 bytes on a 64-bit
-- platform) plus one byte for the NUL terminator.
return msg, #msg + 25
elseif t == "number" then
-- Numbers are passed by value so need not be touched, and cost 8 bytes
-- as all numbers in Lua are doubles.
return msg, 8
elseif t == "boolean" then
-- Booleans are passed by value so need not be touched, and cost 1
-- byte.
return msg, 1
elseif t == "table" then
-- Tables are duplicated. Check if this table has been seen before
-- (self-referential or shared table); if so, reuse the cleaned value
-- of the previous occurrence, maintaining table topology and avoiding
-- infinite recursion, and charge zero bytes for this as the object has
-- already been counted.
back_references = back_references or {}
local bref = back_references[msg]
if bref then
return bref, 0
end
-- Construct a new table by cleaning all the keys and values and adding
-- up their costs, plus 8 bytes as a rough estimate of table overhead.
local cost = 8
local ret = {}
back_references[msg] = ret
for k, v in pairs(msg) do
local k_cost, v_cost
k, k_cost = clean_and_weigh_digiline_message(k, back_references)
v, v_cost = clean_and_weigh_digiline_message(v, back_references)
if k ~= nil and v ~= nil then
-- Only include an element if its key and value are of legal
-- types.
ret[k] = v
end
-- If we only counted the cost of a table element when we actually
-- used it, we would be vulnerable to the following attack:
-- 1. Construct a huge table (too large to pass the cost limit).
-- 2. Insert it somewhere in a table, with a function as a key.
-- 3. Insert it somewhere in another table, with a number as a key.
-- 4. The first occurrence doesnt pay the cost because functions
-- are stripped and therefore the element is dropped.
-- 5. The second occurrence doesnt pay the cost because its in
-- back_references.
-- By counting the costs regardless of whether the objects will be
-- included, we avoid this attack; it may overestimate the cost of
-- some messages, but only those that wont be delivered intact
-- anyway because they contain illegal object types.
cost = cost + k_cost + v_cost
end
return ret, cost
else
return nil, 0
end
end
local function get_digiline_send(pos) local function get_digiline_send(pos)
if not digiline then return end if not minetest.global_exists("digilines") then return end
return function(channel, msg) return function(channel, msg)
-- Make sure channel is string, number or boolean -- Make sure channel is string, number or boolean
if (type(channel) ~= "string" and type(channel) ~= "number" and type(channel) ~= "boolean") then if (type(channel) ~= "string" and type(channel) ~= "number" and type(channel) ~= "boolean") then
return false return false
end end
-- It is technically possible to send functions over the wire since local msg_cost
-- the high performance impact of stripping those from the data has msg, msg_cost = clean_and_weigh_digiline_message(msg)
-- been decided to not be worth the added realism. if msg == nil or msg_cost > mesecon.setting("luacontroller_digiline_maxlen", 50000) then
-- Make sure serialized version of the data is not insanely long to
-- prevent DoS-like attacks
local msg_ser = minetest.serialize(msg)
if #msg_ser > mesecon.setting("luacontroller_digiline_maxlen", 50000) then
return false return false
end end
minetest.after(0, function() minetest.after(0, digilines.receptor_send, pos, digilines.rules.default, channel, msg)
digiline:receptor_send(pos, digiline.rules.default, channel, msg)
end)
return true return true
end end
end end
@ -518,6 +599,7 @@ local digiline = {
receptor = {}, receptor = {},
effector = { effector = {
action = function(pos, node, channel, msg) action = function(pos, node, channel, msg)
msg = clean_and_weigh_digiline_message(msg)
run(pos, {type = "digiline", channel = channel, msg = msg}) run(pos, {type = "digiline", channel = channel, msg = msg})
end end
} }