summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormeir <meir@redis.com>2022-02-06 14:30:15 +0200
committermeir <meir@redis.com>2022-04-27 00:20:54 +0300
commit992f9e23c7ee819ad0dfac0bd6224d8330366960 (patch)
treed5753f6f503293504dc16b7a24925a335c64566f
parent8b33d813a3d47d4daeaaef03b7e42a51f6931f79 (diff)
downloadredis-992f9e23c7ee819ad0dfac0bd6224d8330366960.tar.gz
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 ```
-rw-r--r--src/eval.c29
-rw-r--r--src/script_lua.c4
-rw-r--r--src/script_lua.h2
3 files changed, 10 insertions, 25 deletions
diff --git a/src/eval.c b/src/eval.c
index 4894b83d3..1be0d42f2 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 9a08a7e47..4e1f17649 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 5a4533784..f39c01744 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);