Reliable remote crash via assertion hitting in EVAL (via lua global env manipulation)

[re-posting in github after private reporting, as agreed with antirez]

Due to leaky whitelisting in LUA sandbox and contract violation in an assertion, it is possible to remotely crash redis (in a reliable way) via EVAL commands. This affects all released versions of redis in both 2.8 and 3.0 branches.

The actual bug is due to an incorrect check for existing key when storing a function to the redis dictionary. Normally, two identical LUA snippets are hashed to the same function name; the first one is stored into the dictionary, and the second call should just reuse it.

Looking at src/scripting.c, redis checks for function/snippet existence using the global environment:

    /* Try to lookup the Lua function */
    lua_getglobal(lua, funcname);
    if (lua_isnil(lua,-1)) {
        lua_pop(lua,1); /* remove the nil from the stack */
        /* Function not defined... let's define it if we have the
         * body of the function. If this is an EVALSHA call we can just
         * return an error. */
        if (evalsha) {
            lua_pop(lua,1); /* remove the error handler from the stack. */
            addReply(c, shared.noscripterr);
            return;
        }
        if (luaCreateFunction(c,lua,funcname,c->argv[1]) == REDIS_ERR) {
            lua_pop(lua,1); /* remove the error handler from the stack. */
            /* The error is sent to the client by luaCreateFunction()
             * itself when it returns REDIS_ERR. */
            return;
        }
        /* Now the following is guaranteed to return non nil */
        lua_getglobal(lua, funcname);
        redisAssert(!lua_isnil(lua,-1));
    }

However it is possible to de-synchronize redis dict and LUA global environment via maliciously crafted scripts.

In fact, a simple PoC can be constructed with two identical EVAL commands:

$ echo 'setfenv(0,{})' > /tmp/redis-crash.lua
$ redis-cli --eval /tmp/redis-crash.lua
(nil)
$ redis-cli --eval /tmp/redis-crash.lua
Error: Server closed the connection

This will always results in triggering an assertion (line numbers refer to current 2.8 branch):

 # === ASSERTION FAILED ===
 # ==> scripting.c:878 'retval == DICT_OK' is not true
 # (forcing SIGSEGV to print the bug report.)
 #     Redis 2.8.17 crashed by signal: 11
 #     Failed assertion: retval == DICT_OK (scripting.c:878)

During the first EVAL, $funcname doesn't exist yet so it is created and stored in LUA env and redis dict. When it is executed, it cleans itself from LUA global env, leaving a copy in redis dict. During the second EVAL, lua_getglobal() can not find it so luaCreateFunction() is called to create and store it, under the assumption that it doesn't exist yet. However, an object with that same $funcname is still stored in redis dict, thus making the dictAdd() return a DICT_ERR due an already existing key. This then triggers the assertion above.

While it may be tempting to hotfix this by just hiding the setfenv (which by the way has been removed in LUA 5.2), I think that with current sandboxing there are several other methods to reach that assertion (by manipulating the environment, or as an effect of cluster replication).

redis / redis

Reliable remote crash via assertion hitting in EVAL (via lua global env manipulation) #2853