Skip to content

Commit

Permalink
Lua: implement our own load() instead of monkey-patching Lua code
Browse files Browse the repository at this point in the history
Monkey-patching has multiple problems. The biggest problem for a security
fix like this is that it's way too easy to forget to re-apply when we
update Lua to a newer version.

Instead, we now have the implementation of load() under our control and can
update Lua without risk of reintroducing CVE-2018-1999023.

(cherry-picked from commit 52ae31e)
  • Loading branch information
jyrkive committed Oct 7, 2018
1 parent a414703 commit eb36c60
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/lua/lbaselib.cpp
Expand Up @@ -336,17 +336,16 @@ 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, "t");
status = luaL_loadbufferx(L, s, l, chunkname, mode);
}
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, "t");
status = lua_load(L, generic_reader, NULL, chunkname, mode);
}
return load_aux(L, status, env);
}
Expand Down
41 changes: 41 additions & 0 deletions src/scripting/lua_kernel_base.cpp
Expand Up @@ -125,6 +125,44 @@ int lua_kernel_base::intf_print(lua_State* L)
return 0;
}

/**
* Replacement load function. Mostly the same as regular load, but disallows loading binary chunks
* due to CVE-2018-1999023.
*/
static int intf_load(lua_State* L)
{
std::string chunk = luaL_checkstring(L, 1);
const char* name = luaL_optstring(L, 2, chunk.c_str());
std::string mode = luaL_optstring(L, 3, "t");
bool override_env = !lua_isnone(L, 4);

if(mode != "t") {
return luaL_argerror(L, 3, "binary chunks are not allowed for security reasons");
}

int result = luaL_loadbufferx(L, chunk.data(), chunk.length(), name, "t");
if(result != LUA_OK) {
lua_pushnil(L);
// Move the nil as the first return value, like Lua's own load() does.
lua_insert(L, -2);

return 2;
}

if(override_env) {
// Copy "env" to the top of the stack.
lua_pushvalue(L, 4);
// Set "env" as the first upvalue.
const char* upvalue_name = lua_setupvalue(L, -2, 1);
if(upvalue_name == nullptr) {
// lua_setupvalue() didn't remove the copy of "env" from the stack, so we need to do it ourselves.
lua_pop(L, 1);
}
}

return 1;
}

// The show lua console callback is similarly a method of lua kernel
int lua_kernel_base::intf_show_lua_console(lua_State *L)
{
Expand Down Expand Up @@ -511,6 +549,9 @@ lua_kernel_base::lua_kernel_base()
lua_pushcfunction(L, &dispatch<&lua_kernel_base::intf_print>);
lua_setglobal(L, "print");

lua_pushcfunction(L, intf_load);
lua_setglobal(L, "load");

cmd_log_ << "Initializing package repository...\n";
// Create the package table.
lua_getglobal(L, "wesnoth");
Expand Down

0 comments on commit eb36c60

Please sign in to comment.