Commit e1b17f12 authored by Meir Shpilraien (Spielrein)'s avatar Meir Shpilraien (Spielrein) Committed by Oran Agra
Browse files

Fix #11030, use lua_rawget to avoid triggering metatables and crash. (#11032)

Fix #11030, use lua_rawget to avoid triggering metatables.

#11030 shows how return `_G` from the Lua script (either function or eval), cause the
Lua interpreter to Panic and the Redis processes to exit with error code 1.
Though return `_G` only panic on Redis 7 and 6.2.7, the underline issue exists on older
versions as well (6.0 and 6.2). The underline issue is returning a table with a metatable
such that the metatable raises an error.

The following example demonstrate the issue:
```
127.0.0.1:6379> eval "local a = {}; setmetatable(a,{__index=function() foo() end}) return a" 0
Error: Server closed the connection
```
```
PANIC: unprotected error in call to Lua API (user_script:1: Script attempted to access nonexistent global variable 'foo')
```

The Lua panic happened because when returning the result to the client, Redis needs to
introspect the returning table and transform the table into a resp. In order to scan the table,
Re...
parent 8c76e0f2
...@@ -468,7 +468,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { ...@@ -468,7 +468,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
/* Handle error reply. */ /* Handle error reply. */
/* we took care of the stack size on function start */ /* we took care of the stack size on function start */
lua_pushstring(lua,"err"); lua_pushstring(lua,"err");
lua_gettable(lua,-2); lua_rawget(lua,-2);
t = lua_type(lua,-1); t = lua_type(lua,-1);
if (t == LUA_TSTRING) { if (t == LUA_TSTRING) {
addReplyErrorFormat(c,"-%s",lua_tostring(lua,-1)); addReplyErrorFormat(c,"-%s",lua_tostring(lua,-1));
...@@ -479,7 +479,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { ...@@ -479,7 +479,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
/* Handle status reply. */ /* Handle status reply. */
lua_pushstring(lua,"ok"); lua_pushstring(lua,"ok");
lua_gettable(lua,-2); lua_rawget(lua,-2);
t = lua_type(lua,-1); t = lua_type(lua,-1);
if (t == LUA_TSTRING) { if (t == LUA_TSTRING) {
sds ok = sdsnew(lua_tostring(lua,-1)); sds ok = sdsnew(lua_tostring(lua,-1));
...@@ -493,7 +493,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { ...@@ -493,7 +493,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
/* Handle double reply. */ /* Handle double reply. */
lua_pushstring(lua,"double"); lua_pushstring(lua,"double");
lua_gettable(lua,-2); lua_rawget(lua,-2);
t = lua_type(lua,-1); t = lua_type(lua,-1);
if (t == LUA_TNUMBER) { if (t == LUA_TNUMBER) {
addReplyDouble(c,lua_tonumber(lua,-1)); addReplyDouble(c,lua_tonumber(lua,-1));
...@@ -504,7 +504,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { ...@@ -504,7 +504,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
/* Handle map reply. */ /* Handle map reply. */
lua_pushstring(lua,"map"); lua_pushstring(lua,"map");
lua_gettable(lua,-2); lua_rawget(lua,-2);
t = lua_type(lua,-1); t = lua_type(lua,-1);
if (t == LUA_TTABLE) { if (t == LUA_TTABLE) {
int maplen = 0; int maplen = 0;
...@@ -527,7 +527,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { ...@@ -527,7 +527,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
/* Handle set reply. */ /* Handle set reply. */
lua_pushstring(lua,"set"); lua_pushstring(lua,"set");
lua_gettable(lua,-2); lua_rawget(lua,-2);
t = lua_type(lua,-1); t = lua_type(lua,-1);
if (t == LUA_TTABLE) { if (t == LUA_TTABLE) {
int setlen = 0; int setlen = 0;
...@@ -554,7 +554,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { ...@@ -554,7 +554,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
while(1) { while(1) {
/* we took care of the stack size on function start */ /* we took care of the stack size on function start */
lua_pushnumber(lua,j++); lua_pushnumber(lua,j++);
lua_gettable(lua,-2); lua_rawget(lua,-2);
t = lua_type(lua,-1); t = lua_type(lua,-1);
if (t == LUA_TNIL) { if (t == LUA_TNIL) {
lua_pop(lua,1); lua_pop(lua,1);
......
...@@ -3,6 +3,20 @@ start_server {tags {"scripting"}} { ...@@ -3,6 +3,20 @@ start_server {tags {"scripting"}} {
r eval {return 'hello'} 0 r eval {return 'hello'} 0
} {hello} } {hello}
test {EVAL - Return _G} {
r eval {return _G} 0
} {}
test {EVAL - Return table with a metatable that raise error} {
r eval {local a = {}; setmetatable(a,{__index=function() foo() end}) return a} 0
} {}
test {EVAL - Return table with a metatable that call redis} {
r eval {local a = {}; setmetatable(a,{__index=function() redis.call('set', 'x', '1') end}) return a} 0
# make sure x was not set
r get x
} {}
test {EVAL - Lua integer -> Redis protocol type conversion} { test {EVAL - Lua integer -> Redis protocol type conversion} {
r eval {return 100.5} 0 r eval {return 100.5} 0
} {100} } {100}
......
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