diff options
| -rw-r--r-- | builtin/common/serialize.lua | 20 | ||||
| -rw-r--r-- | builtin/common/tests/serialize_spec.lua | 18 | ||||
| -rw-r--r-- | doc/lua_api.txt | 12 |
3 files changed, 39 insertions, 11 deletions
diff --git a/builtin/common/serialize.lua b/builtin/common/serialize.lua index cf00107c2..163aa67ad 100644 --- a/builtin/common/serialize.lua +++ b/builtin/common/serialize.lua @@ -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 diff --git a/builtin/common/tests/serialize_spec.lua b/builtin/common/tests/serialize_spec.lua index 321d2766a..c41b0a372 100644 --- a/builtin/common/tests/serialize_spec.lua +++ b/builtin/common/tests/serialize_spec.lua @@ -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) diff --git a/doc/lua_api.txt b/doc/lua_api.txt index 10ede5168..acf2f77c1 100644 --- a/doc/lua_api.txt +++ b/doc/lua_api.txt @@ -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` |