Unverified Commit 6b0b04f1 authored by Meir Shpilraien (Spielrein)'s avatar Meir Shpilraien (Spielrein) Committed by GitHub
Browse files

Clean Lua stack before parsing call reply to avoid crash on a call with many arguments (#9809)

This commit 0f8b634c (CVE-2021-32626 released in 6.2.6, 6.0.16, 5.0.14)
fixes an invalid memory write issue by using `lua_checkstack` API to make
sure the Lua stack is not overflow. This fix was added on 3 places:
1. `luaReplyToRedisReply`
2. `ldbRedis`
3. `redisProtocolToLuaType`

On the first 2 functions, `lua_checkstack` is handled gracefully while the
last is handled with an assert and a statement that this situation can
not happened (only with misbehave module):

> the Redis reply might be deep enough to explode the LUA stack (notice
that currently there is no such command in Redis that returns such a nested
reply, but modules might do it)

The issue that was discovered is that user arguments is also considered part
of the stack, and so the following script (for example) make the assertion reachable:
```
local a = {}
for i=1,7999 do
    a[i] = 1
end 
return redis.call("lpush", "l", unpack(a))
```

This is a regression because such a script would have worked before and now
its crashing Redis. The solution is to clear the function arguments from the Lua
stack which makes the original assumption true and the assertion unreachable.
parent a8c1253b
...@@ -790,6 +790,10 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) { ...@@ -790,6 +790,10 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
return raise_error ? luaRaiseError(lua) : 1; return raise_error ? luaRaiseError(lua) : 1;
} }
/* Pop all arguments from the stack, we do not need them anymore
* and this way we guaranty we will have room on the stack for the result. */
lua_pop(lua, argc);
/* Setup our fake client for command execution */ /* Setup our fake client for command execution */
c->argv = argv; c->argv = argv;
c->argc = argc; c->argc = argc;
......
...@@ -622,6 +622,16 @@ start_server {tags {"scripting"}} { ...@@ -622,6 +622,16 @@ start_server {tags {"scripting"}} {
# make sure the connection is still valid # make sure the connection is still valid
assert_equal [r ping] {PONG} assert_equal [r ping] {PONG}
} }
test {Script check unpack with massive arguments} {
r eval {
local a = {}
for i=1,7999 do
a[i] = 1
end
return redis.call("lpush", "l", unpack(a))
} 0
} {7999}
} }
# 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