• Meir Shpilraien (Spielrein)'s avatar
    Fix #11030, use lua_rawget to avoid triggering metatables and crash. (#11032) · 020e046b
    Meir Shpilraien (Spielrein) authored
    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,
    Redis uses `lua_gettable` api which might trigger the metatable (if exists) and might raise an error.
    This code is not running inside `pcall` (Lua protected call), so raising an error causes the
    Lua to panic and exit. Notice that this is not a crash, its a Lua panic that exit with error code 1.
    
    Returning `_G` panics on Redis 7 and 6.2.7 because on those versions `_G` has a metatable
    that raises error when trying to fetch a none existing key.
    
    ### Solution
    
    Instead of using `lua_gettable` that might raise error and cause the issue, use `lua_rawget`
    that simply return the value from the table without triggering any metatable logic.
    This is promised not to raise and error.
    
    The downside of this solution is that it might be considered as breaking change, if someone
    rely on metatable in the returned value. An alternative solution is to wrap this entire logic
    with `pcall` (Lua protected call), this alternative require a much bigger refactoring.
    
    ### Back Porting
    
    The same fix will work on older versions as well (6.2, 6.0). Notice that on those version,
    the issue can cause Redis to crash if inside the metatable logic there is an attempt to accesses
    Redis (`redis.call`). On 7.0, there is not crash and the `redis.call` is executed as if it was done
    from inside the script itself.
    
    ### Tests
    
    Tests was added the verify the fix
    020e046b
scripting.tcl 62.9 KB