From dc4d1bacdda07a6943a265e19ee0b0b3ed3abf97 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Fri, 26 Jul 2024 13:39:44 -0500 Subject: [PATCH] AP_Scripting: adjust string metatable setup to fix sandbox integrity In Lua, strings are the only type that come with a default metatable. The metatable must be shared by all string objects, and it is set to be the `string` library table each time that library is opened. In Ardupilot's scripting engine, the last script to load then has access to the string metatable as the library is opened fresh for each script, as its `string` library will have been set to the metatable. Therefore, if two scripts are loaded, A first and B second, and script B executes e.g. `string.byte = "haha"`, then `string.byte()` and `s:byte()` for script B are broken. Because the metatable is shared, this also breaks `s:byte()` for script A, which violates the integrity of the sandbox. Fix the issue by disabling the metatable setup functionality when the string libary is opened, then manually opening an additional copy of the library (which won't be given to any script) and setting it as the string metatable during intialization. This will break any script that modifies the string metatable for constructive purposes, but such a script could have been broken if it weren't the only script running anyway. --- libraries/AP_Scripting/lua/src/lstrlib.c | 4 ++++ libraries/AP_Scripting/lua_scripts.cpp | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/libraries/AP_Scripting/lua/src/lstrlib.c b/libraries/AP_Scripting/lua/src/lstrlib.c index 5f649b6f6f..99ea2477ed 100644 --- a/libraries/AP_Scripting/lua/src/lstrlib.c +++ b/libraries/AP_Scripting/lua/src/lstrlib.c @@ -1585,7 +1585,11 @@ static void createmetatable (lua_State *L) { */ LUAMOD_API int luaopen_string (lua_State *L) { luaL_newlib(L, strlib); +#if defined(ARDUPILOT_BUILD) + // metatable setup handled by Ardupilot scripting system +#else createmetatable(L); +#endif return 1; } diff --git a/libraries/AP_Scripting/lua_scripts.cpp b/libraries/AP_Scripting/lua_scripts.cpp index d9e410cb5a..a3e587c53e 100644 --- a/libraries/AP_Scripting/lua_scripts.cpp +++ b/libraries/AP_Scripting/lua_scripts.cpp @@ -499,6 +499,16 @@ void lua_scripts::run(void) { lua_atpanic(L, atpanic); load_generated_bindings(L); + // set up string metatable. we set up one for all scripts that no script has + // access to, as it's impossible to set up one per-script and we don't want + // any script to be able to mess with it. + lua_pushliteral(L, ""); /* dummy string */ + lua_createtable(L, 0, 1); /* table to be metatable for strings */ + luaopen_string(L); /* get string library */ + lua_setfield(L, -2, "__index"); /* metatable.__index = string */ + lua_setmetatable(L, -2); /* set table as metatable for strings */ + lua_pop(L, 1); /* pop dummy string */ + #ifndef HAL_CONSOLE_DISABLED const int loaded_mem = lua_gc(L, LUA_GCCOUNT, 0) * 1024 + lua_gc(L, LUA_GCCOUNTB, 0); DEV_PRINTF("Lua: State memory usage: %i + %i\n", inital_mem, loaded_mem - inital_mem);