Skip to content

Commit 666ed7f

Browse files
MeirShpilraienoranagra
authored andcommitted
Fix invalid memory write on lua stack overflow {CVE-2021-32626}
When LUA call our C code, by default, the LUA stack has room for 20 elements. In most cases, this is more than enough but sometimes it's not and the caller must verify the LUA stack size before he pushes elements. On 3 places in the code, there was no verification of the LUA stack size. On specific inputs this missing verification could have lead to invalid memory write: 1. On 'luaReplyToRedisReply', one might return a nested reply that will explode the LUA stack. 2. On 'redisProtocolToLuaType', 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) 3. On 'ldbRedis', one might give a command with enough arguments to    explode the LUA stack (all the arguments will be pushed to the LUA    stack) This commit is solving all those 3 issues by calling 'lua_checkstack' and verify that there is enough room in the LUA stack to push elements. In case 'lua_checkstack' returns an error (there is not enough room in the LUA stack and it's not possible to increase the stack), we will do the following: 1. On 'luaReplyToRedisReply', we will return an error to the user. 2. On 'redisProtocolToLuaType' we will exit with panic (we assume this scenario is rare because it can only happen with a module). 3. On 'ldbRedis', we return an error.
1 parent 6ac3c0b commit 666ed7f

File tree

1 file changed

+41
-0
lines changed

1 file changed

+41
-0
lines changed

Diff for: src/scripting.c

+41
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,16 @@ void sha1hex(char *digest, char *script, size_t len) {
128128
*/
129129

130130
char *redisProtocolToLuaType(lua_State *lua, char* reply) {
131+
132+
if (!lua_checkstack(lua, 5)) {
133+
/*
134+
* Increase the Lua stack if needed, to make sure there is enough room
135+
* to push 5 elements to the stack. On failure, exit with panic.
136+
         * Notice that we need, in the worst case, 5 elements because redisProtocolToLuaType_Aggregate
137+
         * might push 5 elements to the Lua stack.*/
138+
serverPanic("lua stack limit reach when parsing redis.call reply");
139+
}
140+
131141
char *p = reply;
132142

133143
switch(*p) {
@@ -220,6 +230,11 @@ char *redisProtocolToLuaType_Aggregate(lua_State *lua, char *reply, int atype) {
220230
if (atype == '%') {
221231
p = redisProtocolToLuaType(lua,p);
222232
} else {
233+
if (!lua_checkstack(lua, 1)) {
234+
/* Notice that here we need to check the stack again because the recursive
235+
* call to redisProtocolToLuaType might have use the room allocated in the stack */
236+
serverPanic("lua stack limit reach when parsing redis.call reply");
237+
}
223238
lua_pushboolean(lua,1);
224239
}
225240
lua_settable(lua,-3);
@@ -339,6 +354,17 @@ void luaSortArray(lua_State *lua) {
339354
/* Reply to client 'c' converting the top element in the Lua stack to a
340355
* Redis reply. As a side effect the element is consumed from the stack. */
341356
void luaReplyToRedisReply(client *c, lua_State *lua) {
357+
358+
if (!lua_checkstack(lua, 4)) {
359+
/* Increase the Lua stack if needed to make sure there is enough room
360+
* to push 4 elements to the stack. On failure, return error.
361+
         * Notice that we need, in the worst case, 4 elements because returning a map might
362+
* require push 4 elements to the Lua stack.*/
363+
addReplyErrorFormat(c, "reached lua stack limit");
364+
lua_pop(lua,1); // pop the element from the stack
365+
return;
366+
}
367+
342368
int t = lua_type(lua,-1);
343369

344370
switch(t) {
@@ -362,6 +388,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
362388
* field. */
363389

364390
/* Handle error reply. */
391+
// we took care of the stack size on function start
365392
lua_pushstring(lua,"err");
366393
lua_gettable(lua,-2);
367394
t = lua_type(lua,-1);
@@ -407,6 +434,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
407434
if (t == LUA_TTABLE) {
408435
int maplen = 0;
409436
void *replylen = addReplyDeferredLen(c);
437+
/* we took care of the stack size on function start */
410438
lua_pushnil(lua); /* Use nil to start iteration. */
411439
while (lua_next(lua,-2)) {
412440
/* Stack now: table, key, value */
@@ -429,6 +457,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
429457
if (t == LUA_TTABLE) {
430458
int setlen = 0;
431459
void *replylen = addReplyDeferredLen(c);
460+
/* we took care of the stack size on function start */
432461
lua_pushnil(lua); /* Use nil to start iteration. */
433462
while (lua_next(lua,-2)) {
434463
/* Stack now: table, key, true */
@@ -448,6 +477,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
448477
void *replylen = addReplyDeferredLen(c);
449478
int j = 1, mbulklen = 0;
450479
while(1) {
480+
/* we took care of the stack size on function start */
451481
lua_pushnumber(lua,j++);
452482
lua_gettable(lua,-2);
453483
t = lua_type(lua,-1);
@@ -2506,6 +2536,17 @@ void ldbEval(lua_State *lua, sds *argv, int argc) {
25062536
void ldbRedis(lua_State *lua, sds *argv, int argc) {
25072537
int j, saved_rc = server.lua_replicate_commands;
25082538

2539+
if (!lua_checkstack(lua, argc + 1)) {
2540+
/* Increase the Lua stack if needed to make sure there is enough room
2541+
* to push 'argc + 1' elements to the stack. On failure, return error.
2542+
         * Notice that we need, in worst case, 'argc + 1' elements because we push all the arguments
2543+
         * given by the user (without the first argument) and we also push the 'redis' global table and
2544+
         * 'redis.call' function so:
2545+
         * (1 (redis table)) + (1 (redis.call function)) + (argc - 1 (all arguments without the first)) = argc + 1*/
2546+
ldbLogRedisReply("max lua stack reached");
2547+
return;
2548+
}
2549+
25092550
lua_getglobal(lua,"redis");
25102551
lua_pushstring(lua,"call");
25112552
lua_gettable(lua,-2); /* Stack: redis, redis.call */

0 commit comments

Comments
 (0)