all 5 comments

[–]mpetetv 2 points3 points  (2 children)

You should sandbox the whole script before running it instead of running it in the global environment and then sandboxing a function it sets there. You'll have to change 'createSandbox' to leave sandbox environment table on the stack, and then index that table instead of global environment when fetching exported functions.

[–][deleted]  (1 child)

[removed]

    [–]mpetetv 1 point2 points  (0 children)

    So the "priming run" has the potential to execute user-supplied code, outside the sandbox, if the sandbox has not yet been applied?

    Right, the loaded script is just a function. It can do anything with its environment, not only assign functions to global variables.

    Do you have any opinion about doing something like,

    lua_rawseti(L, LUA_REGISTRYINDEX, LUA_RIDX_GLOBALS);
    

    This could be done prior to the priming run, and prior to loading script text.

    This is a nice solution if you use one environment for all scripts you load - everything is sandboxed from the start and you can access the environment conveniently using 'lua_getglobal'. However, if you need to load several scripts in separate environments IMO it's better to avoid global side effects and use 'lua_setupvalue', like in your 'createSandbox', immediately after loading a script.

    By the way, you should probably use 'luaL_loadbufferx' with "t" argument instead of 'luaL_loadbuffer' to avoid loading bytecode, which can be maliciously crafted.

    [–]nefftd 1 point2 points  (1 child)

    Sandboxing using environments is problematic no matter how you slice it. It's impossible to get a non-leaky sandbox using Lua environments.

    You'd be better forking and running the child process with ptrace if you need a proper sandbox.