From b8e82d205b010c2d8c89457ad69e25b1ba620fd0 Mon Sep 17 00:00:00 2001 From: "Meir Shpilraien (Spielrein)" Date: Tue, 30 Nov 2021 12:27:05 +0200 Subject: [PATCH] Swap '\r\n' with spaces when returning a big number reply from Lua script. (#9870) The issue can only happened with a bad Lua script that claims to return a big number while actually return data which is not a big number (contains chars that are not digits). Such thing will not cause an issue unless the big number value contains `\r\n` and then it messes the resp3 structure. The fix changes all the appearances of '\r\n' with spaces. Such an issue can also happened on simple string or error replies but those already handle it the same way this PR does (replace `\r\n` with spaces). Other replies type are not vulnerable to this issue because they are not counting on free text that is terminated with `\r\n` (either it contains the bulk length like string reply or they are typed reply that can not inject free text like boolean or number). The issue only exists on unstable branch, big number reply on Lua script was not yet added to any official release. --- src/scripting.c | 5 ++++- tests/unit/scripting.tcl | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/scripting.c b/src/scripting.c index 3fc804a96..f27840956 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -596,7 +596,10 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { lua_gettable(lua,-2); t = lua_type(lua,-1); if (t == LUA_TSTRING) { - addReplyBigNum(c,(char*)lua_tostring(lua,-1),lua_strlen(lua,-1)); + sds big_num = sdsnewlen(lua_tostring(lua,-1), lua_strlen(lua,-1)); + sdsmapchars(big_num,"\r\n"," ",2); + addReplyBigNum(c,big_num,sdslen(big_num)); + sdsfree(big_num); lua_pop(lua,2); return; } diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index be7147d35..68c819aa0 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -1009,6 +1009,17 @@ start_server {tags {"scripting resp3 needs:debug"}} { } } + test {test resp3 malformed big number protocol parsing} { + set ret [r eval "return {big_number='123\\r\\n123'}" 0] + if {$client_proto == 2} { + # if either Lua or the clien is RESP2 the reply will be RESP2 + assert_equal $ret {$8} + assert_equal [r read] {123 123} + } else { + assert_equal $ret {(123 123} + } + } + test {test resp3 map protocol parsing} { set ret [r eval "redis.setresp($i);return redis.call('debug', 'protocol', 'map')" 0] if {$client_proto == 2 || $i == 2} {