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

@@ -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")')

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

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)