Scripting: globals protection can now be switched on/off.

redis.conf

@@ -392,6 +392,23 @@ auto-aof-rewrite-min-size 64mb
 # Set it to 0 or a negative value for unlimited execution without warnings.
 lua-time-limit 5000
 
+# By default variables in a Lua script are global, this means that a correct
+# script must declare all the local variables explicitly using the 'local'
+# keyword. Lua beginners are known to violate this rule, polluting the global
+# namespace, or creating scripts that may fail under certain conditions, for
+# this reason by default Redis installs a protection that will raise an error
+# every time a script attempts to access a global variable that was not
+# explicitly declared via global().
+#
+# It's worth to note that normal Redis scripts should never use globals, but
+# we don't entirely disable the possibility because from time to time crazy
+# things in the right hands can be pretty powerful.
+#
+# Globals protection may result into a minor performance hint, so it is
+# possible to disable the feature in production environments using the
+# following configuration directive, or at runtime using CONFIG SET.
+lua-protect-globals yes
+
 ################################ REDIS CLUSTER  ###############################
 #
 # Normal Redis instances can't be part of a Redis Cluster, only nodes that are

src/config.c

@@ -311,6 +311,10 @@ void loadServerConfigFromString(char *config) {
             server.cluster.configfile = zstrdup(argv[1]);
         } else if (!strcasecmp(argv[0],"lua-time-limit") && argc == 2) {
             server.lua_time_limit = strtoll(argv[1],NULL,10);
+        } else if (!strcasecmp(argv[0],"lua-protect-globals") && argc == 2) {
+            if ((server.lua_protect_globals = yesnotoi(argv[1])) == -1) {
+                err = "argument must be 'yes' or 'no'"; goto loaderr;
+            }
         } else if (!strcasecmp(argv[0],"slowlog-log-slower-than") &&
                    argc == 2)
         {
@@ -552,6 +556,16 @@ void configSetCommand(redisClient *c) {
     } else if (!strcasecmp(c->argv[2]->ptr,"lua-time-limit")) {
         if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll < 0) goto badfmt;
         server.lua_time_limit = ll;
+    } else if (!strcasecmp(c->argv[2]->ptr,"lua-protect-globals")) {
+        int enable = yesnotoi(o->ptr);
+
+        if (enable == -1) goto badfmt;
+        if (enable == 0 && server.lua_protect_globals == 1) {
+            scriptingDisableGlobalsProtection(server.lua);
+        } else if (enable && server.lua_protect_globals == 0) {
+            scriptingEnableGlobalsProtection(server.lua);
+        }
+        server.lua_protect_globals = enable;
     } else if (!strcasecmp(c->argv[2]->ptr,"slowlog-log-slower-than")) {
         if (getLongLongFromObject(o,&ll) == REDIS_ERR) goto badfmt;
         server.slowlog_log_slower_than = ll;
@@ -735,6 +749,7 @@ void configGetCommand(redisClient *c) {
     config_get_bool_field("daemonize", server.daemonize);
     config_get_bool_field("rdbcompression", server.rdb_compression);
     config_get_bool_field("activerehashing", server.activerehashing);
+    config_get_bool_field("lua-protect-globals", server.lua_protect_globals);
 
     /* Everything we can't handle with macros follows. */
 

src/redis.c

@@ -1068,6 +1068,7 @@ void initServerConfig() {
     server.lua_time_limit = REDIS_LUA_TIME_LIMIT;
     server.lua_client = NULL;
     server.lua_timedout = 0;
+    server.lua_protect_globals = 1;
 
     updateLRUClock();
     resetServerSaveParams();

src/redis.h

@@ -717,6 +717,7 @@ struct redisServer {
     int lua_timedout;     /* True if we reached the time limit for script
                              execution. */
     int lua_kill;         /* Kill the script if true. */
+    int lua_protect_globals;    /* If true globals must be declared */
     /* Assert & bug reportign */
     char *assert_failed;
     char *assert_file;
@@ -1102,6 +1103,8 @@ void clusterPropagatePublish(robj *channel, robj *message);
 
 /* Scripting */
 void scriptingInit(void);
+void scriptingEnableGlobalsProtection(lua_State *lua);
+void scriptingDisableGlobalsProtection(lua_State *lua);
 
 /* Git SHA1 */
 char *redisGitSHA1(void);

src/scripting.c

@@ -412,45 +412,57 @@ void luaLoadLibraries(lua_State *lua) {
 #endif
 }
 
-void scriptingProtectGlobals(lua_State *lua) {
-    char *s[26];
+/* 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.
+ * Note that the function is designed to be called multiple times if needed
+ * without issues, because it is possible to enabled/disable globals protection
+ * at runtime with CONFIG SET. */
+void scriptingEnableGlobalsProtection(lua_State *lua) {
+    char *s[32];
     sds code = sdsempty();
-    int j;
+    int j = 0;
 
-    /* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html */
-    s[0]="local mt = getmetatable(_G)\n";
-    s[1]="if mt == nil then\n";
-    s[2]="  mt = {}\n";
-    s[3]="  setmetatable(_G, mt)\n";
-    s[4]="end\n";
-    s[5]="__STRICT = true\n";
-    s[6]="mt.__declared = {}\n";
-    s[7]="mt.__newindex = function (t, n, v)\n";
-    s[8]="  if __STRICT and not mt.__declared[n] and debug.getinfo(2) then\n";
-    s[9]="    local w = debug.getinfo(2, \"S\").what\n";
-    s[10]="    if w ~= \"main\" and w ~= \"C\" then\n";
-    s[11]="      error(\"assign to undeclared global var '\"..n..\"'\", 2)\n";
-    s[12]="    end\n";
-    s[13]="    mt.__declared[n] = true\n";
-    s[14]="  end\n";
-    s[15]="  rawset(t, n, v)\n";
-    s[16]="end\n";
-    s[17]="mt.__index = function (t, n)\n";
-    s[18]="  if debug.getinfo(2) and not mt.__declared[n] and debug.getinfo(2, \"S\").what ~= \"C\" then\n";
-    s[19]="    error(\"global var '\"..n..\"' is not declared\", 2)\n";
-    s[20]="  end\n";
-    s[21]="  return rawget(t, n)\n";
-    s[22]="end\n";
-    s[23]="function global(...)\n";
-    s[24]="   for _, v in ipairs{...} do mt.__declared[v] = true end\n";
-    s[25]="end\n";
-
-    for (j = 0; j < 26; j++) code = sdscatlen(code,s[j],strlen(s[j]));
-    luaL_loadbuffer(lua,code,sdslen(code),"strict_lua");
+    /* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html.
+     * Modified to be adapted to Redis. */
+    s[j++]="mt = {}\n";
+    s[j++]="setmetatable(_G, mt)\n";
+    s[j++]="mt.declared = {}\n";
+    s[j++]="mt.__newindex = function (t, n, v)\n";
+    s[j++]="  if not mt.declared[n] and debug.getinfo(2) then\n";
+    s[j++]="    local w = debug.getinfo(2, \"S\").what\n";
+    s[j++]="    if w ~= \"main\" and w ~= \"C\" then\n";
+    s[j++]="      error(\"assignment to undeclared global variable '\"..n..\"'\", 2)\n";
+    s[j++]="    end\n";
+    s[j++]="    mt.declared[n] = true\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 debug.getinfo(2) and not mt.declared[n] and debug.getinfo(2, \"S\").what ~= \"C\" then\n";
+    s[j++]="    error(\"global variable '\"..n..\"' is not declared\", 2)\n";
+    s[j++]="  end\n";
+    s[j++]="  return rawget(t, n)\n";
+    s[j++]="end\n";
+    s[j++]="function global(...)\n";
+    s[j++]="   for _, v in ipairs{...} do mt.declared[v] = true end\n";
+    s[j++]="end\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);
 }
 
+void scriptingDisableGlobalsProtection(lua_State *lua) {
+    char *s = "setmetatable(_G, nil)\n";
+    luaL_loadbuffer(lua,s,strlen(s),"disable_strict_lua");
+    lua_pcall(lua,0,0,0);
+}
+
 /* Initialize the scripting environment.
  * It is possible to call this function to reset the scripting environment
  * assuming that we call scriptingRelease() before.
@@ -543,7 +555,8 @@ void scriptingInit(void) {
     /* Lua beginners ofter don't use "local", this is likely to introduce
      * subtle bugs in their code. To prevent problems we protect accesses
      * to global variables. */
-    scriptingProtectGlobals(lua);
+    if (server.lua_protect_globals)
+        scriptingEnableGlobalsProtection(lua);
 
     server.lua = lua;
 }