Add missing lua_pop in luaGetFromRegistry (#11097)

This pr mainly has the following four changes:

1. Add missing lua_pop in `luaGetFromRegistry`.
    This bug affects `redis.register_function`, where `luaGetFromRegistry` in
    `luaRegisterFunction` will return null when we call `redis.register_function` nested.
    .e.g
    ```
    FUNCTION LOAD "#!lua name=mylib \n local lib=redis \n lib.register_function('f2', function(keys, args) lib.register_function('f1', function () end) end)"
    fcall f2 0
    ````
    But since we exit when luaGetFromRegistry returns null, it does not cause the stack to grow indefinitely.

3. When getting `REGISTRY_RUN_CTX_NAME` from the registry, use `serverAssert`
    instead of error return. Since none of these lua functions are registered at the time
    of function load, scriptRunCtx will never be NULL.
4. Add `serverAssert` for `luaLdbLineHook`, `luaEngineLoadHook`.
5. Remove `luaGetFromRegistry` from `redis_math_random` and
    `redis_math_randomseed`, it looks like they are redundant.

src/eval.c

@@ -1629,6 +1629,7 @@ ldbLog(sdsnew("                     next line of code."));
  * to start executing a new line. */
 void luaLdbLineHook(lua_State *lua, lua_Debug *ar) {
     scriptRunCtx* rctx = luaGetFromRegistry(lua, REGISTRY_RUN_CTX_NAME);
+    serverAssert(rctx); /* Only supported inside script invocation */
     lua_getstack(lua,0,ar);
     lua_getinfo(lua,"Sl",ar);
     ldb.currentline = ar->currentline;

src/function_lua.c

@@ -83,6 +83,7 @@ typedef struct registerFunctionArgs {
 static void luaEngineLoadHook(lua_State *lua, lua_Debug *ar) {
     UNUSED(ar);
     loadCtx *load_ctx = luaGetFromRegistry(lua, REGISTRY_LOAD_CTX_NAME);
+    serverAssert(load_ctx); /* Only supported inside script invocation */
     uint64_t duration = elapsedMs(load_ctx->start_time);
     if (duration > LOAD_TIMEOUT_MS) {
         lua_sethook(lua, luaEngineLoadHook, LUA_MASKLINE, 0);

src/script_lua.c

@@ -173,6 +173,7 @@ void* luaGetFromRegistry(lua_State* lua, const char* name) {
     lua_gettable(lua, LUA_REGISTRYINDEX);
 
     if (lua_isnil(lua, -1)) {
+        lua_pop(lua, 1); /* pops the value */
         return NULL;
     }
     /* must be light user data */
@@ -838,10 +839,7 @@ static robj **luaArgsToRedisArgv(lua_State *lua, int *argc) {
 static int luaRedisGenericCommand(lua_State *lua, int raise_error) {
     int j;
     scriptRunCtx* rctx = luaGetFromRegistry(lua, REGISTRY_RUN_CTX_NAME);
-    if (!rctx) {
-        luaPushError(lua, "redis.call/pcall can only be called inside a script invocation");
-        return luaError(lua);
-    }
+    serverAssert(rctx); /* Only supported inside script invocation */
     sds err = NULL;
     client* c = rctx->c;
     sds reply;
@@ -1052,10 +1050,7 @@ static int luaRedisSetReplCommand(lua_State *lua) {
     int flags, argc = lua_gettop(lua);
 
     scriptRunCtx* rctx = luaGetFromRegistry(lua, REGISTRY_RUN_CTX_NAME);
-    if (!rctx) {
-        luaPushError(lua, "redis.set_repl can only be called inside a script invocation");
-        return luaError(lua);
-    }
+    serverAssert(rctx); /* Only supported inside script invocation */
 
     if (argc != 1) {
         luaPushError(lua, "redis.set_repl() requires two arguments.");
@@ -1077,10 +1072,7 @@ static int luaRedisSetReplCommand(lua_State *lua) {
  * Checks ACL permissions for given command for the current user. */
 static int luaRedisAclCheckCmdPermissionsCommand(lua_State *lua) {
     scriptRunCtx* rctx = luaGetFromRegistry(lua, REGISTRY_RUN_CTX_NAME);
-    if (!rctx) {
-        luaPushError(lua, "redis.acl_check_cmd can only be called inside a script invocation");
-        return luaError(lua);
-    }
+    serverAssert(rctx); /* Only supported inside script invocation */
     int raise_error = 0;
 
     int argc;
@@ -1152,10 +1144,7 @@ static int luaLogCommand(lua_State *lua) {
 /* redis.setresp() */
 static int luaSetResp(lua_State *lua) {
     scriptRunCtx* rctx = luaGetFromRegistry(lua, REGISTRY_RUN_CTX_NAME);
-    if (!rctx) {
-        luaPushError(lua, "redis.setresp can only be called inside a script invocation");
-        return luaError(lua);
-    }
+    serverAssert(rctx); /* Only supported inside script invocation */
     int argc = lua_gettop(lua);
 
     if (argc != 1) {
@@ -1481,11 +1470,6 @@ static void luaCreateArray(lua_State *lua, robj **elev, int elec) {
 /* The following implementation is the one shipped with Lua itself but with
  * rand() replaced by redisLrand48(). */
 static int redis_math_random (lua_State *L) {
-  scriptRunCtx* rctx = luaGetFromRegistry(L, REGISTRY_RUN_CTX_NAME);
-  if (!rctx) {
-    return luaL_error(L, "math.random can only be called inside a script invocation");
-  }
-
   /* the `%' avoids the (rare) case of r==1, and is needed also because on
      some systems (SunOS!) `rand()' may return a value larger than RAND_MAX */
   lua_Number r = (lua_Number)(redisLrand48()%REDIS_LRAND48_MAX) /
@@ -1514,10 +1498,6 @@ static int redis_math_random (lua_State *L) {
 }
 
 static int redis_math_randomseed (lua_State *L) {
-  scriptRunCtx* rctx = luaGetFromRegistry(L, REGISTRY_RUN_CTX_NAME);
-  if (!rctx) {
-    return luaL_error(L, "math.randomseed can only be called inside a script invocation");
-  }
   redisSrand48(luaL_checkint(L, 1));
   return 0;
 }
@@ -1526,6 +1506,7 @@ static int redis_math_randomseed (lua_State *L) {
 static void luaMaskCountHook(lua_State *lua, lua_Debug *ar) {
     UNUSED(ar);
     scriptRunCtx* rctx = luaGetFromRegistry(lua, REGISTRY_RUN_CTX_NAME);
+    serverAssert(rctx); /* Only supported inside script invocation */
     if (scriptInterrupt(rctx) == SCRIPT_KILL) {
         serverLog(LL_WARNING,"Lua script killed by user with SCRIPT KILL.");
 

tests/unit/functions.tcl

@@ -644,6 +644,15 @@ start_server {tags {"scripting"}} {
         set _ $e
     } {*attempted to access nonexistent global variable 'set_repl'*}
 
+    test {LIBRARIES - redis.acl_check_cmd from function load} {
+        catch {
+            r function load replace {#!lua name=lib2
+                return redis.acl_check_cmd('set','xx',1)
+            }
+        } e
+        set _ $e
+    } {*attempted to access nonexistent global variable 'acl_check_cmd'*}
+
     test {LIBRARIES - malicious access test} {
         # the 'library' API is not exposed inside a
         # function context and the 'redis' API is not