Skip to content
Permalink
Browse files

Fix potential security issue(s), documentation on minetest.deserializ…

…e() (#9369)

Also adds an unittest
  • Loading branch information
sfan5 committed Mar 5, 2020
1 parent ef09e8a commit 8d6a0b917ce1e7f4f1017835af0ca76e79c98c38
Showing with 39 additions and 11 deletions.
  1. +13 −7 builtin/common/serialize.lua
  2. +17 −1 builtin/common/tests/serialize_spec.lua
  3. +9 −3 doc/lua_api.txt
@@ -177,13 +177,16 @@ end

-- Deserialization

local env = {
loadstring = loadstring,
}
local function safe_loadstring(...)
local func, err = loadstring(...)
if func then
setfenv(func, {})
return func
end
return nil, err
end

local safe_env = {
loadstring = function() end,
}
local function dummy_func() end

function core.deserialize(str, safe)
if type(str) ~= "string" then
@@ -195,7 +198,10 @@ function core.deserialize(str, safe)
end
local f, err = loadstring(str)
if not f then return nil, err end
setfenv(f, safe and safe_env or env)

-- The environment is recreated every time so deseralized code cannot
-- pollute it with permanent references.
setfenv(f, {loadstring = safe and dummy_func or safe_loadstring})

local good, data = pcall(f)
if good then
@@ -1,6 +1,6 @@
_G.core = {}

_G.setfenv = function() end
_G.setfenv = require 'busted.compatibility'.setfenv

dofile("builtin/common/serialize.lua")

@@ -25,4 +25,20 @@ describe("serialize", function()
local test_out = core.deserialize(core.serialize(test_in))
assert.same(test_in, test_out)
end)

it("strips functions in safe mode", function()
local test_in = {
func = function(a, b)
error("test")
end,
foo = "bar"
}

local str = core.serialize(test_in)
assert.not_nil(str:find("loadstring"))

local test_out = core.deserialize(str, true)
assert.is_nil(test_out.func)
assert.equals(test_out.foo, "bar")
end)
end)
@@ -5275,10 +5275,16 @@ Misc.
* Convert a table containing tables, strings, numbers, booleans and `nil`s
into string form readable by `minetest.deserialize`
* Example: `serialize({foo='bar'})`, returns `'return { ["foo"] = "bar" }'`
* `minetest.deserialize(string)`: returns a table
* Convert a string returned by `minetest.deserialize` into a table
* `minetest.deserialize(string[, safe])`: returns a table
* Convert a string returned by `minetest.serialize` into a table
* `string` is loaded in an empty sandbox environment.
* Will load functions, but they cannot access the global environment.
* Will load functions if safe is false or omitted. Although these functions
cannot directly access the global environment, they could bypass this
restriction with maliciously crafted Lua bytecode if mod security is
disabled.
* This function should not be used on untrusted data, regardless of the
value of `safe`. It is fine to serialize then deserialize user-provided
data, but directly providing user input to deserialize is always unsafe.
* Example: `deserialize('return { ["foo"] = "bar" }')`,
returns `{foo='bar'}`
* Example: `deserialize('print("foo")')`, returns `nil`

0 comments on commit 8d6a0b9

Please sign in to comment.
You can’t perform that action at this time.