Lua sandboxing

[nstbayless]

Lua is not sandboxed well today.

Ideally, I would like the following to be true regarding Lua:

• It's safe to run maps downloaded from strangers. We could even sync maps over netplay.
• It's incapable of producing a desync in multiplayer and during replays.

This PR just removes some functionality that doesn't seem to be used anywhere: The entirety of "jit" and "ffi" (allows arbitrary C code execution!), "io" and "os" (filesystem operations) are removed, save for io.open and os.remove (which some existing maps use, and is enough for saving/loading/deleting). Later, we can add special save data lua functions for this purpose which can sync over multiplayer.

Also removes package.loadlib which can run DLLs.

Also switch from using rand() to using prng that all clients can predict.

[cerwym]

Please go the full distance on this, I don't want it possible for any lua script to operate with os, io or debug at all

lua_getglobal(L, "package");
lua_getfield(L, -1, "loaders");
if (lua_istable(L, -1)) {
	lua_pushnil(L);
	lua_rawseti(L, -2, 3);
	lua_pushnil(L);
	lua_rawseti(L, -2, 4);
}
lua_pop(L, 2);

Would disable the package loaders too that would allow bypass of require() which now you've highlighted this, im terrified of

[nstbayless]

Hm. The problem is that existing Lua maps use these for saving and loading. So we need a new API to replace that. What would you suggest?

[cerwym]

That's correct, and I think what we need to do, in the vein of security is give a specialized API function that can only write to a sane location in our control;

see PlatformWindows::GetUserPrefDir() for an example of PlatformIO.

What I envision is an IFileSystem interface that also has C bindings for API methods in Lua to use.

Basically I don't want any path traversal or any ability to break out of the sandbox. This would be net-new and not use any of the Lb* functions.

I saw this PR and got to to thinking, in 5 minutes I was able to put up a POC and invite Loob into a game where I performed an RCE on his machine.

It's safe to run maps downloaded from strangers. We could even sync maps over netplay.

In order to fulfill that dream, we'd have to really think about this deeply, you're on the right track with this

[PieterVdc]

debug specifically I do rely on to check if a function is properly serializable, don't see why that one would be an issue?

[nstbayless]

I agree we have to think deeply, but I'm also not intending that this PR will solve 100% of vulnerabilities. My goal with this PR is to remove all vulnerabilities which are not presently relied on for any current lua maps.

[nstbayless]

IFileSystem interface

I can see this being useful for loading datafiles perhaps. But why give write access at all? If we want to let the script store persistent data, IMO we should have a special section in the save files for it or something like that.

Buuut I don't want to touch save file data right now ;)

[cerwym]

But why give write access at all?

So you can load sounds and things, lua scripts can legitimately play audio files

[nstbayless]

that's read access, no?

[cerwym]

It's still inherently bad. Realistically, our allowance of IO should be mapped to a VFS, as it stands, a lua script could read any file on the system.

The TLDR solution is to wrap any lua IO code with something akin to this;

static int safe_io_open(lua_State *L) {
    const char *path = luaL_checkstring(L, 1);
    const char *mode = luaL_optstring(L, 2, "r");

    // Prevent path traversal attempts
    if (strstr(path, "..") != NULL || path[0] == '/') {
        return luaL_error(L, "Access denied: Invalid file path.");
    }

    // Force all file paths into a restricted directory
    char safe_path[512];
    snprintf(safe_path, sizeof(safe_path), "sandbox_dir/%s", path);

    // Call underlying fopen
    FILE *f = fopen(safe_path, mode);
    if (!f) return luaL_fileresult(L, 0, safe_path);

    // Push standard Lua file handle object
    luaL_Stream *p = (luaL_Stream *)lua_newuserdata(L, sizeof(luaL_Stream));
    p->f = f;
    p->closef = &io_fclose; // standard lua close function
    luaL_setmetatable(L, LUA_FILEHANDLE);
    return 1;
}

If io.open was to be allowed you'd rely on OS process isolation, but for KeeperFX that solution would be unbelievably silly, overkill and hurt performance