From 2a8a2445d3f6c7e47a4235e696a1b9187e5b2bcf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Enrique=20Garci=CC=81a=20Cota?= <kikito@gmail.com>
Date: Tue, 5 Jan 2021 14:23:24 +0100
Subject: [PATCH] feat(sandbox) block bytecode when possible

---
 README.md             | 35 ++++++++++++++++++++++++++++++++++-
 sandbox.lua           | 13 +++++--------
 spec/sandbox_spec.lua | 16 ++++++----------
 3 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/README.md b/README.md
index 00ee06b..b47fa8d 100644
--- a/README.md
+++ b/README.md
@@ -9,6 +9,16 @@ It's possible to provide extra functions via the `options.env` parameter.
 
 Infinite loops are prevented via the `debug` library.
 
+Supported Lua versions:
+======================
+
+All the features of sandbox.lua work in the following Lua environments:
+
+
+* PUC-Rio Lua 5.1 **allows execution of bytecode**, which is a huge limitation (see the bytecode section below)
+* PUC-Rio Lua 5.2, 5.3, 5.4 have total support.
+* LuaJIT is not protected against infinite loops (see the notes in `options.quota` below)
+
 Usage
 =====
 
@@ -51,6 +61,29 @@ local sf = sandbox.protect([[
 sf() -- error: quota exceeded
 ```
 
+### Bytecode
+
+It is possible to exit a sandbox using Lua bytecode. References:
+
+* http://apocrypha.numin.it/talks/lua_bytecode_exploitation.pdf
+* https://github.com/erezto/lua-sandbox-escape
+* https://gist.github.com/corsix/6575486
+
+Because of this, the sandbox deactivates bytecode in all the versions of Lua where it is possible:
+
+* PUC-Rio Lua 5.2, 5.3, 5.4
+* LuaJIT
+
+In other words, _all except PUC-Rio Lua 5.1_.
+
+** The sandbox can be exploited in PUC-Rio Lua 5.1 via bytecode **
+
+The only reason we keep Lua 5.1 in the list of supported versions of Lua is because
+sandboxing can help against users attempting to delete a file by mistake. _It does not provide
+protection against malicious users_.
+
+As a result we _strongly recommend updating to a more recent version when possible_.
+
 ### options.quota
 
 `sandbox.lua` prevents infinite loops from halting the program by hooking the `debug` library to the sandboxed function, and "counting instructions". When
@@ -95,7 +128,7 @@ recommended to discard it after use.
 
 ### sandbox.run
 
-`sandbox.run(code)` sanboxes and executes `code` in a single line. `code` must be a string with Lua code inside.
+`sandbox.run(code)` sandboxes 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`.
 
diff --git a/sandbox.lua b/sandbox.lua
index db2cca3..ee3db14 100644
--- a/sandbox.lua
+++ b/sandbox.lua
@@ -33,9 +33,9 @@ local sandbox = {
 local quota_supported = type(_G.jit) == "nil"
 sandbox.quota_supported = quota_supported
 
--- 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
+-- PUC-Rio Lua 5.1 does not support deactivation of bytecode
+local bytecode_blocked = _ENV or type(_G.jit) == "table"
+sandbox.bytecode_blocked = bytecode_blocked
 
 -- The base environment is merged with the given env option (or an empty table, if no env provided)
 --
@@ -143,12 +143,9 @@ function sandbox.protect(code, options)
 
 
   local f
-  if mode_supported then
-    f = assert(load(code, nil, options.mode, env))
+  if bytecode_blocked then
+    f = assert(load(code, nil, 't', 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
diff --git a/spec/sandbox_spec.lua b/spec/sandbox_spec.lua
index 9155bef..f0f889b 100644
--- a/spec/sandbox_spec.lua
+++ b/spec/sandbox_spec.lua
@@ -9,19 +9,15 @@ describe('sandbox.run', function()
       assert.equal(r, 'hello')
     end)
 
-    it('can run bytecode strings by default', function()
-      local fn = function() end
-      assert.has_no.error(function() sandbox.run(string.dump(fn)) end)
-    end)
-
-    if sandbox.mode_supported then
-      it('can\'t run bytecode strings if given a \'t\' mode option', function()
+    if sandbox.bytecode_blocked then
+      it('rejects bytecode', function()
         local fn = function() end
-        assert.error(function() sandbox.run(string.dump(fn), { mode = 't' }) end)
+        assert.error(function() sandbox.run(string.dump(fn)) end)
       end)
     else
-      it('throws an error if the mode option is used', function()
-        assert.error(function() sandbox.run("", { mode = 't' }) end)
+      it('accepts bytecode (PUC Rio 5.1)', function()
+        local fn = function() end
+        assert.has.no.error(function() sandbox.run(string.dump(fn)) end)
       end)
     end