summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMeir Shpilraien (Spielrein) <meir@redis.com>2021-11-28 11:59:39 +0200
committerOran Agra <oran@redislabs.com>2023-01-17 14:59:41 +0200
commita511af7c5e4c274925a2fa707c44e3e07ba9bee1 (patch)
treefdc308870f3d9e56ae742d2a3287ed0fd379b361
parent2adbbbcdc0797ff9bb8f21e7fb65fea943bd00b5 (diff)
downloadredis-a511af7c5e4c274925a2fa707c44e3e07ba9bee1.tar.gz
Clean Lua stack before parsing call reply to avoid crash on a call with many arguments (#9809)
This commit 0f8b634cd (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. (cherry picked from commit 6b0b04f1b265c429bd19d6c99c9e7e2921723601)
-rw-r--r--src/scripting.c4
-rw-r--r--tests/unit/scripting.tcl10
2 files changed, 14 insertions, 0 deletions
diff --git a/src/scripting.c b/src/scripting.c
index 5c96a3604..c5393c16e 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -589,6 +589,10 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
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 */
c->argv = argv;
c->argc = argc;
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index df3909e89..8314f0268 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -557,6 +557,16 @@ start_server {tags {"scripting"}} {
set res [r eval {redis.setresp(2); return redis.call('hgetall', KEYS[1])} 1 hash]
assert_equal $res $expected_list
}
+
+ 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