From 50bfa4abca138a9bb473cea8358f47f0e0c8d5db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Garci=CC=81a=20Cota?= Date: Tue, 5 Jan 2021 00:40:59 +0100 Subject: [PATCH] feat(sandbox): only allow strings of Lua as params This change drops support for "protecting" raw Lua functions. There are two main reasons for this change: * More modern versions of PUC Rio Lua don't have `setfenv`. It is possible to get around this by using the debug library, but that library is not available in all environments. * Solutions based on `load` (which only allow string inputs) are objectively better since they give the user more control. For instance, you can deactivate support for binary code selectively. As a result, we are using the `load`-based sandbox in all versions of Lua that supports it, using `setfenv`-based sandboxing only when nothing else is available (PUC Rio 5.1). We are also explicitly raising an error if `options.mode` is passed but we are using `setfenv`. This is to prevent users from believing they are protected against binary code, when in fact they are not. --- README.md | 23 ++++++++++++----------- sandbox.lua | 42 +++++++++++++++++------------------------- spec/sandbox_spec.lua | 24 +++++++++++++----------- 3 files changed, 42 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 0ee1f9c..264b970 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,8 @@ local sandbox = require 'sandbox' ### sandbox.protect -`sandbox.protect(f)` (or `sandbox(f)`) produces a sandboxed version of `f`. `f` can be a Lua function or a string with Lua code. - -A sandboxed function works as regular functions as long as they don't access any insecure features: +`sandbox.protect("lua code")` (or `sandbox("lua code")`) produces a sandboxed function. The resulting sandboxed +function works as regular functions as long as they don't access any insecure features: ```lua local sandboxed_f = sandbox(function() return 'hey' end) @@ -34,9 +33,10 @@ Sandboxed options can not access unsafe Lua modules. (See the [source code](http When a sandboxed function tries to access an unsafe module, an error is produced. ```lua -local sf = sandbox.protect(function() +local sf = sandbox.protect([[ os.execute('rm -rf /') -- this will throw an error, no damage done -end) +end +]]) sf() -- error: os.execute not found ``` @@ -44,9 +44,9 @@ sf() -- error: os.execute not found Sandboxed functions will eventually throw an error if they contain infinite loops: ```lua -local sf = sandbox.protect(function() +local sf = sandbox.protect([[ while true do end -end) +]]) sf() -- error: quota exceeded ``` @@ -93,14 +93,15 @@ recommended to discard it after use. ### sandbox.run -`sandbox.run(f)` sanboxes and executes `f` in a single line. `f` can be either a string or a function +`sandbox.run(code)` sanboxes and executes `code` in a single line. `code` must be a string with Lua code inside. You can pass `options` param, and it will work like in `sandbox.protect`. -Any extra parameters will just be passed to the sandboxed function when executed. -In other words, `sandbox.run(f, o, ...)` is equivalent to `sandbox.protect(f,o)(...)`. +Any extra parameters will just be passed to the sandboxed function when executed, and available on the top-level scope via the `...` varargs parameters. -Notice that if `f` throws an error, it is *NOT* captured by `sandbox.run`. Use `pcall` if you want your app to be immune to errors, like this: +In other words, `sandbox.run(c, o, ...)` is equivalent to `sandbox.protect(c, o)(...)`. + +Notice that if `code` throws an error, it is *NOT* captured by `sandbox.run`. Use `pcall` if you want your app to be immune to errors, like this: ``` lua local ok, result = pcall(sandbox.run, 'error("this just throws an error")') diff --git a/sandbox.lua b/sandbox.lua index c17604f..4ac8ecd 100644 --- a/sandbox.lua +++ b/sandbox.lua @@ -28,26 +28,10 @@ local sandbox = { ]] } --- Lua 5.2, 5.3 and above --- https://leafo.net/guides/setfenv-in-lua52-and-above.html#setfenv-implementation -local setfenv = setfenv or function(fn, env) - local i = 1 - while true do - local name = debug.getupvalue(fn, i) - if name == "_ENV" then - debug.upvaluejoin(fn, i, (function() - return env - end), 1) - break - elseif not name then - break - end - i = i + 1 - end - - return fn -end +-- PUC-Rio Lua 5.1 does not support deactivation of binary code +local mode_supported = _ENV or type(_G.jit) == "table" +sandbox.mode_supported = mode_supported -- The base environment is merged with the given env option (or an empty table, if no env provided) -- @@ -137,7 +121,7 @@ local function cleanup() end -- Public interface: sandbox.protect -function sandbox.protect(f, options) +function sandbox.protect(code, options) options = options or {} local quota = false @@ -148,9 +132,17 @@ function sandbox.protect(f, options) local env = merge(options.env or {}, BASE_ENV) env._G = env._G or env - if type(f) == 'string' then - f = assert(load(f, nil, options.mode, env)) + assert(type(code) == 'string', "expected a string") + + + local f + if mode_supported then + f = assert(load(code, nil, options.mode, env)) else + if options.mode then + error("options.mode is not supported on this environment (usually PUC-Rio Lua 5.1). Please unset options.mode") + end + f = assert(loadstring(code)) setfenv(f, env) end @@ -177,11 +169,11 @@ function sandbox.protect(f, options) end -- Public interface: sandbox.run -function sandbox.run(f, options, ...) - return sandbox.protect(f, options)(...) +function sandbox.run(code, options, ...) + return sandbox.protect(code, options)(...) end -- make sandbox(f) == sandbox.protect(f) -setmetatable(sandbox, {__call = function(_,f,o) return sandbox.protect(f,o) end}) +setmetatable(sandbox, {__call = function(_,code,o) return sandbox.protect(code,o) end}) return sandbox diff --git a/spec/sandbox_spec.lua b/spec/sandbox_spec.lua index 9f33388..e0f4295 100644 --- a/spec/sandbox_spec.lua +++ b/spec/sandbox_spec.lua @@ -3,10 +3,6 @@ local sandbox = require 'sandbox' describe('sandbox.run', function() describe('when handling base cases', function() - it('can run harmless functions', function() - local r = sandbox.run(function() return 'hello' end) - assert.equal(r, 'hello') - end) it('can run harmless strings', function() local r = sandbox.run("return 'hello'") @@ -18,10 +14,16 @@ describe('sandbox.run', function() assert.has_no.error(function() sandbox.run(string.dump(fn)) end) end) - it('can\'t run bytecode strings if given a \'t\' mode option', function() - local fn = function() end - assert.error(function() sandbox.run(string.dump(fn), { mode = 't' }) end) - end) + if sandbox.mode_supported then + it('can\'t run bytecode strings if given a \'t\' mode option', function() + local fn = function() end + assert.error(function() sandbox.run(string.dump(fn), { mode = 't' }) end) + end) + else + it('throws an error if the mode option is used', function() + assert.error(function() sandbox.run("", { mode = 't' }) end) + end) + end it('has access to safe methods', function() assert.equal(10, sandbox.run("return tonumber('10')")) @@ -56,13 +58,13 @@ describe('sandbox.run', function() assert.equal('hellohello', string.rep('hello', 2)) end) - it('passes parameters to the function', function() - assert.equal(sandbox.run(function(a,b) return a + b end, {}, 1,2), 3) + it('passes parameters to the code', function() + assert.equal(sandbox.run("local a, b = ...; return a + b", {}, 1,2), 3) end) end) - describe('when the sandboxed function tries to modify the base environment', function() + describe('when the sandboxed code tries to modify the base environment', function() it('does not allow modifying the modules', function() assert.error(function() sandbox.run("string.foo = 1") end)