Merge pull request #245 from GinjaNinja32/sandbox-metatable

Improve sandboxing setup; re-allow get/setmetatable

CMakeLists.txt

@@ -173,6 +173,7 @@ set(source_files #All SeriousProton's objects to compile
     src/resources.cpp
     src/scriptInterface.cpp
     src/scriptInterfaceMagic.cpp
+    src/scriptInterfaceSandbox.cpp
     src/shaderManager.cpp
     src/soundManager.cpp
     src/stringImproved.cpp
@@ -252,6 +253,7 @@ set(source_files #All SeriousProton's objects to compile
     src/resources.h
     src/scriptInterface.h
     src/scriptInterfaceMagic.h
+    src/scriptInterfaceSandbox.h
     src/shaderManager.h
     src/soundManager.h
     src/stringImproved.h

src/scriptInterface.cpp

@@ -1,8 +1,10 @@
 #include <sys/stat.h>
+#include <cstring>
 
 #include "random.h"
 #include "resources.h"
 #include "scriptInterface.h"
+#include "scriptInterfaceSandbox.h"
 
 static int random(lua_State* L)
 {
@@ -107,6 +109,13 @@ static const luaL_Reg loadedlibs[] = {
   {NULL, NULL}
 };
 
+static const char* safe_functions[] = {
+    "assert", "error", "getmetatable", "ipairs", "next", "pairs", "pcall",
+    "print", "rawequal", "rawget", "rawlen", "rawset", "require", "select",
+    "setmetatable", "tonumber", "tostring", "type", "xpcall",
+    NULL,
+};
+
 void ScriptObject::createLuaState()
 {
     if (L == NULL)
@@ -119,36 +128,80 @@ void ScriptObject::createLuaState()
             luaL_requiref(L, lib->name, lib->func, 1);
             lua_pop(L, 1);  /* remove lib */
         }
-    }
 
-    //Remove unsafe base functions.
-    lua_pushnil(L);
-    lua_setglobal(L, "collectgarbage");
-    lua_pushnil(L);
-    lua_setglobal(L, "dofile");
-    lua_pushnil(L);
-    lua_setglobal(L, "getmetatable");
-    lua_pushnil(L);
-    lua_setglobal(L, "loadfile");
-    lua_pushnil(L);
-    lua_setglobal(L, "load");
-    lua_pushnil(L);
-    lua_setglobal(L, "rawequal");
-    lua_pushnil(L);
-    lua_setglobal(L, "setmetatable");
+        // Protect the metatable the string library sets on strings.
+        // This metatable points to the global `string` library, rather than the environment's copy.
+        // This global string library table can't be accessed directly, but it means a function defining
+        // `function string.foo(...)` can't call that function as `"some_string":foo()` since the metatable
+        // points to the global version rather than its own copy.
+        // TODO see if this can be fixed without breaking the sandbox. probably not.
+        lua_pushstring(L, "");
+        protectLuaMetatable(L);
+        lua_pop(L, 1);
+    }
 
     //Setup a new table as the first upvalue. This will be used as "global" environment for the script. And thus will prevent global namespace polution.
     lua_newtable(L);  /* environment for loaded function */
 
+    // set a _G in the script's environment pointing to its own global environment
+    lua_pushstring(L, "_G");
+    lua_pushvalue(L, -2);
+    lua_rawset(L, -3);
+
+    // Copy in the safe global functions.
+    for (const char **fn = safe_functions; *fn; fn++)
+    {
+        lua_pushstring(L, *fn);
+        lua_getglobal(L, *fn);
+        lua_rawset(L, -3);
+    }
+
+    // Copy in the global libraries.
+    for (const luaL_Reg *lib = loadedlibs; lib->func; lib++)
+    {
+        if (!strcmp(lib->name, "_G")) {
+            continue;
+        }
+
+        // Make a table for the library.
+        lua_newtable(L);              // [env] [local]
+
+        // Set it into the script environment.
+        lua_pushstring(L, lib->name); // [env] [local] [libname]
+        lua_pushvalue(L, -2);         // [env] [local] [libname] [local]
+        lua_rawset(L, -4);            // [env] [local]
+
+        // Iterate the global library.
+        lua_getglobal(L, lib->name);  // [env] [local] [global]
+        lua_pushnil(L);               // [env] [local] [global] nil
+        while (lua_next(L, -2))
+        {                             // [env] [local] [global] [key] [value]
+            if (!lua_isfunction(L, -1) && !lua_isnumber(L, -1))
+            {
+                // This doesn't trigger on anything right now; it's here in case anything gets added to any of the libraries that would potentially break the sandbox.
+                LOG(WARNING) << "ignoring non-{function,number} in " << lib->name;
+                lua_pop(L, 1);
+                continue;
+            }
+
+            // Functions and numbers are safe to share - copy the value into the script's library table
+            lua_pushvalue(L, -2); // [env] [local] [global] [key] [value] [key]
+            lua_rotate(L, -2, 1); // [env] [local] [global] [key] [key] [value]
+            lua_rawset(L, -5);    // [env] [local] [global] [key]
+        }
+                       // [env] [local] [global]
+        lua_pop(L, 2); // [env]
+    }
+
     //Register all global functions for our game.
     for(ScriptClassInfo* item = scriptClassInfoList; item != NULL; item = item->next)
+    {
         item->register_function(L);
 
-    lua_newtable(L);  /* meta table for the environment, with an __index pointing to the general global table so we can access every global function */
-    lua_pushstring(L, "__index");
-    lua_pushglobaltable(L);
-    lua_rawset(L, -3);
-    lua_setmetatable(L, -2);
+        lua_pushstring(L, item->class_name.c_str());
+        lua_getglobal(L, item->class_name.c_str());
+        lua_rawset(L, -3);
+    }
 
     //Register the destroyScript function. This needs a reference back to the script object, we pass this as an upvalue.
     lua_pushstring(L, "destroyScript");
@@ -241,7 +294,7 @@ void ScriptObject::setVariable(string variable_name, string value)
     //Set our variable in this environment table
     lua_pushstring(L, variable_name.c_str());
     lua_pushstring(L, value.c_str());
-    lua_settable(L, -3);
+    lua_rawset(L, -3);
 
     //Pop the table
     lua_pop(L, 1);
@@ -258,7 +311,7 @@ void ScriptObject::registerObject(P<PObject> object, string variable_name)
 
     if (convert< P<PObject> >::returnType(L, object))
     {
-        lua_settable(L, -3);
+        lua_rawset(L, -3);
         //Pop the environment table
         lua_pop(L, 1);
     }else{
@@ -382,7 +435,7 @@ bool ScriptObject::callFunction(string name)
     lua_gettable(L, LUA_REGISTRYINDEX);
     //Get the function from the environment
     lua_pushstring(L, name.c_str());
-    lua_gettable(L, -2);
+    lua_rawget(L, -2);
     //Call the function
     if (lua_pcall(L, 0, 0, 0))
     {
@@ -436,7 +489,7 @@ void ScriptObject::update(float delta)
     lua_gettable(L, LUA_REGISTRYINDEX);
     // Get the update function from the script environment
     lua_pushstring(L, "update");
-    lua_gettable(L, -2);
+    lua_rawget(L, -2);
 
     // If it's a function, call it, if not, pop the environment and the function from the stack.
     if (!lua_isfunction(L, -1))
@@ -527,7 +580,7 @@ void ScriptCallback::operator() ()
         lua_pop(L, 1);
         return;
     }
-    
+
     lua_pushnil(L);
     while (lua_next(L, -2) != 0)
     {
@@ -542,7 +595,7 @@ void ScriptCallback::operator() ()
                 //Stack is [callback_table] [callback_key] [callback_entry_table] [script_pointer]
                 lua_pushvalue(L, -3);
                 lua_pushnil(L);
-                lua_settable(L, -6);
+                lua_rawset(L, -6);
                 lua_pop(L, 1);
             }else{
                 lua_pop(L, 1);
@@ -713,7 +766,7 @@ template<> void convert<ScriptSimpleCallback>::param(lua_State* L, int& idx, Scr
 
     //Stack is now: [function_environment] [callback object pointer] [table] "script_pointer"
     lua_pushstring(L, "__script_pointer");
-    lua_gettable(L, -5);
+    lua_rawget(L, -5);
     if (lua_isnil(L, -1))
     {
         //Simple functions that do not access globals do not inherit their environment from their creator, so they have nil here.

src/scriptInterfaceMagic.h

@@ -10,6 +10,7 @@
 #include "P.h"
 #include "stringImproved.h"
 #include "lua/lua.hpp"
+#include "scriptInterfaceSandbox.h"
 #include "glm/gtc/type_precision.hpp"
 #include <typeinfo>
 #include <optional>
@@ -124,7 +125,7 @@ struct convert<T*>
             return;
         }
         lua_pushstring(L, "__ptr");
-        lua_gettable(L, idx++);
+        lua_rawget(L, idx++);
 
         P<PObject>** p = static_cast< P<PObject>** >(lua_touserdata(L, -1));
         lua_pop(L, 1);
@@ -153,7 +154,7 @@ struct convert<P<T>>
             return;
         }
         lua_pushstring(L, "__ptr");
-        lua_gettable(L, idx++);
+        lua_rawget(L, idx++);
 
         P<PObject>** p = static_cast< P<PObject>** >(lua_touserdata(L, -1));
         lua_pop(L, 1);
@@ -196,7 +197,8 @@ struct convert<P<T>>
             P<PObject>** p = static_cast< P<PObject>** >(lua_newuserdata(L, sizeof(P<PObject>*)));
             *p = new P<PObject>();
             (**p) = ptr;
-            lua_settable(L, -3);
+
+            lua_rawset(L, -3);
 
             lua_pushlightuserdata(L, ptr);
             lua_pushvalue(L, -2);
@@ -367,7 +369,7 @@ template<class T> struct call<T, ScriptCallback T::* >
         if (!lua_istable(L, -1))
             luaL_error(L, "??[setcallbackFunction] Upvalue 1 of function is not a table...");
         lua_pushstring(L, "__script_pointer");
-        lua_gettable(L, -2);
+        lua_rawget(L, -2);
         if (!lua_islightuserdata(L, -1))
             luaL_error(L, "??[setcallbackFunction] Cannot find reference back to script...");
         //Stack is now: [function_environment] [pointer]
@@ -523,7 +525,7 @@ template<class T> class scriptBindObject
         if (!lua_istable(L, -1))
             return 0;
         lua_pushstring(L, "__ptr");
-        lua_gettable(L, -2);
+        lua_rawget(L, -2);
         if (lua_isuserdata(L, -1)) //When a subclass is destroyed, it's metatable might call the __gc function on it's sub-metatable. So we can get nil values here, ignore that.
         {
             PT* p = static_cast< PT* >(lua_touserdata(L, -1));
@@ -598,6 +600,10 @@ template<class T> class scriptBindObject
             lua_pop(L, 1);
             luaL_newmetatable(L, objectTypeName);
         }
+
+        lua_pushstring(L, "__metatable"); // protect the metatable
+        lua_pushstring(L, "sandbox");
+        lua_settable(L, metatable);
 
         lua_pushstring(L, "__gc");
         lua_pushcfunction(L, gc_collect);

src/scriptInterfaceSandbox.cpp

@@ -0,0 +1,19 @@
+#include "lua/lua.hpp"
+#include "scriptInterfaceSandbox.h"
+
+// Add or protect a metatable for the provided object
+// Input Lua stack: [object]
+// Output Lua stack: [object]
+void protectLuaMetatable(lua_State* L)
+{
+    if (!lua_getmetatable(L, -1)) {
+        lua_newtable(L);         // [object] [mt]
+        lua_pushvalue(L, -1);    // [object] [mt] [mt]
+        lua_setmetatable(L, -3); // [object] [mt]
+    }
+
+    lua_pushstring(L, "__metatable"); // [object] [mt] "__metatable"
+    lua_pushstring(L, "sandbox");     // [object] [mt] "__metatable" "sandbox"
+    lua_rawset(L, -3);                // [object] [mt]
+    lua_pop(L, 1);                    // [object]
+}

src/scriptInterfaceSandbox.h

@@ -0,0 +1,11 @@
+#ifndef SCRIPT_INTERFACE_SANDBOX_H
+#define SCRIPT_INTERFACE_SANDBOX_H
+
+#include "lua/lua.hpp"
+
+// Add or protect a metatable for the provided object such that the metatable can't be read or changed from inside the sandbox.
+// Input Lua stack: [object]
+// Output Lua stack: [object]
+void protectLuaMetatable(lua_State* L);
+
+#endif // SCRIPT_INTERFACE_SANDBOX_H