Commit b2ce3719 authored by meir's avatar meir Committed by Oran Agra
Browse files

Protect globals of evals scripts.

Use the new `lua_enablereadonlytable` Lua API to protect the global tables of
evals scripts. The implemetation is easy, we simply call `lua_enablereadonlytable`
on the global table to turn it into a readonly table.
parent 21414ad4
...@@ -1086,43 +1086,39 @@ void luaRemoveUnsupportedFunctions(lua_State *lua) { ...@@ -1086,43 +1086,39 @@ void luaRemoveUnsupportedFunctions(lua_State *lua) {
lua_setglobal(lua,"dofile"); lua_setglobal(lua,"dofile");
} }
/* This function installs metamethods in the global table _G that prevent static int luaProtectedTableError(lua_State *lua) {
* the creation of globals accidentally. int argc = lua_gettop(lua);
if (argc != 2) {
serverLog(LL_WARNING, "malicious code trying to call luaProtectedTableError with wrong arguments");
luaL_error(lua, "Wrong number of arguments to luaProtectedTableError");
}
if (!lua_isstring(lua, -1) && !lua_isnumber(lua, -1)) {
luaL_error(lua, "Second argument to luaProtectedTableError must be a string or number");
}
const char* variable_name = lua_tostring(lua, -1);
luaL_error(lua, "Script attempted to access nonexistent global variable '%s'", variable_name);
return 0;
}
/* Set global protection on table on the top of the stack.
* After called, it will no longer be possible to set
* new items on the table. Any attempt to access a protected
* table for write will result in an error. Any attempt to
* get a none existing element from a locked table will result
* in an error.
* *
* It should be the last to be called in the scripting engine initialization * The function assumes the Lua stack have a least enough
* sequence, because it may interact with creation of globals. */ * space to push 2 element, its up to the caller to verify
void scriptingEnableGlobalsProtection(lua_State *lua) { * this before calling this function. */
char *s[32]; void luaSetGlobalProtection(lua_State *lua) {
sds code = sdsempty(); lua_newtable(lua); /* push metatable */
int j = 0; lua_pushcfunction(lua, luaProtectedTableError); /* push get error handler */
lua_setfield(lua, -2, "__index");
lua_enablereadonlytable(lua, -1, 1); /* protect the metatable */
/* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html. lua_setmetatable(lua, -2);
* Modified to be adapted to Redis. */
s[j++]="local dbg=debug\n"; lua_enablereadonlytable(lua, -1, 1);
s[j++]="local mt = {}\n";
s[j++]="setmetatable(_G, mt)\n";
s[j++]="mt.__newindex = function (t, n, v)\n";
s[j++]=" if dbg.getinfo(2) then\n";
s[j++]=" local w = dbg.getinfo(2, \"S\").what\n";
s[j++]=" if w ~= \"C\" then\n";
s[j++]=" error(\"Script attempted to create global variable '\"..tostring(n)..\"'\", 2)\n";
s[j++]=" end\n";
s[j++]=" end\n";
s[j++]=" rawset(t, n, v)\n";
s[j++]="end\n";
s[j++]="mt.__index = function (t, n)\n";
s[j++]=" if dbg.getinfo(2) and dbg.getinfo(2, \"S\").what ~= \"C\" then\n";
s[j++]=" error(\"Script attempted to access nonexistent global variable '\"..tostring(n)..\"'\", 2)\n";
s[j++]=" end\n";
s[j++]=" return rawget(t, n)\n";
s[j++]="end\n";
s[j++]="debug = nil\n";
s[j++]=NULL;
for (j = 0; s[j] != NULL; j++) code = sdscatlen(code,s[j],strlen(s[j]));
luaL_loadbuffer(lua,code,sdslen(code),"@enable_strict_lua");
lua_pcall(lua,0,0,0);
sdsfree(code);
} }
/* Initialize the scripting environment. /* Initialize the scripting environment.
...@@ -1308,10 +1304,10 @@ void scriptingInit(int setup) { ...@@ -1308,10 +1304,10 @@ void scriptingInit(int setup) {
server.lua_client->flags |= CLIENT_DENY_BLOCKING; server.lua_client->flags |= CLIENT_DENY_BLOCKING;
} }
/* Lua beginners often don't use "local", this is likely to introduce /* Lock the global table from any changes */
* subtle bugs in their code. To prevent problems we protect accesses lua_pushvalue(lua, LUA_GLOBALSINDEX);
* to global variables. */ luaSetGlobalProtection(lua);
scriptingEnableGlobalsProtection(lua); lua_pop(lua, 1);
server.lua = lua; server.lua = lua;
} }
...@@ -1342,7 +1338,9 @@ void luaSetGlobalArray(lua_State *lua, char *var, robj **elev, int elec) { ...@@ -1342,7 +1338,9 @@ void luaSetGlobalArray(lua_State *lua, char *var, robj **elev, int elec) {
lua_pushlstring(lua,(char*)elev[j]->ptr,sdslen(elev[j]->ptr)); lua_pushlstring(lua,(char*)elev[j]->ptr,sdslen(elev[j]->ptr));
lua_rawseti(lua,-2,j+1); lua_rawseti(lua,-2,j+1);
} }
lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 0);
lua_setglobal(lua,var); lua_setglobal(lua,var);
lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 1);
} }
/* --------------------------------------------------------------------------- /* ---------------------------------------------------------------------------
......
...@@ -390,7 +390,7 @@ start_server {tags {"scripting"}} { ...@@ -390,7 +390,7 @@ start_server {tags {"scripting"}} {
test {Globals protection setting an undeclared global*} { test {Globals protection setting an undeclared global*} {
catch {r eval {a=10} 0} e catch {r eval {a=10} 0} e
set e set e
} {*ERR*attempted to create global*} } {ERR*Attempt to modify a readonly table*}
test {Test an example script DECR_IF_GT} { test {Test an example script DECR_IF_GT} {
set decr_if_gt { set decr_if_gt {
...@@ -595,6 +595,43 @@ start_server {tags {"scripting"}} { ...@@ -595,6 +595,43 @@ start_server {tags {"scripting"}} {
set res [r eval {redis.setresp(2); return redis.call('hgetall', KEYS[1])} 1 hash] set res [r eval {redis.setresp(2); return redis.call('hgetall', KEYS[1])} 1 hash]
assert_equal $res $expected_list assert_equal $res $expected_list
} }
test "Try trick global protection 1" {
catch {
r eval {
setmetatable(_G, {})
} 0
} e
set _ $e
} {*Attempt to modify a readonly table*}
test "Try trick global protection 2" {
catch {
r eval {
local g = getmetatable(_G)
g.__index = {}
} 0
} e
set _ $e
} {*Attempt to modify a readonly table*}
test "Try trick global protection 3" {
catch {
r eval {
redis = function() return 1 end
} 0
} e
set _ $e
} {*Attempt to modify a readonly table*}
test "Try trick global protection 4" {
catch {
r eval {
_G = {}
} 0
} e
set _ $e
} {*Attempt to modify a readonly table*}
} }
# Start a new server since the last test in this stanza will kill the # Start a new server since the last test in this stanza will kill the
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment