diff --git a/src/ffi.rs b/src/ffi.rs index 4ac0f52..9a13d21 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -121,8 +121,26 @@ extern "C" { pub fn lua_error(state: *mut lua_State) -> !; pub fn lua_atpanic(state: *mut lua_State, panic: lua_CFunction) -> lua_CFunction; + pub fn luaopen_base(state: *mut lua_State) -> c_int; + pub fn luaopen_coroutine(state: *mut lua_State) -> c_int; + pub fn luaopen_table(state: *mut lua_State) -> c_int; + pub fn luaopen_io(state: *mut lua_State) -> c_int; + pub fn luaopen_os(state: *mut lua_State) -> c_int; + pub fn luaopen_string(state: *mut lua_State) -> c_int; + pub fn luaopen_utf8(state: *mut lua_State) -> c_int; + pub fn luaopen_math(state: *mut lua_State) -> c_int; + pub fn luaopen_debug(state: *mut lua_State) -> c_int; + pub fn luaopen_package(state: *mut lua_State) -> c_int; + pub fn luaL_newstate() -> *mut lua_State; pub fn luaL_openlibs(state: *mut lua_State); + pub fn luaL_requiref( + state: *mut lua_State, + modname: *const c_char, + openf: lua_CFunction, + glb: c_int, + ); + pub fn luaL_loadbufferx( state: *mut lua_State, buf: *const c_char, diff --git a/src/lua.rs b/src/lua.rs index 8ce38dd..dcb892f 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -1302,7 +1302,18 @@ impl Lua { let state = ffi::luaL_newstate(); stack_guard(state, 0, || { - ffi::luaL_openlibs(state); + // Do not open the debug library, currently it can be used to cause unsafety. + ffi::luaL_requiref(state, cstr!("_G"), ffi::luaopen_base, 1); + ffi::luaL_requiref(state, cstr!("base"), ffi::luaopen_base, 1); + ffi::luaL_requiref(state, cstr!("coroutine"), ffi::luaopen_coroutine, 1); + ffi::luaL_requiref(state, cstr!("table"), ffi::luaopen_table, 1); + ffi::luaL_requiref(state, cstr!("io"), ffi::luaopen_io, 1); + ffi::luaL_requiref(state, cstr!("os"), ffi::luaopen_os, 1); + ffi::luaL_requiref(state, cstr!("string"), ffi::luaopen_string, 1); + ffi::luaL_requiref(state, cstr!("utf8"), ffi::luaopen_utf8, 1); + ffi::luaL_requiref(state, cstr!("math"), ffi::luaopen_math, 1); + ffi::luaL_requiref(state, cstr!("package"), ffi::luaopen_package, 1); + ffi::lua_pop(state, 10); // Create the userdata registry table @@ -1348,7 +1359,8 @@ impl Lua { ffi::lua_rawset(state, ffi::LUA_REGISTRYINDEX); - // Override pcall / xpcall + // Override pcall, xpcall, and setmetatable with versions that cannot be used to + // cause unsafety. ffi::lua_rawgeti(state, ffi::LUA_REGISTRYINDEX, ffi::LUA_RIDX_GLOBALS); @@ -1360,6 +1372,10 @@ impl Lua { ffi::lua_pushcfunction(state, safe_xpcall); ffi::lua_rawset(state, -3); + push_string(state, "setmetatable"); + ffi::lua_pushcfunction(state, safe_setmetatable); + ffi::lua_rawset(state, -3); + ffi::lua_pop(state, 1); }); diff --git a/src/tests.rs b/src/tests.rs index dfee102..13dc671 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -919,6 +919,33 @@ fn test_pcall_xpcall() { ); } +#[test] +fn test_setmetatable_gc() { + let lua = Lua::new(); + lua.exec::<()>( + r#" + val = nil + table = {} + setmetatable(table, { + __gc = function() + val = "gcwascalled" + end + }) + table_badgc = {} + setmetatable(table_badgc, { + __gc = "what's a gc" + }) + table = nil + table_badgc = nil + collectgarbage("collect") + "#, + None, + ).unwrap(); + + let globals = lua.globals(); + assert_eq!(globals.get::<_, String>("val").unwrap(), "gcwascalled"); +} + // Need to use compiletest-rs or similar to make sure these don't compile. /* #[test] diff --git a/src/util.rs b/src/util.rs index 1cd2a26..2acaabf 100644 --- a/src/util.rs +++ b/src/util.rs @@ -290,22 +290,19 @@ pub unsafe fn handle_error(state: *mut ffi::lua_State, err: c_int) -> Result<()> } ffi::LUA_ERRERR => Error::ErrorError(err_string), ffi::LUA_ERRMEM => { - // This is not impossible to hit, but this library is not set up - // to handle this properly. Lua does a longjmp on out of memory - // (like all lua errors), but it can do this from a huge number - // of lua functions, and it is extremely difficult to set up the - // pcall protection for every lua function that might allocate. - // If lua does this in an unprotected context, it will abort - // anyway, so the best we can do right now is guarantee an abort - // even in a protected context. - println!("Lua memory error, aborting!"); + // TODO: We should provide lua with custom allocators that guarantee aborting on + // OOM, instead of relying on the system allocator. Currently, this is a + // theoretical soundness bug, because an OOM could trigger a longjmp out of + // arbitrary rust code that is unprotectedly calling a lua function marked as + // potentially causing memory errors, which is most of them. + eprintln!("Lua memory error, aborting!"); process::abort() } ffi::LUA_ERRGCMM => { - // This should be impossible, or at least is indicative of an - // internal bug. Similarly to LUA_ERRMEM, this could indicate a - // longjmp out of rust code, so we just abort. - println!("Lua error during __gc, aborting!"); + // This should be impossible, since we wrap setmetatable to protect __gc + // metamethods, but if we do end up here then the same logic as setmetatable + // applies and we must abort. + eprintln!("Lua error during __gc, aborting!"); process::abort() } _ => lua_panic!(state, "internal error: unrecognized lua error code"), @@ -489,7 +486,44 @@ pub unsafe extern "C" fn safe_xpcall(state: *mut ffi::lua_State) -> c_int { } } -/// Does not call checkstack, uses 1 stack space +// Safely call setmetatable, if a __gc function is given, will wrap it in pcall, and panic on error. +pub unsafe extern "C" fn safe_setmetatable(state: *mut ffi::lua_State) -> c_int { + if ffi::lua_gettop(state) < 2 { + push_string(state, "not enough arguments to setmetatable"); + ffi::lua_error(state); + } + + // Wrapping the __gc method in setmetatable ONLY works because Lua 5.3 only honors the __gc + // method when it exists upon calling setmetatable, and ignores it if it is set later. + push_string(state, "__gc"); + if ffi::lua_rawget(state, -2) == ffi::LUA_TFUNCTION { + unsafe extern "C" fn safe_gc(state: *mut ffi::lua_State) -> c_int { + ffi::lua_pushvalue(state, ffi::lua_upvalueindex(1)); + ffi::lua_insert(state, 1); + if ffi::lua_pcall(state, 1, 0, 0) != ffi::LUA_OK { + // If a user supplied __gc metamethod causes an error, we must always abort. We may + // be inside a protected context due to being in a callback, but inside an + // unprotected ffi call that can cause memory errors, so may be at risk of + // longjmping over arbitrary rust. + eprintln!("Lua error during __gc, aborting!"); + process::abort() + } else { + ffi::lua_gettop(state) + } + } + + ffi::lua_pushcclosure(state, safe_gc, 1); + push_string(state, "__gc"); + ffi::lua_insert(state, -2); + ffi::lua_rawset(state, -3); + } else { + ffi::lua_pop(state, 1); + } + ffi::lua_setmetatable(state, -2); + 0 +} + +// Does not call checkstack, uses 1 stack space pub unsafe fn main_state(state: *mut ffi::lua_State) -> *mut ffi::lua_State { ffi::lua_rawgeti(state, ffi::LUA_REGISTRYINDEX, ffi::LUA_RIDX_MAINTHREAD); let main_state = ffi::lua_tothread(state, -1);