File bsc1198952.patch of Package redis.28794
From 8b33d813a3d47d4daeaaef03b7e42a51f6931f79 Mon Sep 17 00:00:00 2001
From: meir <meir@redis.com>
Date: Sun, 6 Feb 2022 13:43:18 +0200
Subject: [PATCH 1/4] Added support for Lua readonly tables.
The new feature can be turned off and on using the new `lua_enablereadonlytable` Lua API.
---
deps/lua/src/lapi.c | 14 ++++++++++++++
deps/lua/src/ldebug.c | 1 -
deps/lua/src/lobject.h | 3 ++-
deps/lua/src/ltable.c | 1 +
deps/lua/src/lua.h | 2 ++
deps/lua/src/lvm.c | 2 ++
6 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/deps/lua/src/lapi.c b/deps/lua/src/lapi.c
index 5d5145d2ebad..1a9455629450 100644
--- a/deps/lua/src/lapi.c
+++ b/deps/lua/src/lapi.c
@@ -674,6 +674,8 @@ LUA_API void lua_rawset (lua_State *L, int idx) {
api_checknelems(L, 2);
t = index2adr(L, idx);
api_check(L, ttistable(t));
+ if (hvalue(t)->readonly)
+ luaG_runerror(L, "Attempt to modify a readonly table");
setobj2t(L, luaH_set(L, hvalue(t), L->top-2), L->top-1);
luaC_barriert(L, hvalue(t), L->top-1);
L->top -= 2;
@@ -687,6 +689,8 @@ LUA_API void lua_rawseti (lua_State *L, int idx, int n) {
api_checknelems(L, 1);
o = index2adr(L, idx);
api_check(L, ttistable(o));
+ if (hvalue(o)->readonly)
+ luaG_runerror(L, "Attempt to modify a readonly table");
setobj2t(L, luaH_setnum(L, hvalue(o), n), L->top-1);
luaC_barriert(L, hvalue(o), L->top-1);
L->top--;
@@ -709,6 +713,8 @@ LUA_API int lua_setmetatable (lua_State *L, int objindex) {
}
switch (ttype(obj)) {
case LUA_TTABLE: {
+ if (hvalue(obj)->readonly)
+ luaG_runerror(L, "Attempt to modify a readonly table");
hvalue(obj)->metatable = mt;
if (mt)
luaC_objbarriert(L, hvalue(obj), mt);
@@ -1085,3 +1091,11 @@ LUA_API const char *lua_setupvalue (lua_State *L, int funcindex, int n) {
return name;
}
+LUA_API void lua_enablereadonlytable (lua_State *L, int objindex, int enabled) {
+ const TValue* o = index2adr(L, objindex);
+ api_check(L, ttistable(o));
+ Table* t = hvalue(o);
+ api_check(L, t != hvalue(registry(L)));
+ t->readonly = enabled;
+}
+
diff --git a/deps/lua/src/ldebug.c b/deps/lua/src/ldebug.c
index 50ad3d38035e..4940e65a6c8f 100644
--- a/deps/lua/src/ldebug.c
+++ b/deps/lua/src/ldebug.c
@@ -80,7 +80,6 @@ LUA_API int lua_gethookcount (lua_State *L) {
return L->basehookcount;
}
-
LUA_API int lua_getstack (lua_State *L, int level, lua_Debug *ar) {
int status;
CallInfo *ci;
diff --git a/deps/lua/src/lobject.h b/deps/lua/src/lobject.h
index f1e447ef3ba8..10e46b178022 100644
--- a/deps/lua/src/lobject.h
+++ b/deps/lua/src/lobject.h
@@ -337,7 +337,8 @@ typedef struct Node {
typedef struct Table {
CommonHeader;
- lu_byte flags; /* 1<<p means tagmethod(p) is not present */
+ lu_byte flags; /* 1<<p means tagmethod(p) is not present */
+ int readonly;
lu_byte lsizenode; /* log2 of size of `node' array */
struct Table *metatable;
TValue *array; /* array part */
diff --git a/deps/lua/src/ltable.c b/deps/lua/src/ltable.c
index ec84f4fabc51..f75fe19fe39a 100644
--- a/deps/lua/src/ltable.c
+++ b/deps/lua/src/ltable.c
@@ -364,6 +364,7 @@ Table *luaH_new (lua_State *L, int narray, int nhash) {
t->array = NULL;
t->sizearray = 0;
t->lsizenode = 0;
+ t->readonly = 0;
t->node = cast(Node *, dummynode);
setarrayvector(L, t, narray);
setnodevector(L, t, nhash);
diff --git a/deps/lua/src/lua.h b/deps/lua/src/lua.h
index a4b73e743ede..e478d14c02e3 100644
--- a/deps/lua/src/lua.h
+++ b/deps/lua/src/lua.h
@@ -358,6 +358,8 @@ struct lua_Debug {
int i_ci; /* active function */
};
+LUA_API void lua_enablereadonlytable (lua_State *L, int index, int enabled);
+
/* }====================================================================== */
diff --git a/deps/lua/src/lvm.c b/deps/lua/src/lvm.c
index e0a0cd85219d..5cd313b5e3a0 100644
--- a/deps/lua/src/lvm.c
+++ b/deps/lua/src/lvm.c
@@ -138,6 +138,8 @@ void luaV_settable (lua_State *L, const TValue *t, TValue *key, StkId val) {
const TValue *tm;
if (ttistable(t)) { /* `t' is a table? */
Table *h = hvalue(t);
+ if (h->readonly)
+ luaG_runerror(L, "Attempt to modify a readonly table");
TValue *oldval = luaH_set(L, h, key); /* do a primitive set */
if (!ttisnil(oldval) || /* result is no nil? */
(tm = fasttm(L, h->metatable, TM_NEWINDEX)) == NULL) { /* or no TM? */
From 992f9e23c7ee819ad0dfac0bd6224d8330366960 Mon Sep 17 00:00:00 2001
From: meir <meir@redis.com>
Date: Sun, 6 Feb 2022 14:30:15 +0200
Subject: [PATCH 2/4] Move user eval function to be located on Lua registry.
Today, Redis wrap the user Lua code with a Lua function.
For example, assuming the user code is:
```
return redis.call('ping')
```
The actual code that would have sent to the Lua interpreter was:
```
f_b3a02c833904802db9c34a3cf1292eee3246df3c() return redis.call('ping') end
```
The wraped code would have been saved on the global dictionary with the
following name: `f_<script sha>` (in our example `f_b3a02c833904802db9c34a3cf1292eee3246df3c`).
This approach allows one user to easily override the implementation a another user code, example:
```
f_b3a02c833904802db9c34a3cf1292eee3246df3c = function() return 'hacked' end
```
Running the above code will cause `evalsha b3a02c833904802db9c34a3cf1292eee3246df3c 0` to return
hacked although it should have returned `pong`.
Another disadventage is that Redis basically runs code on the loading (compiling) phase without been
aware of it. User can do code injection like this:
```
return 1 end <run code on compling phase> function() return 1
```
The wraped code will look like this and the entire `<run code on compling phase>` block will run outside
of eval or evalsha context:
```
f_<sha>() return 1 end <run code on compling phase> function() return 1 end
```
---
src/eval.c | 29 +++++++----------------------
src/script_lua.c | 4 ++--
src/script_lua.h | 2 +-
3 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/src/eval.c b/src/eval.c
index 4894b83d326d..1be0d42f2616 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -269,7 +269,7 @@ void scriptingInit(int setup) {
/* Lua beginners often don't use "local", this is likely to introduce
* subtle bugs in their code. To prevent problems we protect accesses
* to global variables. */
- luaEnableGlobalsProtection(lua, 1);
+ luaEnableGlobalsProtection(lua);
lctx.lua = lua;
}
@@ -378,35 +378,20 @@ sds luaCreateFunction(client *c, robj *body) {
sdsfreesplitres(parts, numparts);
}
- /* Build the lua function to be loaded */
- sds funcdef = sdsempty();
- funcdef = sdscat(funcdef,"function ");
- funcdef = sdscatlen(funcdef,funcname,42);
- funcdef = sdscatlen(funcdef,"() ",3);
/* Note that in case of a shebang line we skip it but keep the line feed to conserve the user's line numbers */
- funcdef = sdscatlen(funcdef,(char*)body->ptr + shebang_len,sdslen(body->ptr) - shebang_len);
- funcdef = sdscatlen(funcdef,"\nend",4);
-
- if (luaL_loadbuffer(lctx.lua,funcdef,sdslen(funcdef),"@user_script")) {
+ if (luaL_loadbuffer(lctx.lua,(char*)body->ptr + shebang_len,sdslen(body->ptr) - shebang_len,"@user_script")) {
if (c != NULL) {
addReplyErrorFormat(c,
"Error compiling script (new function): %s",
lua_tostring(lctx.lua,-1));
}
lua_pop(lctx.lua,1);
- sdsfree(funcdef);
return NULL;
}
- sdsfree(funcdef);
- if (lua_pcall(lctx.lua,0,0,0)) {
- if (c != NULL) {
- addReplyErrorFormat(c,"Error running script (new function): %s",
- lua_tostring(lctx.lua,-1));
- }
- lua_pop(lctx.lua,1);
- return NULL;
- }
+ serverAssert(lua_isfunction(lctx.lua, -1));
+
+ lua_setfield(lctx.lua, LUA_REGISTRYINDEX, funcname);
/* We also save a SHA1 -> Original script map in a dictionary
* so that we can replicate / write in the AOF all the
@@ -479,7 +464,7 @@ void evalGenericCommand(client *c, int evalsha) {
lua_getglobal(lua, "__redis__err__handler");
/* Try to lookup the Lua function */
- lua_getglobal(lua, funcname);
+ lua_getfield(lua, LUA_REGISTRYINDEX, funcname);
if (lua_isnil(lua,-1)) {
lua_pop(lua,1); /* remove the nil from the stack */
/* Function not defined... let's define it if we have the
@@ -497,7 +482,7 @@ void evalGenericCommand(client *c, int evalsha) {
return;
}
/* Now the following is guaranteed to return non nil */
- lua_getglobal(lua, funcname);
+ lua_getfield(lua, LUA_REGISTRYINDEX, funcname);
serverAssert(!lua_isnil(lua,-1));
}
diff --git a/src/script_lua.c b/src/script_lua.c
index 9a08a7e47a31..4e1f176492d2 100644
--- a/src/script_lua.c
+++ b/src/script_lua.c
@@ -1144,7 +1144,7 @@ sds luaGetStringSds(lua_State *lua, int index) {
* On Legacy Lua (eval) we need to check 'w ~= \"main\"' otherwise we will not be able
* to create the global 'function <sha> ()' variable. On Functions Lua engine we do not use
* this trick so it's not needed. */
-void luaEnableGlobalsProtection(lua_State *lua, int is_eval) {
+void luaEnableGlobalsProtection(lua_State *lua) {
char *s[32];
sds code = sdsempty();
int j = 0;
@@ -1157,7 +1157,7 @@ void luaEnableGlobalsProtection(lua_State *lua, int is_eval) {
s[j++]="mt.__newindex = function (t, n, v)\n";
s[j++]=" if dbg.getinfo(2) then\n";
s[j++]=" local w = dbg.getinfo(2, \"S\").what\n";
- s[j++]= is_eval ? " if w ~= \"main\" and w ~= \"C\" then\n" : " if w ~= \"C\" then\n";
+ s[j++]=" if w ~= \"C\" then\n";
s[j++]=" error(\"Script attempted to create global variable '\"..tostring(n)..\"'\", 2)\n";
s[j++]=" end\n";
s[j++]=" end\n";
diff --git a/src/script_lua.h b/src/script_lua.h
index 5a4533784ce8..f39c017441c4 100644
--- a/src/script_lua.h
+++ b/src/script_lua.h
@@ -67,7 +67,7 @@ typedef struct errorInfo {
void luaRegisterRedisAPI(lua_State* lua);
sds luaGetStringSds(lua_State *lua, int index);
-void luaEnableGlobalsProtection(lua_State *lua, int is_eval);
+void luaEnableGlobalsProtection(lua_State *lua);
void luaRegisterGlobalProtectionFunction(lua_State *lua);
void luaSetGlobalProtection(lua_State *lua);
void luaRegisterLogFunction(lua_State* lua);
From 3731580b6b80c586322cadc6bc4be2b8b2bbb206 Mon Sep 17 00:00:00 2001
From: meir <meir@redis.com>
Date: Sun, 6 Feb 2022 17:41:55 +0200
Subject: [PATCH 3/4] Protect globals of both evals scripts and functions.
Use the new `lua_enablereadonlytable` Lua API to protect the global tables of
both evals scripts and functions. For eval scripts, the implemetation is easy,
We simply call `lua_enablereadonlytable` on the global table to turn it into
a readonly table.
On functions its more complecated, we want to be able to switch globals between
load run and function run. To achieve this, we create a new empty table that
acts as the globals table for function, we control the actual globals using metatable
manipulation. Notice that even if the user gets a pointer to the original tables, all
the tables are set to be readonly (using `lua_enablereadonlytable` Lua API) so he can
not change them. The following inlustration better explain the solution:
```
Global table {} <- global table metatable {.__index = __real_globals__}
```
The `__real_globals__` is set depends on the run context (function load or function call).
Why this solution is needed and its not enough to simply switch globals?
When we run in the context of function load and create our functions, our function gets
the current globals that was set when they were created. Replacing the globals after
the creation will not effect them. This is why this trick it mandatory.
---
src/eval.c | 8 +--
src/function_lua.c | 113 +++++++++++++++------------------
src/script_lua.c | 132 +++++++++++----------------------------
src/script_lua.h | 1 -
tests/unit/functions.tcl | 65 ++++++++++---------
tests/unit/scripting.tcl | 39 +++++++++++-
6 files changed, 165 insertions(+), 193 deletions(-)
diff --git a/src/eval.c b/src/eval.c
index 1be0d42f2616..f9998b9ed750 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -266,10 +266,10 @@ void scriptingInit(int setup) {
lctx.lua_client->flags |= CLIENT_DENY_BLOCKING;
}
- /* Lua beginners often don't use "local", this is likely to introduce
- * subtle bugs in their code. To prevent problems we protect accesses
- * to global variables. */
- luaEnableGlobalsProtection(lua);
+ /* Lock the global table from any changes */
+ lua_pushvalue(lua, LUA_GLOBALSINDEX);
+ luaSetGlobalProtection(lua);
+ lua_pop(lua, 1);
lctx.lua = lua;
}
diff --git a/src/function_lua.c b/src/function_lua.c
index 1e6363fd464f..f8ca51f05ce5 100644
--- a/src/function_lua.c
+++ b/src/function_lua.c
@@ -50,6 +50,7 @@
#define REGISTRY_ERROR_HANDLER_NAME "__ERROR_HANDLER__"
#define REGISTRY_LOAD_CTX_NAME "__LIBRARY_CTX__"
#define LIBRARY_API_NAME "__LIBRARY_API__"
+#define GLOBALS_API_NAME "__GLOBALS_API__"
#define LOAD_TIMEOUT_MS 500
/* Lua engine ctx */
@@ -99,42 +100,23 @@ static void luaEngineLoadHook(lua_State *lua, lua_Debug *ar) {
* Return NULL on compilation error and set the error to the err variable
*/
static int luaEngineCreate(void *engine_ctx, functionLibInfo *li, sds blob, sds *err) {
+ int ret = C_ERR;
luaEngineCtx *lua_engine_ctx = engine_ctx;
lua_State *lua = lua_engine_ctx->lua;
- /* Each library will have its own global distinct table.
- * We will create a new fresh Lua table and use
- * lua_setfenv to set the table as the library globals
- * (https://www.lua.org/manual/5.1/manual.html#lua_setfenv)
- *
- * At first, populate this new table with only the 'library' API
- * to make sure only 'library' API is available at start. After the
- * initial run is finished and all functions are registered, add
- * all the default globals to the library global table and delete
- * the library API.
- *
- * There are 2 ways to achieve the last part (add default
- * globals to the new table):
- *
- * 1. Initialize the new table with all the default globals
- * 2. Inheritance using metatable (https://www.lua.org/pil/14.3.html)
- *
- * For now we are choosing the second, we can change it in the future to
- * achieve a better isolation between functions. */
- lua_newtable(lua); /* Global table for the library */
- lua_pushstring(lua, REDIS_API_NAME);
- lua_pushstring(lua, LIBRARY_API_NAME);
- lua_gettable(lua, LUA_REGISTRYINDEX); /* get library function from registry */
- lua_settable(lua, -3); /* push the library table to the new global table */
-
- /* Set global protection on the new global table */
- luaSetGlobalProtection(lua_engine_ctx->lua);
+ /* set load library globals */
+ lua_getmetatable(lua, LUA_GLOBALSINDEX);
+ lua_enablereadonlytable(lua, -1, 0); /* disable global protection */
+ lua_getfield(lua, LUA_REGISTRYINDEX, LIBRARY_API_NAME);
+ lua_setfield(lua, -2, "__index");
+ lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 1); /* enable global protection */
+ lua_pop(lua, 1); /* pop the metatable */
/* compile the code */
if (luaL_loadbuffer(lua, blob, sdslen(blob), "@user_function")) {
*err = sdscatprintf(sdsempty(), "Error compiling function: %s", lua_tostring(lua, -1));
- lua_pop(lua, 2); /* pops the error and globals table */
- return C_ERR;
+ lua_pop(lua, 1); /* pops the error */
+ goto done;
}
serverAssert(lua_isfunction(lua, -1));
@@ -144,45 +126,31 @@ static int luaEngineCreate(void *engine_ctx, functionLibInfo *li, sds blob, sds
};
luaSaveOnRegistry(lua, REGISTRY_LOAD_CTX_NAME, &load_ctx);
- /* set the function environment so only 'library' API can be accessed. */
- lua_pushvalue(lua, -2); /* push global table to the front */
- lua_setfenv(lua, -2);
-
lua_sethook(lua,luaEngineLoadHook,LUA_MASKCOUNT,100000);
/* Run the compiled code to allow it to register functions */
if (lua_pcall(lua,0,0,0)) {
errorInfo err_info = {0};
luaExtractErrorInformation(lua, &err_info);
*err = sdscatprintf(sdsempty(), "Error registering functions: %s", err_info.msg);
- lua_pop(lua, 2); /* pops the error and globals table */
- lua_sethook(lua,NULL,0,0); /* Disable hook */
- luaSaveOnRegistry(lua, REGISTRY_LOAD_CTX_NAME, NULL);
+ lua_pop(lua, 1); /* pops the error */
luaErrorInformationDiscard(&err_info);
- return C_ERR;
+ goto done;
}
- lua_sethook(lua,NULL,0,0); /* Disable hook */
- luaSaveOnRegistry(lua, REGISTRY_LOAD_CTX_NAME, NULL);
- /* stack contains the global table, lets rearrange it to contains the entire API. */
- /* delete 'redis' API */
- lua_pushstring(lua, REDIS_API_NAME);
- lua_pushnil(lua);
- lua_settable(lua, -3);
-
- /* create metatable */
- lua_newtable(lua);
- lua_pushstring(lua, "__index");
- lua_pushvalue(lua, LUA_GLOBALSINDEX); /* push original globals */
- lua_settable(lua, -3);
- lua_pushstring(lua, "__newindex");
- lua_pushvalue(lua, LUA_GLOBALSINDEX); /* push original globals */
- lua_settable(lua, -3);
-
- lua_setmetatable(lua, -2);
+ ret = C_OK;
- lua_pop(lua, 1); /* pops the global table */
+done:
+ /* restore original globals */
+ lua_getmetatable(lua, LUA_GLOBALSINDEX);
+ lua_enablereadonlytable(lua, -1, 0); /* disable global protection */
+ lua_getfield(lua, LUA_REGISTRYINDEX, GLOBALS_API_NAME);
+ lua_setfield(lua, -2, "__index");
+ lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 1); /* enable global protection */
+ lua_pop(lua, 1); /* pop the metatable */
- return C_OK;
+ lua_sethook(lua,NULL,0,0); /* Disable hook */
+ luaSaveOnRegistry(lua, REGISTRY_LOAD_CTX_NAME, NULL);
+ return ret;
}
/*
@@ -458,8 +426,8 @@ int luaEngineInitEngine() {
luaRegisterRedisAPI(lua_engine_ctx->lua);
/* Register the library commands table and fields and store it to registry */
- lua_pushstring(lua_engine_ctx->lua, LIBRARY_API_NAME);
- lua_newtable(lua_engine_ctx->lua);
+ lua_newtable(lua_engine_ctx->lua); /* load library globals */
+ lua_newtable(lua_engine_ctx->lua); /* load library `redis` table */
lua_pushstring(lua_engine_ctx->lua, "register_function");
lua_pushcfunction(lua_engine_ctx->lua, luaRegisterFunction);
@@ -468,7 +436,12 @@ int luaEngineInitEngine() {
luaRegisterLogFunction(lua_engine_ctx->lua);
luaRegisterVersion(lua_engine_ctx->lua);
- lua_settable(lua_engine_ctx->lua, LUA_REGISTRYINDEX);
+ luaSetGlobalProtection(lua_engine_ctx->lua); /* protect redis */
+
+ lua_setfield(lua_engine_ctx->lua, -2, REDIS_API_NAME);
+
+ luaSetGlobalProtection(lua_engine_ctx->lua); /* protect load library globals */
+ lua_setfield(lua_engine_ctx->lua, LUA_REGISTRYINDEX, LIBRARY_API_NAME);
/* Save error handler to registry */
lua_pushstring(lua_engine_ctx->lua, REGISTRY_ERROR_HANDLER_NAME);
@@ -492,17 +465,29 @@ int luaEngineInitEngine() {
lua_pcall(lua_engine_ctx->lua,0,1,0);
lua_settable(lua_engine_ctx->lua, LUA_REGISTRYINDEX);
- /* Save global protection to registry */
- luaRegisterGlobalProtectionFunction(lua_engine_ctx->lua);
-
- /* Set global protection on globals */
lua_pushvalue(lua_engine_ctx->lua, LUA_GLOBALSINDEX);
luaSetGlobalProtection(lua_engine_ctx->lua);
lua_pop(lua_engine_ctx->lua, 1);
+ /* Save default globals to registry */
+ lua_pushvalue(lua_engine_ctx->lua, LUA_GLOBALSINDEX);
+ lua_setfield(lua_engine_ctx->lua, LUA_REGISTRYINDEX, GLOBALS_API_NAME);
+
/* save the engine_ctx on the registry so we can get it from the Lua interpreter */
luaSaveOnRegistry(lua_engine_ctx->lua, REGISTRY_ENGINE_CTX_NAME, lua_engine_ctx);
+ /* Create new empty table to be the new globals, we will be able to control the real globals
+ * using metatable */
+ lua_newtable(lua_engine_ctx->lua); /* new globals */
+ lua_newtable(lua_engine_ctx->lua); /* new globals metatable */
+ lua_pushvalue(lua_engine_ctx->lua, LUA_GLOBALSINDEX);
+ lua_setfield(lua_engine_ctx->lua, -2, "__index");
+ lua_enablereadonlytable(lua_engine_ctx->lua, -1, 1); /* protect the metatable */
+ lua_setmetatable(lua_engine_ctx->lua, -2);
+ lua_enablereadonlytable(lua_engine_ctx->lua, -1, 1); /* protect the new global table */
+ lua_replace(lua_engine_ctx->lua, LUA_GLOBALSINDEX); /* set new global table as the new globals */
+
+
engine *lua_engine = zmalloc(sizeof(*lua_engine));
*lua_engine = (engine) {
.engine_ctx = lua_engine_ctx,
diff --git a/src/script_lua.c b/src/script_lua.c
index 4e1f176492d2..406706b7f1e2 100644
--- a/src/script_lua.c
+++ b/src/script_lua.c
@@ -1135,107 +1135,39 @@ sds luaGetStringSds(lua_State *lua, int index) {
return str_sds;
}
-/* This function installs metamethods in the global table _G that prevent
- * the creation of globals accidentally.
- *
- * It should be the last to be called in the scripting engine initialization
- * sequence, because it may interact with creation of globals.
- *
- * On Legacy Lua (eval) we need to check 'w ~= \"main\"' otherwise we will not be able
- * to create the global 'function <sha> ()' variable. On Functions Lua engine we do not use
- * this trick so it's not needed. */
-void luaEnableGlobalsProtection(lua_State *lua) {
- char *s[32];
- sds code = sdsempty();
- int j = 0;
-
- /* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html.
- * Modified to be adapted to Redis. */
- s[j++]="local dbg=debug\n";
- s[j++]="local mt = {}\n";
- s[j++]="setmetatable(_G, mt)\n";
- s[j++]="mt.__newindex = function (t, n, v)\n";
- s[j++]=" if dbg.getinfo(2) then\n";
- s[j++]=" local w = dbg.getinfo(2, \"S\").what\n";
- s[j++]=" if w ~= \"C\" then\n";
- s[j++]=" error(\"Script attempted to create global variable '\"..tostring(n)..\"'\", 2)\n";
- s[j++]=" end\n";
- s[j++]=" end\n";
- s[j++]=" rawset(t, n, v)\n";
- s[j++]="end\n";
- s[j++]="mt.__index = function (t, n)\n";
- s[j++]=" if dbg.getinfo(2) and dbg.getinfo(2, \"S\").what ~= \"C\" then\n";
- s[j++]=" error(\"Script attempted to access nonexistent global variable '\"..tostring(n)..\"'\", 2)\n";
- s[j++]=" end\n";
- s[j++]=" return rawget(t, n)\n";
- s[j++]="end\n";
- s[j++]="debug = nil\n";
- s[j++]=NULL;
-
- for (j = 0; s[j] != NULL; j++) code = sdscatlen(code,s[j],strlen(s[j]));
- luaL_loadbuffer(lua,code,sdslen(code),"@enable_strict_lua");
- lua_pcall(lua,0,0,0);
- sdsfree(code);
-}
-
-/* Create a global protection function and put it to registry.
- * This need to be called once in the lua_State lifetime.
- * After called it is possible to use luaSetGlobalProtection
- * to set global protection on a give table.
- *
- * The function assumes the Lua stack have a least enough
- * space to push 2 element, its up to the caller to verify
- * this before calling this function.
- *
- * Notice, the difference between this and luaEnableGlobalsProtection
- * is that luaEnableGlobalsProtection is enabling global protection
- * on the current Lua globals. This registering a global protection
- * function that later can be applied on any table. */
-void luaRegisterGlobalProtectionFunction(lua_State *lua) {
- lua_pushstring(lua, REGISTRY_SET_GLOBALS_PROTECTION_NAME);
- char *global_protection_func = "local dbg = debug\n"
- "local globals_protection = function (t)\n"
- " local mt = {}\n"
- " setmetatable(t, mt)\n"
- " mt.__newindex = function (t, n, v)\n"
- " if dbg.getinfo(2) then\n"
- " local w = dbg.getinfo(2, \"S\").what\n"
- " if w ~= \"C\" then\n"
- " error(\"Script attempted to create global variable '\"..tostring(n)..\"'\", 2)\n"
- " end"
- " end"
- " rawset(t, n, v)\n"
- " end\n"
- " mt.__index = function (t, n)\n"
- " if dbg.getinfo(2) and dbg.getinfo(2, \"S\").what ~= \"C\" then\n"
- " error(\"Script attempted to access nonexistent global variable '\"..tostring(n)..\"'\", 2)\n"
- " end\n"
- " return rawget(t, n)\n"
- " end\n"
- "end\n"
- "return globals_protection";
- int res = luaL_loadbuffer(lua, global_protection_func, strlen(global_protection_func), "@global_protection_def");
- serverAssert(res == 0);
- res = lua_pcall(lua,0,1,0);
- serverAssert(res == 0);
- lua_settable(lua, LUA_REGISTRYINDEX);
+static int luaProtectedTableError(lua_State *lua) {
+ int argc = lua_gettop(lua);
+ if (argc != 2) {
+ serverLog(LL_WARNING, "malicious code trying to call luaProtectedTableError with wrong arguments");
+ luaL_error(lua, "Wrong number of arguments to luaProtectedTableError");
+ }
+ if (!lua_isstring(lua, -1) && !lua_isnumber(lua, -1)) {
+ luaL_error(lua, "Second argument to luaProtectedTableError must be a string or number");
+ }
+ const char* variable_name = lua_tostring(lua, -1);
+ luaL_error(lua, "Script attempted to access nonexistent global variable '%s'", variable_name);
+ return 0;
}
-/* Set global protection on a given table.
- * The table need to be located on the top of the lua stack.
+/* Set global protection on table on the top of the stack.
* After called, it will no longer be possible to set
- * new items on the table. The function is not removing
- * the table from the top of the stack!
+ * new items on the table. Any attempt to access a protected
+ * table for write will result in an error. Any attempt to
+ * get a none existing element from a locked table will result
+ * in an error.
*
* The function assumes the Lua stack have a least enough
* space to push 2 element, its up to the caller to verify
* this before calling this function. */
void luaSetGlobalProtection(lua_State *lua) {
- lua_pushstring(lua, REGISTRY_SET_GLOBALS_PROTECTION_NAME);
- lua_gettable(lua, LUA_REGISTRYINDEX);
- lua_pushvalue(lua, -2);
- int res = lua_pcall(lua, 1, 0, 0);
- serverAssert(res == 0);
+ lua_newtable(lua); /* push metatable */
+ lua_pushcfunction(lua, luaProtectedTableError); /* push get error handler */
+ lua_setfield(lua, -2, "__index");
+ lua_enablereadonlytable(lua, -1, 1); /* protect the metatable */
+
+ lua_setmetatable(lua, -2);
+
+ lua_enablereadonlytable(lua, -1, 1);
}
void luaRegisterVersion(lua_State* lua) {
@@ -1504,9 +1436,19 @@ void luaCallFunction(scriptRunCtx* run_ctx, lua_State *lua, robj** keys, size_t
* EVAL received. */
luaCreateArray(lua,keys,nkeys);
/* On eval, keys and arguments are globals. */
- if (run_ctx->flags & SCRIPT_EVAL_MODE) lua_setglobal(lua,"KEYS");
+ if (run_ctx->flags & SCRIPT_EVAL_MODE){
+ /* open global protection to set KEYS */
+ lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 0);
+ lua_setglobal(lua,"KEYS");
+ lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 1);
+ }
luaCreateArray(lua,args,nargs);
- if (run_ctx->flags & SCRIPT_EVAL_MODE) lua_setglobal(lua,"ARGV");
+ if (run_ctx->flags & SCRIPT_EVAL_MODE){
+ /* open global protection to set ARGV */
+ lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 0);
+ lua_setglobal(lua,"ARGV");
+ lua_enablereadonlytable(lua, LUA_GLOBALSINDEX, 1);
+ }
/* At this point whether this script was never seen before or if it was
* already defined, we can call it.
diff --git a/src/script_lua.h b/src/script_lua.h
index f39c017441c4..e6ca4f730955 100644
--- a/src/script_lua.h
+++ b/src/script_lua.h
@@ -67,7 +67,6 @@ typedef struct errorInfo {
void luaRegisterRedisAPI(lua_State* lua);
sds luaGetStringSds(lua_State *lua, int index);
-void luaEnableGlobalsProtection(lua_State *lua);
void luaRegisterGlobalProtectionFunction(lua_State *lua);
void luaSetGlobalProtection(lua_State *lua);
void luaRegisterLogFunction(lua_State* lua);
diff --git a/tests/unit/functions.tcl b/tests/unit/functions.tcl
index 7b781c5881cf..62c070f90617 100644
--- a/tests/unit/functions.tcl
+++ b/tests/unit/functions.tcl
@@ -624,16 +624,16 @@ start_server {tags {"scripting"}} {
}
} e
set _ $e
- } {*attempt to call field 'call' (a nil value)*}
+ } {*attempted to access nonexistent global variable 'call'*}
- test {LIBRARIES - redis.call from function load} {
+ test {LIBRARIES - redis.setresp from function load} {
catch {
r function load replace {#!lua name=lib2
return redis.setresp(3)
}
} e
set _ $e
- } {*attempt to call field 'setresp' (a nil value)*}
+ } {*attempted to access nonexistent global variable 'setresp'*}
test {LIBRARIES - redis.set_repl from function load} {
catch {
@@ -642,7 +642,7 @@ start_server {tags {"scripting"}} {
}
} e
set _ $e
- } {*attempt to call field 'set_repl' (a nil value)*}
+ } {*attempted to access nonexistent global variable 'set_repl'*}
test {LIBRARIES - malicious access test} {
# the 'library' API is not exposed inside a
@@ -669,37 +669,18 @@ start_server {tags {"scripting"}} {
end)
end)
}
- assert_equal {OK} [r fcall f1 0]
+ catch {[r fcall f1 0]} e
+ assert_match {*Attempt to modify a readonly table*} $e
catch {[r function load {#!lua name=lib2
redis.math.random()
}]} e
- assert_match {*can only be called inside a script invocation*} $e
-
- catch {[r function load {#!lua name=lib2
- redis.math.randomseed()
- }]} e
- assert_match {*can only be called inside a script invocation*} $e
+ assert_match {*Script attempted to access nonexistent global variable 'math'*} $e
catch {[r function load {#!lua name=lib2
redis.redis.call('ping')
}]} e
- assert_match {*can only be called inside a script invocation*} $e
-
- catch {[r function load {#!lua name=lib2
- redis.redis.pcall('ping')
- }]} e
- assert_match {*can only be called inside a script invocation*} $e
-
- catch {[r function load {#!lua name=lib2
- redis.redis.setresp(3)
- }]} e
- assert_match {*can only be called inside a script invocation*} $e
-
- catch {[r function load {#!lua name=lib2
- redis.redis.set_repl(redis.redis.REPL_NONE)
- }]} e
- assert_match {*can only be called inside a script invocation*} $e
+ assert_match {*Script attempted to access nonexistent global variable 'redis'*} $e
catch {[r fcall f2 0]} e
assert_match {*can only be called on FUNCTION LOAD command*} $e
@@ -756,7 +737,7 @@ start_server {tags {"scripting"}} {
}
} e
set _ $e
- } {*attempted to create global variable 'a'*}
+ } {*Attempt to modify a readonly table*}
test {LIBRARIES - named arguments} {
r function load {#!lua name=lib
@@ -1198,4 +1179,32 @@ start_server {tags {"scripting"}} {
redis.register_function('foo', function() return 1 end)
}
} {foo}
+
+ test {FUNCTION - trick global protection 1} {
+ r FUNCTION FLUSH
+
+ r FUNCTION load {#!lua name=test1
+ redis.register_function('f1', function()
+ mt = getmetatable(_G)
+ original_globals = mt.__index
+ original_globals['redis'] = function() return 1 end
+ end)
+ }
+
+ catch {[r fcall f1 0]} e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test {FUNCTION - test getmetatable on script load} {
+ r FUNCTION FLUSH
+
+ catch {
+ r FUNCTION load {#!lua name=test1
+ mt = getmetatable(_G)
+ }
+ } e
+
+ set _ $e
+ } {*Script attempted to access nonexistent global variable 'getmetatable'*}
+
}
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index a5a253325f38..9e2a4c4b24f7 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -452,7 +452,7 @@ start_server {tags {"scripting"}} {
test {Globals protection setting an undeclared global*} {
catch {run_script {a=10} 0} e
set e
- } {ERR *attempted to create global*}
+ } {ERR *Attempt to modify a readonly table*}
test {Test an example script DECR_IF_GT} {
set decr_if_gt {
@@ -741,6 +741,43 @@ start_server {tags {"scripting"}} {
return loadstring(string.dump(function() return 1 end))()
} 0}
}
+
+ test "Try trick global protection 1" {
+ catch {
+ run_script {
+ setmetatable(_G, {})
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Try trick global protection 2" {
+ catch {
+ run_script {
+ local g = getmetatable(_G)
+ g.__index = {}
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Try trick global protection 3" {
+ catch {
+ run_script {
+ redis = function() return 1 end
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Try trick global protection 4" {
+ catch {
+ run_script {
+ _G = {}
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
}
# Start a new server since the last test in this stanza will kill the
From efa162bcd7ab1477c07d8ee85537e27c0cb1524b Mon Sep 17 00:00:00 2001
From: meir <meir@redis.com>
Date: Tue, 15 Feb 2022 20:18:49 +0200
Subject: [PATCH 4/4] Protect any table which is reachable from globals and
added globals white list.
The white list is done by setting a metatable on the global table before initializing
any library. The metatable set the `__newindex` field to a function that check
the white list before adding the field to the table. Fields which is not on the
white list are simply ignored.
After initialization phase is done we protect the global table and each table
that might be reachable from the global table. For each table we also protect
the table metatable if exists.
---
deps/lua/src/lapi.c | 8 ++
deps/lua/src/lua.h | 1 +
src/eval.c | 17 +---
src/function_lua.c | 10 +-
src/script_lua.c | 204 +++++++++++++++++++++++++++++++++++----
src/script_lua.h | 4 +-
tests/unit/scripting.tcl | 63 ++++++++++++
7 files changed, 270 insertions(+), 37 deletions(-)
diff --git a/deps/lua/src/lapi.c b/deps/lua/src/lapi.c
index 1a9455629450..e8ef41ea248b 100644
--- a/deps/lua/src/lapi.c
+++ b/deps/lua/src/lapi.c
@@ -1099,3 +1099,11 @@ LUA_API void lua_enablereadonlytable (lua_State *L, int objindex, int enabled) {
t->readonly = enabled;
}
+LUA_API int lua_isreadonlytable (lua_State *L, int objindex) {
+ const TValue* o = index2adr(L, objindex);
+ api_check(L, ttistable(o));
+ Table* t = hvalue(o);
+ api_check(L, t != hvalue(registry(L)));
+ return t->readonly;
+}
+
diff --git a/deps/lua/src/lua.h b/deps/lua/src/lua.h
index e478d14c02e3..280ef23821ab 100644
--- a/deps/lua/src/lua.h
+++ b/deps/lua/src/lua.h
@@ -359,6 +359,7 @@ struct lua_Debug {
};
LUA_API void lua_enablereadonlytable (lua_State *L, int index, int enabled);
+LUA_API int lua_isreadonlytable (lua_State *L, int index);
/* }====================================================================== */
diff --git a/src/eval.c b/src/eval.c
index f9998b9ed750..332aec9ce5aa 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -218,24 +218,13 @@ void scriptingInit(int setup) {
lua_setglobal(lua,"redis");
- /* Add a helper function that we use to sort the multi bulk output of non
- * deterministic commands, when containing 'false' elements. */
- {
- char *compare_func = "function __redis__compare_helper(a,b)\n"
- " if a == false then a = '' end\n"
- " if b == false then b = '' end\n"
- " return a<b\n"
- "end\n";
- luaL_loadbuffer(lua,compare_func,strlen(compare_func),"@cmp_func_def");
- lua_pcall(lua,0,0,0);
- }
-
/* Add a helper function we use for pcall error reporting.
* Note that when the error is in the C function we want to report the
* information about the caller, that's what makes sense from the point
* of view of the user debugging a script. */
{
char *errh_func = "local dbg = debug\n"
+ "debug = nil\n"
"function __redis__err__handler(err)\n"
" local i = dbg.getinfo(2,'nSl')\n"
" if i and i.what == 'C' then\n"
@@ -268,7 +257,9 @@ void scriptingInit(int setup) {
/* Lock the global table from any changes */
lua_pushvalue(lua, LUA_GLOBALSINDEX);
- luaSetGlobalProtection(lua);
+ luaSetErrorMetatable(lua);
+ /* Recursively lock all tables that can be reached from the global table */
+ luaSetTableProtectionRecursively(lua);
lua_pop(lua, 1);
lctx.lua = lua;
diff --git a/src/function_lua.c b/src/function_lua.c
index f8ca51f05ce5..2e0250ea25ef 100644
--- a/src/function_lua.c
+++ b/src/function_lua.c
@@ -436,16 +436,17 @@ int luaEngineInitEngine() {
luaRegisterLogFunction(lua_engine_ctx->lua);
luaRegisterVersion(lua_engine_ctx->lua);
- luaSetGlobalProtection(lua_engine_ctx->lua); /* protect redis */
-
+ luaSetErrorMetatable(lua_engine_ctx->lua);
lua_setfield(lua_engine_ctx->lua, -2, REDIS_API_NAME);
- luaSetGlobalProtection(lua_engine_ctx->lua); /* protect load library globals */
+ luaSetErrorMetatable(lua_engine_ctx->lua);
+ luaSetTableProtectionRecursively(lua_engine_ctx->lua); /* protect load library globals */
lua_setfield(lua_engine_ctx->lua, LUA_REGISTRYINDEX, LIBRARY_API_NAME);
/* Save error handler to registry */
lua_pushstring(lua_engine_ctx->lua, REGISTRY_ERROR_HANDLER_NAME);
char *errh_func = "local dbg = debug\n"
+ "debug = nil\n"
"local error_handler = function (err)\n"
" local i = dbg.getinfo(2,'nSl')\n"
" if i and i.what == 'C' then\n"
@@ -466,7 +467,8 @@ int luaEngineInitEngine() {
lua_settable(lua_engine_ctx->lua, LUA_REGISTRYINDEX);
lua_pushvalue(lua_engine_ctx->lua, LUA_GLOBALSINDEX);
- luaSetGlobalProtection(lua_engine_ctx->lua);
+ luaSetErrorMetatable(lua_engine_ctx->lua);
+ luaSetTableProtectionRecursively(lua_engine_ctx->lua); /* protect globals */
lua_pop(lua_engine_ctx->lua, 1);
/* Save default globals to registry */
diff --git a/src/script_lua.c b/src/script_lua.c
index 406706b7f1e2..36868ec2b219 100644
--- a/src/script_lua.c
+++ b/src/script_lua.c
@@ -41,6 +41,97 @@
#include <ctype.h>
#include <math.h>
+/* Globals that are added by the Lua libraries */
+static char *libraries_allow_list[] = {
+ "string",
+ "cjson",
+ "bit",
+ "cmsgpack",
+ "math",
+ "table",
+ "struct",
+ NULL,
+};
+
+/* Redis Lua API globals */
+static char *redis_api_allow_list[] = {
+ "redis",
+ "__redis__err__handler", /* error handler for eval, currently located on globals.
+ Should move to registry. */
+ NULL,
+};
+
+/* Lua builtins */
+static char *lua_builtins_allow_list[] = {
+ "xpcall",
+ "tostring",
+ "getfenv",
+ "setmetatable",
+ "next",
+ "assert",
+ "tonumber",
+ "rawequal",
+ "collectgarbage",
+ "getmetatable",
+ "rawset",
+ "pcall",
+ "coroutine",
+ "type",
+ "_G",
+ "select",
+ "unpack",
+ "gcinfo",
+ "pairs",
+ "rawget",
+ "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[] = {
+ "newproxy",
+ NULL,
+};
+
+/* Lua builtins which are allowed on initialization but will be removed right after */
+static char *lua_builtins_removed_after_initialization_allow_list[] = {
+ "debug", /* debug will be set to nil after the error handler will be created */
+ NULL,
+};
+
+/* Those allow lists was created from the globals that was
+ * available to the user when the allow lists was first introduce.
+ * Because we do not want to break backward compatibility we keep
+ * all the globals. The allow lists will prevent us from accidentally
+ * creating unwanted globals in the future.
+ *
+ * Also notice that the allow list is only checked on start time,
+ * after that the global table is locked so not need to check anything.*/
+static char **allow_lists[] = {
+ libraries_allow_list,
+ redis_api_allow_list,
+ lua_builtins_allow_list,
+ lua_builtins_not_documented_allow_list,
+ lua_builtins_removed_after_initialization_allow_list,
+ NULL,
+};
+
+/* Deny list contains elements which we know we do not want to add to globals
+ * and there is no need to print a warning message form them. We will print a
+ * log message only if an element was added to the globals and the element is
+ * not on the allow list nor on the back list. */
+static char *deny_list[] = {
+ "dofile",
+ "loadfile",
+ "print",
+ NULL,
+};
+
static int redis_math_random (lua_State *L);
static int redis_math_randomseed (lua_State *L);
static void redisProtocolToLuaType_Int(void *ctx, long long val, const char *proto, size_t proto_len);
@@ -1113,15 +1204,6 @@ static void luaLoadLibraries(lua_State *lua) {
#endif
}
-/* Remove a functions that we don't want to expose to the Redis scripting
- * environment. */
-static void luaRemoveUnsupportedFunctions(lua_State *lua) {
- lua_pushnil(lua);
- lua_setglobal(lua,"loadfile");
- lua_pushnil(lua);
- lua_setglobal(lua,"dofile");
-}
-
/* Return sds of the string value located on stack at the given index.
* Return NULL if the value is not a string. */
sds luaGetStringSds(lua_State *lua, int index) {
@@ -1144,30 +1226,111 @@ static int luaProtectedTableError(lua_State *lua) {
if (!lua_isstring(lua, -1) && !lua_isnumber(lua, -1)) {
luaL_error(lua, "Second argument to luaProtectedTableError must be a string or number");
}
- const char* variable_name = lua_tostring(lua, -1);
+ const char *variable_name = lua_tostring(lua, -1);
luaL_error(lua, "Script attempted to access nonexistent global variable '%s'", variable_name);
return 0;
}
-/* Set global protection on table on the top of the stack.
- * After called, it will no longer be possible to set
- * new items on the table. Any attempt to access a protected
- * table for write will result in an error. Any attempt to
- * get a none existing element from a locked table will result
- * in an error.
+/* Set a special metatable on the table on the top of the stack.
+ * The metatable will raise an error if the user tries to fetch
+ * an un-existing value.
*
* The function assumes the Lua stack have a least enough
* space to push 2 element, its up to the caller to verify
* this before calling this function. */
-void luaSetGlobalProtection(lua_State *lua) {
+void luaSetErrorMetatable(lua_State *lua) {
lua_newtable(lua); /* push metatable */
lua_pushcfunction(lua, luaProtectedTableError); /* push get error handler */
lua_setfield(lua, -2, "__index");
- lua_enablereadonlytable(lua, -1, 1); /* protect the metatable */
+ lua_setmetatable(lua, -2);
+}
+
+static int luaNewIndexAllowList(lua_State *lua) {
+ int argc = lua_gettop(lua);
+ if (argc != 3) {
+ serverLog(LL_WARNING, "malicious code trying to call luaProtectedTableError with wrong arguments");
+ luaL_error(lua, "Wrong number of arguments to luaNewIndexAllowList");
+ }
+ if (!lua_istable(lua, -3)) {
+ luaL_error(lua, "first argument to luaNewIndexAllowList must be a table");
+ }
+ if (!lua_isstring(lua, -2) && !lua_isnumber(lua, -2)) {
+ luaL_error(lua, "Second argument to luaNewIndexAllowList must be a string or number");
+ }
+ const char *variable_name = lua_tostring(lua, -2);
+ /* check if the key is in our allow list */
+
+ char ***allow_l = allow_lists;
+ for (; *allow_l ; ++allow_l){
+ char **c = *allow_l;
+ for (; *c ; ++c) {
+ if (strcmp(*c, variable_name) == 0) {
+ break;
+ }
+ }
+ if (*c) {
+ break;
+ }
+ }
+ if (!*allow_l) {
+ /* 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;
+ for ( ; *c ; ++c) {
+ if (strcmp(*c, variable_name) == 0) {
+ break;
+ }
+ }
+ if (!*c) {
+ serverLog(LL_WARNING, "A key '%s' was added to Lua globals which is not on the globals allow list nor listed on the deny list.", variable_name);
+ }
+ } else {
+ lua_rawset(lua, -3);
+ }
+ return 0;
+}
+/* Set a metatable with '__newindex' function that verify that
+ * the new index appears on our globals while list.
+ *
+ * The metatable is set on the table which located on the top
+ * of the stack.
+ */
+void luaSetAllowListProtection(lua_State *lua) {
+ lua_newtable(lua); /* push metatable */
+ lua_pushcfunction(lua, luaNewIndexAllowList); /* push get error handler */
+ lua_setfield(lua, -2, "__newindex");
lua_setmetatable(lua, -2);
+}
+/* Set the readonly flag on the table located on the top of the stack
+ * and recursively call this function on each table located on the original
+ * table. Also, recursively call this function on the metatables.*/
+void luaSetTableProtectionRecursively(lua_State *lua) {
+ /* This protect us from a loop in case we already visited the table
+ * For example, globals has '_G' key which is pointing back to globals. */
+ if (lua_isreadonlytable(lua, -1)) {
+ return;
+ }
+
+ /* protect the current table */
lua_enablereadonlytable(lua, -1, 1);
+
+ lua_checkstack(lua, 2);
+ lua_pushnil(lua); /* Use nil to start iteration. */
+ while (lua_next(lua,-2)) {
+ /* Stack now: table, key, value */
+ if (lua_istable(lua, -1)) {
+ luaSetTableProtectionRecursively(lua);
+ }
+ lua_pop(lua, 1);
+ }
+
+ /* protect the metatable if exists */
+ if (lua_getmetatable(lua, -1)) {
+ luaSetTableProtectionRecursively(lua);
+ lua_pop(lua, 1); /* pop the metatable */
+ }
}
void luaRegisterVersion(lua_State* lua) {
@@ -1204,8 +1367,11 @@ void luaRegisterLogFunction(lua_State* lua) {
}
void luaRegisterRedisAPI(lua_State* lua) {
+ lua_pushvalue(lua, LUA_GLOBALSINDEX);
+ luaSetAllowListProtection(lua);
+ lua_pop(lua, 1);
+
luaLoadLibraries(lua);
- luaRemoveUnsupportedFunctions(lua);
lua_pushcfunction(lua,luaRedisPcall);
lua_setglobal(lua, "pcall");
diff --git a/src/script_lua.h b/src/script_lua.h
index e6ca4f730955..4c2b34804e5e 100644
--- a/src/script_lua.h
+++ b/src/script_lua.h
@@ -68,7 +68,9 @@ typedef struct errorInfo {
void luaRegisterRedisAPI(lua_State* lua);
sds luaGetStringSds(lua_State *lua, int index);
void luaRegisterGlobalProtectionFunction(lua_State *lua);
-void luaSetGlobalProtection(lua_State *lua);
+void luaSetErrorMetatable(lua_State *lua);
+void luaSetAllowListProtection(lua_State *lua);
+void luaSetTableProtectionRecursively(lua_State *lua);
void luaRegisterLogFunction(lua_State* lua);
void luaRegisterVersion(lua_State* lua);
void luaPushErrorBuff(lua_State *lua, sds err_buff);
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index 9e2a4c4b24f7..439a1e71afee 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -778,6 +778,69 @@ start_server {tags {"scripting"}} {
} e
set _ $e
} {*Attempt to modify a readonly table*}
+
+ test "Try trick readonly table on redis table" {
+ catch {
+ run_script {
+ redis.call = function() return 1 end
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Try trick readonly table on json table" {
+ catch {
+ run_script {
+ cjson.encode = function() return 1 end
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Try trick readonly table on cmsgpack table" {
+ catch {
+ run_script {
+ cmsgpack.pack = function() return 1 end
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Try trick readonly table on bit table" {
+ catch {
+ run_script {
+ bit.lshift = function() return 1 end
+ } 0
+ } e
+ set _ $e
+ } {*Attempt to modify a readonly table*}
+
+ test "Test loadfile are not available" {
+ catch {
+ run_script {
+ loadfile('some file')
+ } 0
+ } e
+ set _ $e
+ } {*Script attempted to access nonexistent global variable 'loadfile'*}
+
+ test "Test dofile are not available" {
+ catch {
+ run_script {
+ dofile('some file')
+ } 0
+ } e
+ set _ $e
+ } {*Script attempted to access nonexistent global variable 'dofile'*}
+
+ test "Test print are not available" {
+ catch {
+ run_script {
+ print('some data')
+ } 0
+ } e
+ set _ $e
+ } {*Script attempted to access nonexistent global variable 'print'*}
}
# Start a new server since the last test in this stanza will kill the
