diff options
author | meir <meir@redis.com> | 2022-03-22 11:42:48 +0200 |
---|---|---|
committer | Oran Agra <oran@redislabs.com> | 2022-04-27 16:31:52 +0300 |
commit | 21414ad4802659eeb3e81d20d86b437d3ab882dc (patch) | |
tree | 10aa6f6ac155fa5710afdce4f034742f32285344 | |
parent | 13c1e1f2986adcd8788f464a1931cdfe744f15f9 (diff) | |
download | redis-21414ad4802659eeb3e81d20d86b437d3ab882dc.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/scripting.c | 29 |
1 files changed, 7 insertions, 22 deletions
diff --git a/src/scripting.c b/src/scripting.c index ccd78a8bf..cc3924c7a 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -1104,7 +1104,7 @@ void scriptingEnableGlobalsProtection(lua_State *lua) { 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 ~= \"main\" and 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"; @@ -1423,14 +1423,7 @@ sds luaCreateFunction(client *c, lua_State *lua, robj *body) { return dictGetKey(de); } - sds funcdef = sdsempty(); - funcdef = sdscat(funcdef,"function "); - funcdef = sdscatlen(funcdef,funcname,42); - funcdef = sdscatlen(funcdef,"() ",3); - funcdef = sdscatlen(funcdef,body->ptr,sdslen(body->ptr)); - funcdef = sdscatlen(funcdef,"\nend",4); - - if (luaL_loadbuffer(lua,funcdef,sdslen(funcdef),"@user_script")) { + if (luaL_loadbuffer(lua,body->ptr,sdslen(body->ptr),"@user_script")) { if (c != NULL) { addReplyErrorFormat(c, "Error compiling script (new function): %s\n", @@ -1438,20 +1431,12 @@ sds luaCreateFunction(client *c, lua_State *lua, robj *body) { } lua_pop(lua,1); sdsfree(sha); - sdsfree(funcdef); return NULL; } - sdsfree(funcdef); - if (lua_pcall(lua,0,0,0)) { - if (c != NULL) { - addReplyErrorFormat(c,"Error running script (new function): %s\n", - lua_tostring(lua,-1)); - } - lua_pop(lua,1); - sdsfree(sha); - return NULL; - } + serverAssert(lua_isfunction(lua, -1)); + + lua_setfield(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 @@ -1579,7 +1564,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 @@ -1597,7 +1582,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)); } |