From 6dd003e88feace83e55491f32376f6927896e31e Mon Sep 17 00:00:00 2001 From: Madelyn Olson Date: Fri, 3 Oct 2025 06:32:24 -0700 Subject: [PATCH] Merge commit from fork Signed-off-by: Madelyn Olson --- deps/lua/src/lbaselib.c | 7 ++- deps/lua/src/llex.c | 33 +++++++---- deps/lua/src/lparser.c | 6 +- deps/lua/src/ltable.c | 3 +- src/config.c | 11 ++++ src/eval.h | 2 +- src/lua/engine_lua.c | 2 + src/lua/function_lua.c | 2 + src/lua/script_lua.c | 58 ++++++++++++++++--- src/lua/script_lua.h | 1 + src/server.h | 2 + tests/unit/scripting.tcl | 121 ++++++++++++++++++++++++++++++++++++++- 12 files changed, 220 insertions(+), 28 deletions(-) diff --git a/deps/lua/src/lbaselib.c b/deps/lua/src/lbaselib.c index 2ab550bd4..26172d15b 100644 --- a/deps/lua/src/lbaselib.c +++ b/deps/lua/src/lbaselib.c @@ -340,13 +340,14 @@ static int luaB_assert (lua_State *L) { static int luaB_unpack (lua_State *L) { - int i, e, n; + int i, e; + unsigned int n; luaL_checktype(L, 1, LUA_TTABLE); i = luaL_optint(L, 2, 1); e = luaL_opt(L, luaL_checkint, 3, luaL_getn(L, 1)); if (i > e) return 0; /* empty range */ - n = e - i + 1; /* number of elements */ - if (n <= 0 || !lua_checkstack(L, n)) /* n <= 0 means arith. overflow */ + n = (unsigned int)e - (unsigned int)i; /* number of elements minus 1 */ + if (n >= INT_MAX || !lua_checkstack(L, ++n)) return luaL_error(L, "too many results to unpack"); lua_rawgeti(L, 1, i); /* push arg[i] (avoiding overflow problems) */ while (i++ < e) /* push arg[i + 1...e] */ diff --git a/deps/lua/src/llex.c b/deps/lua/src/llex.c index 88c6790c0..3712edec8 100644 --- a/deps/lua/src/llex.c +++ b/deps/lua/src/llex.c @@ -138,6 +138,7 @@ static void inclinenumber (LexState *ls) { void luaX_setinput (lua_State *L, LexState *ls, ZIO *z, TString *source) { + ls->t.token = 0; ls->decpoint = '.'; ls->L = L; ls->lookahead.token = TK_EOS; /* no look-ahead token */ @@ -207,8 +208,13 @@ static void read_numeral (LexState *ls, SemInfo *seminfo) { } -static int skip_sep (LexState *ls) { - int count = 0; +/* +** reads a sequence '[=*[' or ']=*]', leaving the last bracket. +** If a sequence is well-formed, return its number of '='s + 2; otherwise, +** return 1 if there is no '='s or 0 otherwise (an unfinished '[==...'). +*/ +static size_t skip_sep (LexState *ls) { + size_t count = 0; int s = ls->current; lua_assert(s == '[' || s == ']'); save_and_next(ls); @@ -216,11 +222,13 @@ static int skip_sep (LexState *ls) { save_and_next(ls); count++; } - return (ls->current == s) ? count : (-count) - 1; + return (ls->current == s) ? count + 2 + : (count == 0) ? 1 + : 0; } -static void read_long_string (LexState *ls, SemInfo *seminfo, int sep) { +static void read_long_string (LexState *ls, SemInfo *seminfo, size_t sep) { int cont = 0; (void)(cont); /* avoid warnings when `cont' is not used */ save_and_next(ls); /* skip 2nd `[' */ @@ -270,8 +278,8 @@ static void read_long_string (LexState *ls, SemInfo *seminfo, int sep) { } } endloop: if (seminfo) - seminfo->ts = luaX_newstring(ls, luaZ_buffer(ls->buff) + (2 + sep), - luaZ_bufflen(ls->buff) - 2*(2 + sep)); + seminfo->ts = luaX_newstring(ls, luaZ_buffer(ls->buff) + sep, + luaZ_bufflen(ls->buff) - 2 * sep); } @@ -346,9 +354,9 @@ static int llex (LexState *ls, SemInfo *seminfo) { /* else is a comment */ next(ls); if (ls->current == '[') { - int sep = skip_sep(ls); + size_t sep = skip_sep(ls); luaZ_resetbuffer(ls->buff); /* `skip_sep' may dirty the buffer */ - if (sep >= 0) { + if (sep >= 2) { read_long_string(ls, NULL, sep); /* long comment */ luaZ_resetbuffer(ls->buff); continue; @@ -360,13 +368,14 @@ static int llex (LexState *ls, SemInfo *seminfo) { continue; } case '[': { - int sep = skip_sep(ls); - if (sep >= 0) { + size_t sep = skip_sep(ls); + if (sep >= 2) { read_long_string(ls, seminfo, sep); return TK_STRING; } - else if (sep == -1) return '['; - else luaX_lexerror(ls, "invalid long string delimiter", TK_STRING); + else if (sep == 0) /* '[=...' missing second bracket */ + luaX_lexerror(ls, "invalid long string delimiter", TK_STRING); + return '['; } case '=': { next(ls); diff --git a/deps/lua/src/lparser.c b/deps/lua/src/lparser.c index dda7488dc..ee7d90c90 100644 --- a/deps/lua/src/lparser.c +++ b/deps/lua/src/lparser.c @@ -384,13 +384,17 @@ Proto *luaY_parser (lua_State *L, ZIO *z, Mbuffer *buff, const char *name) { struct LexState lexstate; struct FuncState funcstate; lexstate.buff = buff; - luaX_setinput(L, &lexstate, z, luaS_new(L, name)); + TString *tname = luaS_new(L, name); + setsvalue2s(L, L->top, tname); + incr_top(L); + luaX_setinput(L, &lexstate, z, tname); open_func(&lexstate, &funcstate); funcstate.f->is_vararg = VARARG_ISVARARG; /* main func. is always vararg */ luaX_next(&lexstate); /* read first token */ chunk(&lexstate); check(&lexstate, TK_EOS); close_func(&lexstate); + --L->top; lua_assert(funcstate.prev == NULL); lua_assert(funcstate.f->nups == 0); lua_assert(lexstate.fs == NULL); diff --git a/deps/lua/src/ltable.c b/deps/lua/src/ltable.c index f75fe19fe..55575a8ac 100644 --- a/deps/lua/src/ltable.c +++ b/deps/lua/src/ltable.c @@ -434,8 +434,7 @@ static TValue *newkey (lua_State *L, Table *t, const TValue *key) { ** search function for integers */ const TValue *luaH_getnum (Table *t, int key) { - /* (1 <= key && key <= t->sizearray) */ - if (cast(unsigned int, key-1) < cast(unsigned int, t->sizearray)) + if (1 <= key && key <= t->sizearray) return &t->array[key-1]; else { lua_Number nk = cast_num(key); diff --git a/src/config.c b/src/config.c index e799e24ff..d0158b2c4 100644 --- a/src/config.c +++ b/src/config.c @@ -35,6 +35,7 @@ #include "bio.h" #include "module.h" #include "cluster_migrateslots.h" +#include "eval.h" #include #include @@ -2631,6 +2632,15 @@ int invalidateClusterSlotsResp(const char **err) { return 1; } +static int updateLuaEnableInsecureApi(const char **err) { + UNUSED(err); + if (server.lua_insecure_api_current != server.lua_enable_insecure_api) { + evalReset(server.lazyfree_lazy_user_flush ? 1 : 0); + } + server.lua_insecure_api_current = server.lua_enable_insecure_api; + return 1; +} + int updateRequirePass(const char **err) { UNUSED(err); /* The old "requirepass" directive just translates to setting @@ -3229,6 +3239,7 @@ standardConfig static_configs[] = { createBoolConfig("enable-debug-assert", NULL, IMMUTABLE_CONFIG | HIDDEN_CONFIG, server.enable_debug_assert, 0, NULL, NULL), createBoolConfig("cluster-slot-stats-enabled", NULL, MODIFIABLE_CONFIG, server.cluster_slot_stats_enabled, 0, NULL, NULL), createBoolConfig("hide-user-data-from-log", NULL, MODIFIABLE_CONFIG, server.hide_user_data_from_log, 1, NULL, NULL), + createBoolConfig("lua-enable-insecure-api", "lua-enable-deprecated-api", MODIFIABLE_CONFIG | HIDDEN_CONFIG | PROTECTED_CONFIG, server.lua_enable_insecure_api, 0, NULL, updateLuaEnableInsecureApi), createBoolConfig("import-mode", NULL, DEBUG_CONFIG | MODIFIABLE_CONFIG, server.import_mode, 0, NULL, NULL), /* String Configs */ diff --git a/src/eval.h b/src/eval.h index 4f30b3d37..f9c217ef0 100644 --- a/src/eval.h +++ b/src/eval.h @@ -2,7 +2,7 @@ #define _EVAL_H_ void evalInit(void); - +void evalReset(int async); void *evalActiveDefragScript(void *ptr); #endif /* _EVAL_H_ */ diff --git a/src/lua/engine_lua.c b/src/lua/engine_lua.c index 3d1b98413..d628f0413 100644 --- a/src/lua/engine_lua.c +++ b/src/lua/engine_lua.c @@ -104,6 +104,8 @@ static void luaStateLockGlobalTable(lua_State *lua) { /* Recursively lock all tables that can be reached from the global table */ luaSetTableProtectionRecursively(lua); lua_pop(lua, 1); + /* Set metatables of basic types (string, number, nil etc.) readonly. */ + luaSetTableProtectionForBasicTypes(lua); } diff --git a/src/lua/function_lua.c b/src/lua/function_lua.c index 1449c812e..d4fb00a8e 100644 --- a/src/lua/function_lua.c +++ b/src/lua/function_lua.c @@ -435,6 +435,8 @@ void luaFunctionInitializeLuaState(lua_State *lua) { lua_setmetatable(lua, -2); lua_enablereadonlytable(lua, -1, 1); /* protect the new global table */ lua_replace(lua, LUA_GLOBALSINDEX); /* set new global table as the new globals */ + /* Set metatables of basic types (string, number, nil etc.) readonly. */ + luaSetTableProtectionForBasicTypes(lua); } void luaFunctionFreeFunction(lua_State *lua, void *function) { diff --git a/src/lua/script_lua.c b/src/lua/script_lua.c index f884a9493..d186eeec6 100644 --- a/src/lua/script_lua.c +++ b/src/lua/script_lua.c @@ -70,7 +70,6 @@ static char *server_api_allow_list[] = { static char *lua_builtins_allow_list[] = { "xpcall", "tostring", - "getfenv", "setmetatable", "next", "assert", @@ -91,15 +90,16 @@ static char *lua_builtins_allow_list[] = { "loadstring", "ipairs", "_VERSION", - "setfenv", "load", "error", NULL, }; -/* Lua builtins which are not documented on the Lua documentation */ -static char *lua_builtins_not_documented_allow_list[] = { +/* Lua builtins which are deprecated for sandboxing concerns */ +static char *lua_builtins_deprecated[] = { "newproxy", + "setfenv", + "getfenv", NULL, }; @@ -121,7 +121,6 @@ static char **allow_lists[] = { libraries_allow_list, server_api_allow_list, lua_builtins_allow_list, - lua_builtins_not_documented_allow_list, lua_builtins_removed_after_initialization_allow_list, NULL, }; @@ -1325,7 +1324,21 @@ static int luaNewIndexAllowList(lua_State *lua) { break; } } - if (!*allow_l) { + int allowed = (*allow_l != NULL); + /* If not explicitly allowed, check if it's a deprecated function. If so, + * allow it only if 'lua_enable_insecure_api' config is enabled. */ + int deprecated = 0; + if (!allowed) { + char **c = lua_builtins_deprecated; + for (; *c; ++c) { + if (strcmp(*c, variable_name) == 0) { + deprecated = 1; + allowed = server.lua_enable_insecure_api ? 1 : 0; + break; + } + } + } + if (!allowed) { /* Search the value on the back list, if its there we know that it was removed * on purpose and there is no need to print a warning. */ char **c = deny_list; @@ -1334,7 +1347,7 @@ static int luaNewIndexAllowList(lua_State *lua) { break; } } - if (!*c) { + if (!*c && !deprecated) { serverLog(LL_WARNING, "A key '%s' was added to Lua globals which is neither on the globals allow list nor listed on the " "deny list.", @@ -1389,6 +1402,37 @@ void luaSetTableProtectionRecursively(lua_State *lua) { } } +/* Set the readonly flag on the metatable of basic types (string, nil etc.) */ +void luaSetTableProtectionForBasicTypes(lua_State *lua) { + static const int types[] = { + LUA_TSTRING, + LUA_TNUMBER, + LUA_TBOOLEAN, + LUA_TNIL, + LUA_TFUNCTION, + LUA_TTHREAD, + LUA_TLIGHTUSERDATA + }; + + for (size_t i = 0; i < sizeof(types) / sizeof(types[0]); i++) { + /* Push a dummy value of the type to get its metatable */ + switch (types[i]) { + case LUA_TSTRING: lua_pushstring(lua, ""); break; + case LUA_TNUMBER: lua_pushnumber(lua, 0); break; + case LUA_TBOOLEAN: lua_pushboolean(lua, 0); break; + case LUA_TNIL: lua_pushnil(lua); break; + case LUA_TFUNCTION: lua_pushcfunction(lua, NULL); break; + case LUA_TTHREAD: lua_newthread(lua); break; + case LUA_TLIGHTUSERDATA: lua_pushlightuserdata(lua, (void*)lua); break; + } + if (lua_getmetatable(lua, -1)) { + luaSetTableProtectionRecursively(lua); + lua_pop(lua, 1); /* pop metatable */ + } + lua_pop(lua, 1); /* pop dummy value */ + } +} + void luaRegisterVersion(lua_State *lua) { /* For legacy compatibility reasons include Redis versions. */ lua_pushstring(lua, "REDIS_VERSION_NUM"); diff --git a/src/lua/script_lua.h b/src/lua/script_lua.h index 1eb40d77a..3ecbdf44c 100644 --- a/src/lua/script_lua.h +++ b/src/lua/script_lua.h @@ -71,6 +71,7 @@ void luaRegisterGlobalProtectionFunction(lua_State *lua); void luaSetErrorMetatable(lua_State *lua); void luaSetAllowListProtection(lua_State *lua); void luaSetTableProtectionRecursively(lua_State *lua); +void luaSetTableProtectionForBasicTypes(lua_State *lua); void luaRegisterLogFunction(lua_State *lua); void luaRegisterVersion(lua_State *lua); void luaPushErrorBuff(lua_State *lua, sds err_buff); diff --git a/src/server.h b/src/server.h index b3c6b4356..19cae91bf 100644 --- a/src/server.h +++ b/src/server.h @@ -2239,6 +2239,8 @@ struct valkeyServer { mstime_t busy_reply_threshold; /* Script / module timeout in milliseconds */ int pre_command_oom_state; /* OOM before command (script?) was started */ int script_disable_deny_script; /* Allow running commands marked "noscript" inside a script. */ + int lua_enable_insecure_api; /* Config to enable insecure api */ + int lua_insecure_api_current; /* Current value of if insecure apis are enabled, used to determine if flush is needed. */ /* Lazy free */ int lazyfree_lazy_eviction; int lazyfree_lazy_expire; diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index 9f8bde5fa..1c5513732 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -618,13 +618,91 @@ start_server {tags {"scripting"}} { assert_error {NOSCRIPT*} {r evalsha fd758d1589d044dd850a6f05d52f2eefd27f033f 1 mykey} } + + test {EVAL - Test table unpack with invalid indexes} { + catch {r eval { return {unpack({1,2,3}, -2, 2147483647)} } 0} e + assert_match {*too many results to unpack*} $e + catch {r eval { return {unpack({1,2,3}, 0, 2147483647)} } 0} e + assert_match {*too many results to unpack*} $e + catch {r eval { return {unpack({1,2,3}, -2147483648, -2)} } 0} e + assert_match {*too many results to unpack*} $e + set res [r eval { return {unpack({1,2,3}, -1, -2)} } 0] + assert_match {} $res + set res [r eval { return {unpack({1,2,3}, 1, -1)} } 0] + assert_match {} $res + + # unpack with range -1 to 5, verify nil indexes + set res [r eval { + local function unpack_to_list(t, i, j) + local n, v = select('#', unpack(t, i, j)), {unpack(t, i, j)} + for i = 1, n do v[i] = v[i] or '_NIL_' end + v.n = n + return v + end + + return unpack_to_list({1,2,3}, -1, 5) + } 0] + assert_match {_NIL_ _NIL_ 1 2 3 _NIL_ _NIL_} $res + + # unpack with negative range, verify nil indexes + set res [r eval { + local function unpack_to_list(t, i, j) + local n, v = select('#', unpack(t, i, j)), {unpack(t, i, j)} + for i = 1, n do v[i] = v[i] or '_NIL_' end + v.n = n + return v + end + + return unpack_to_list({1,2,3}, -2147483648, -2147483646) + } 0] + assert_match {_NIL_ _NIL_ _NIL_} $res + } {} + + test "Try trick readonly table on basic types metatable" { + # Run the following scripts for basic types. Either getmetatable() + # should return nil or the metatable must be readonly. + set scripts { + {getmetatable(nil).__index = function() return 1 end} + {getmetatable('').__index = function() return 1 end} + {getmetatable(123.222).__index = function() return 1 end} + {getmetatable(true).__index = function() return 1 end} + {getmetatable(function() return 1 end).__index = function() return 1 end} + {getmetatable(coroutine.create(function() return 1 end)).__index = function() return 1 end} + } + + foreach code $scripts { + catch {r eval $code 0} e + assert { + [string match "*attempt to index a nil value*" $e] || + [string match "*Attempt to modify a readonly table*" $e] + } + } + } + + test {Dynamic reset of lua engine with insecure API config change} { + # Ensure insecure API is not available by default + assert_error {*Script attempted to access nonexistent global variable 'getfenv'*} { + r eval "return getfenv()" 0 + } + + # Verify that enabling the config `lua-enable-insecure-api` allows insecure API access + r config set lua-enable-insecure-api yes + assert_equal {} [r eval "return getfenv()" 0] + + r config set lua-enable-insecure-api no + assert_error {*Script attempted to access nonexistent global variable 'getfenv'*} { + r eval "return getfenv()" 0 + } + } + test {SCRIPTING FLUSH ASYNC} { + r script flush sync for {set j 0} {$j < 100} {incr j} { r script load "return $j" } - assert { [string match "*number_of_cached_scripts:100*" [r info Memory]] } + assert_match "*number_of_cached_scripts:100*" [r info Memory] r script flush async - assert { [string match "*number_of_cached_scripts:0*" [r info Memory]] } + assert_match "*number_of_cached_scripts:0*" [r info Memory] } test {SCRIPT EXISTS - can detect already defined scripts?} { @@ -1155,6 +1233,45 @@ start_server {tags {"scripting"}} { } {*Script attempted to access nonexistent global variable 'print'*} } +# start a new server to test the large-memory tests +start_server {tags {"scripting external:skip large-memory"}} { + test {EVAL - Test long escape sequences for strings} { + r eval { + -- Generate 1gb '==...==' separator + local s = string.rep('=', 1024 * 1024) + local t = {} for i=1,1024 do t[i] = s end + local sep = table.concat(t) + collectgarbage('collect') + + local code = table.concat({'return [',sep,'[x]',sep,']'}) + collectgarbage('collect') + + -- Load the code and run it. Script will return the string length. + -- Escape sequence: [=....=[ to ]=...=] will be ignored + -- Actual string is a single character: 'x'. Script will return 1 + local func = loadstring(code) + return #func() + } 0 + } {1} + + test {EVAL - Lua can parse string with too many new lines} { + # Create a long string consisting only of newline characters. When Lua + # fails to parse a string, it typically includes a snippet like + # "... near ..." in the error message to indicate the last recognizable + # token. In this test, since the input contains only newlines, there + # should be no identifiable token, so the error message should contain + # only the actual error, without a near clause. + + r eval { + local s = string.rep('\n', 1024 * 1024) + local t = {} for i=1,2048 do t[#t+1] = s end + local lines = table.concat(t) + local fn, err = loadstring(lines) + return err + } 0 + } {*chunk has too many lines} +} + # Start a new server since the last test in this stanza will kill the # instance at all. start_server {tags {"scripting"}} {