disallow loading lua bytecode via load/dofile (CVE-2018-1999023)
This could otherwise be used to escape the lua sandbox, as described in
multiple sources. For example one can use it to reenable the os.execute
function to do shell commands
The affected functions were
load,loadstring,wesnoth.dofile,wesnoth.require and various places in the
wesnoth source where lua chunks were loaded for example by the ai code.
This commit also changes the lua source to change luas load (which is
the same as loadstring), alternatively we could add a wrapper around the
original load function that always passes "t" as third parameter, i went
this way mostly because it was easier to implement, but also because i
was not 100% sure that is is impossible to query the upvalues of a
function via lua (wesnoth disables debug.getupvalue but still).
There is also an occurance in the application_lua_kernel that was not fixed
because i assumed that umc cannot contian application lua scipts.
As further security measure we might want to disable printing the function
adress in luas tostring for c functions, this cannot be exploited by itself
but it can be used to defeat ASLR in some cases.
(cherry-picked from commit 2554c166dd
)
This commit is contained in:
parent
8668050971
commit
120e8d9911
6 changed files with 12 additions and 6 deletions
|
@ -42,6 +42,8 @@
|
|||
### Miscellaneous and bug fixes
|
||||
|
||||
## Version 1.14.3+dev
|
||||
### Security Fixes
|
||||
* Fixed lua can escape sandbox through load/loadstring (CVE-2018-1999023).
|
||||
### Add-ons server
|
||||
* Made it so plain-text .po catalogues in add-ons are detected and added to
|
||||
the list of translations for them.
|
||||
|
|
|
@ -994,7 +994,7 @@ static size_t generate_and_push_ai_state(lua_State* L, ai::engine_lua* engine)
|
|||
|
||||
lua_ai_context* lua_ai_context::create(lua_State *L, char const *code, ai::engine_lua *engine)
|
||||
{
|
||||
int res_ai = luaL_loadstring(L, code); // [-1: AI code]
|
||||
int res_ai = luaL_loadbufferx(L, code, strlen(code), /*name*/ code, "t"); // [-1: AI code]
|
||||
if (res_ai != 0)
|
||||
{
|
||||
|
||||
|
@ -1036,7 +1036,7 @@ void lua_ai_context::update_state()
|
|||
|
||||
lua_ai_action_handler* lua_ai_action_handler::create(lua_State *L, char const *code, lua_ai_context &context)
|
||||
{
|
||||
int res = luaL_loadstring(L, code);//stack size is now 1 [ -1: f]
|
||||
int res = luaL_loadbufferx(L, code, strlen(code), /*name*/ code, "t");//stack size is now 1 [ -1: f]
|
||||
if (res)
|
||||
{
|
||||
char const *m = lua_tostring(L, -1);
|
||||
|
|
|
@ -336,16 +336,17 @@ static int luaB_load (lua_State *L) {
|
|||
size_t l;
|
||||
const char *s = lua_tolstring(L, 1, &l);
|
||||
const char *mode = luaL_optstring(L, 3, "bt");
|
||||
(void) mode;
|
||||
int env = (!lua_isnone(L, 4) ? 4 : 0); /* 'env' index or 0 if no 'env' */
|
||||
if (s != NULL) { /* loading a string? */
|
||||
const char *chunkname = luaL_optstring(L, 2, s);
|
||||
status = luaL_loadbufferx(L, s, l, chunkname, mode);
|
||||
status = luaL_loadbufferx(L, s, l, chunkname, "t");
|
||||
}
|
||||
else { /* loading from a reader function */
|
||||
const char *chunkname = luaL_optstring(L, 2, "=(load)");
|
||||
luaL_checktype(L, 1, LUA_TFUNCTION);
|
||||
lua_settop(L, RESERVEDSLOT); /* create reserved slot */
|
||||
status = lua_load(L, generic_reader, NULL, chunkname, mode);
|
||||
status = lua_load(L, generic_reader, NULL, chunkname, "t");
|
||||
}
|
||||
return load_aux(L, status, env);
|
||||
}
|
||||
|
|
|
@ -171,6 +171,7 @@ application_lua_kernel::thread * application_lua_kernel::load_script_from_string
|
|||
DBG_LUA << "created thread: status = " << lua_status(T) << (lua_status(T) == LUA_OK ? " == OK" : " == ?") << "\n";
|
||||
DBG_LUA << "loading script from string:\n<<\n" << prog << "\n>>\n";
|
||||
|
||||
// note: this is unsafe for umc as it allows loading lua baytecode, but umc cannot add application lua kernel scipts.
|
||||
int errcode = luaL_loadstring(T, prog.c_str());
|
||||
if (errcode != LUA_OK) {
|
||||
const char * err_str = lua_tostring(T, -1);
|
||||
|
|
|
@ -217,7 +217,7 @@ public:
|
|||
//lua uses '@' to know that this is a file (as opposed to something loaded via loadstring )
|
||||
std::string chunkname = '@' + relativename;
|
||||
LOG_LUA << "starting to read from " << fname << "\n";
|
||||
return lua_load(L, &lua_filestream::lua_read_data, &lfs, chunkname.c_str(), nullptr);
|
||||
return lua_load(L, &lua_filestream::lua_read_data, &lfs, chunkname.c_str(), "t");
|
||||
}
|
||||
private:
|
||||
char buff_[LUAL_BUFFERSIZE];
|
||||
|
|
|
@ -675,7 +675,9 @@ bool lua_kernel_base::protected_call(lua_State * L, int nArgs, int nRets, error_
|
|||
|
||||
bool lua_kernel_base::load_string(char const * prog, error_handler e_h)
|
||||
{
|
||||
int errcode = luaL_loadstring(mState, prog);
|
||||
// pass 't' to prevent loading bytecode which is unsafe and can be used to escape the sandbox.
|
||||
// todo: maybe allow a 'name' parameter to give better error messages.
|
||||
int errcode = luaL_loadbufferx(mState, prog, strlen(prog), /*name*/ prog, "t");
|
||||
if (errcode != LUA_OK) {
|
||||
char const * msg = lua_tostring(mState, -1);
|
||||
std::string message = msg ? msg : "null string";
|
||||
|
|
Loading…
Add table
Reference in a new issue