Re: Upgrading C sandbox from 5.1 to 5.2/5.3

[Luke Mewburn <luke@...>]

On Tue, Feb 10, 2015 at 11:14:59AM -0500, Michael Welsh Duggan wrote:
  | Luke Mewburn <luke@mewburn.net> writes:
  | > Current use in Lua 5.1:
  | >
  | > The sandboxing setup technique used in Lua 5.1 is:
  | > 	a) create a table for the environment
  | > 	b) copy names of "safe" globals into the table, such as
  | > 	   assert(), ipairs(), string.*, (etc)
  | > 	c) add other (private) extensions to the table
  | > 	d) add the table to the global environment with a well-known name
  | >
  | > To invoke a chunk or function 
  | > 	a) limit instructions / maskcount
  | > 	b) get the sandbox table from the global environment
  | > 	c) set the environment for the chunk/function with lua_setfenv()
  | > 	d) call the chunk/function
  | 
  | So, typically in 5.2, it makes more sense to:
  |         a) create the sandbox table
  |         b) load the user code as chunks
  |         c) set the first upvalue of each chunk to the sandbox table
  | 
  | Then user functions are just used normally.  Their globals will always
  | be from the sandbox table.

Thanks for the clarification!

With some minor adjustments to my code I've managed to get the wrapper
library and the testsuite working with both Lua 5.1 and Lua 5.2.

To summarize the differences in the code:
	Lua 5.1:
		- setfenv to the sandbox table for chunk pcall
		- setfenv to the sandbox table for function pcall
	Lua 5.2:
		- setupvalue to a copy of the sandbox table for chunk pcall
		- nothing special for function pcall

In (pseudo)code my methods changed the following, with the
new code in the appropriate #ifs

	// Setup sandbox and call chunk on the stack
	callChunk(lua_state * LL)
		lua_newtable(L)			// populate table for sandbox
		setupSandbox(L)
	#if LUA_VERSION_NUM >= 502
		lua_pushvalue(L, -1);		// copy sandbox for upvalue
		lua_setupvalue(L, -3, 1))
	#endif
		lua_setglobal(L, "LuaSandbox");	// store sandbox in global var
		sandboxCall(L, 0, 0)

	// Call function on stack in a sandbox (maskcount, restricted environment)
	sandboxCall(lua_state * L, int iNumArgs, int iNumResults)
		lua_sethook(L, &maskcount, LUA_MASKCOUNT, someLimit);
	#if LUA_VERSION_NUM < 502
		lua_getglobal(L, "LuaSandBox");	// set environment to sandbox
		lua_setfenv(L, -iNumArgs - 2);
	#endif
		lua_pcall(L, iNumArgs, iNumResults, 0);
		lua_sethook(L, &maskcount, LUA_MASKCOUNT, 0);


I also had to change the mechanism I used to copy items from the
top-level of the default environment into the sandbox (e.g. "assert()")
from just referencing the table at LUA_GLOBALSINDEX directly to
explicitly accessing the globals with:
	#if LUA_VERSION_NUM >= 502
		lua_pushglobaltable(L);
	#else
		lua_pushvalue(L, LUA_GLOBALSINDEX);
	#endif

It was this change that fixed my testsuite exercising calling sandboxed
copies of top-level functions.



  | If you really want to change as little as possible, then something like
  | the below is an option.  During setup it creates a chunk that just calls
  | the passed in function and arguments.  To call a function within an
  | environment, it pushes the apply chunk, sets it's _ENV upvalue, and then
  | calls it with the function and its arguments.  (Code written in my MUA,
  | and as such completely untested!  May contain indexing errors.  But
  | should be sufficient to explain a possible solution.)
  | 
  | const char *apply_code = "local fn = ...; return fn(select(2,...))";
  | static int apply_ref;
  | static void setup(lua_State *L) {
  |     luaL_loadstring(L, apply_code);
  |     call_with_env_ref = luaL_ref(L, LUA_REGISTRYINDEX);
  | }
  | 
  | int sandboxpcall(lua_State *L, int envtable, int nargs, int nresults) {
  |     int rv;
  |     int fn_index = lua_absindex(L, -nargs - 1);
  |     envtable = lua_absindex(L, envtable);
  |     lua_rawgeti(L, LUA_REGISTRYINDEX, apply);
  |     lua_pushvalue(L, envtable);
  |     lua_setupvalue(L, -2, 1);
  |     lua_insert(L, fn_index);
  |     return lua_pcall(L, nargs + 1, nresults, 0);
  | }

Interesting idea. I may use that elsewhere as necessary.


cheers,
Luke.

Attachment: pgpSEyhTKt2Zo.pgp
Description: PGP signature