lua: use a function allow list instead of a deny list

The Lua library surface area is small enough to manage an allow list,
which is generally better than a deny list, as we'll explicitly need
to opt-in to new functions provided by the Lua runtime.

src/detect-lua.c

@@ -469,7 +469,7 @@ static void *DetectLuaThreadInit(void *data)
     if (lua->allow_restricted_functions) {
         luaL_openlibs(t->luastate);
     } else {
-        SCLuaSbLoadRestricted(t->luastate);
+        SCLuaSbLoadLibs(t->luastate);
     }
 
     LuaRegisterExtensions(t->luastate);
@@ -572,7 +572,7 @@ static int DetectLuaSetupPrime(DetectEngineCtx *de_ctx, DetectLuaData *ld, const
     if (ld->allow_restricted_functions) {
         luaL_openlibs(luastate);
     } else {
-        SCLuaSbLoadRestricted(luastate);
+        SCLuaSbLoadLibs(luastate);
     }
 
     /* hackish, needed to allow unittests to pass buffers as scripts instead of files */

src/util-lua-sandbox.c

@@ -26,6 +26,7 @@
 #include "lua.h"
 #include "lauxlib.h"
 #include "lualib.h"
+#include "util-debug.h"
 
 #include "util-debug.h"
 #include "util-validate.h"
@@ -33,11 +34,6 @@
 
 #define SANDBOX_CTX "SANDBOX_CTX"
 
-typedef struct BlockedFunction {
-    const char *module;
-    const char *name;
-} BlockedFunction;
-
 static void HookFunc(lua_State *L, lua_Debug *ar);
 static int OpenSandbox(lua_State *L);
 
@@ -79,6 +75,160 @@ static void *LuaAlloc(void *ud, void *ptr, size_t osize, size_t nsize)
     }
 }
 
+/**
+ * Function put in place of Lua functions that are blocked.
+ *
+ * TODO: Might want to create a version of this for each library that
+ * has blocked functions, so it can display the name of the
+ * library. As it doesn't appear that can be retrieved.
+ */
+static int LuaBlockedFunction(lua_State *L)
+{
+    lua_Debug ar;
+    lua_getstack(L, 0, &ar);
+    lua_getinfo(L, "n", &ar);
+    if (ar.name) {
+        luaL_error(L, "Blocked Lua function called: %s", ar.name);
+    } else {
+        luaL_error(L, "Blocked Lua function: name not available");
+    }
+    return -1;
+}
+
+/**
+ * Check if a Lua function in a specific module is allowed.
+ *
+ * This is essentially an allow list for Lua functions.
+ */
+static bool IsAllowed(const char *module, const char *fname)
+{
+    static const char *base_allowed[] = {
+        "assert",
+        "ipairs",
+        "next",
+        "pairs",
+        "print",
+        "rawequal",
+        "rawlen",
+        "select",
+        "tonumber",
+        "tostring",
+        "type",
+        "warn",
+        "rawget",
+        "rawset",
+        "error",
+        NULL,
+    };
+
+    /* Allow all. */
+    static const char *table_allowed[] = {
+        "concat",
+        "insert",
+        "move",
+        "pack",
+        "remove",
+        "sort",
+        "unpack",
+        NULL,
+    };
+
+    /* Allow all. */
+    static const char *string_allowed[] = {
+        "byte",
+        "char",
+        "dump",
+        "find",
+        "format",
+        "gmatch",
+        "gsub",
+        "len",
+        "lower",
+        "match",
+        "pack",
+        "packsize",
+        "rep",
+        "reverse",
+        "sub",
+        "unpack",
+        "upper",
+        NULL,
+    };
+
+    /* Allow all. */
+    static const char *math_allowed[] = {
+        "abs",
+        "acos",
+        "asin",
+        "atan",
+        "atan2",
+        "ceil",
+        "cos",
+        "cosh",
+        "deg",
+        "exp",
+        "floor",
+        "fmod",
+        "frexp",
+        "ldexp",
+        "log",
+        "log10",
+        "max",
+        "min",
+        "modf",
+        "pow",
+        "rad",
+        "random",
+        "randomseed",
+        "sin",
+        "sinh",
+        "sqrt",
+        "tan",
+        "tanh",
+        "tointeger",
+        "type",
+        "ult",
+        NULL,
+    };
+
+    /* Allow all. */
+    static const char *utf8_allowed[] = {
+        "offset",
+        "len",
+        "codes",
+        "char",
+        "codepoint",
+        NULL,
+    };
+
+    const char **allowed = NULL;
+
+    if (strcmp(module, LUA_GNAME) == 0) {
+        allowed = base_allowed;
+    } else if (strcmp(module, LUA_TABLIBNAME) == 0) {
+        allowed = table_allowed;
+    } else if (strcmp(module, LUA_STRLIBNAME) == 0) {
+        allowed = string_allowed;
+    } else if (strcmp(module, LUA_MATHLIBNAME) == 0) {
+        allowed = math_allowed;
+    } else if (strcmp(module, LUA_UTF8LIBNAME) == 0) {
+        allowed = utf8_allowed;
+    } else {
+        /* This is a programming error. */
+        FatalError("Unknown Lua module %s", module);
+    }
+
+    if (allowed) {
+        for (int i = 0; allowed[i] != NULL; i++) {
+            if (strcmp(allowed[i], fname) == 0) {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
 /**
  * Set of libs that are allowed and loaded into the Lua state.
  */
@@ -89,83 +239,56 @@ static const luaL_Reg AllowedLibs[] = {
     { LUA_STRLIBNAME, luaopen_string },
     { LUA_MATHLIBNAME, luaopen_math },
     { LUA_UTF8LIBNAME, luaopen_utf8 },
-
-    /* TODO: Review these libs... */
-#if 0
-    {LUA_LOADLIBNAME, luaopen_package},
-    {LUA_COLIBNAME, luaopen_coroutine},
-    {LUA_IOLIBNAME, luaopen_io},
-    {LUA_OSLIBNAME, luaopen_os},
-#endif
-
-    /* What is this for? */
-    { LUA_DBLIBNAME, OpenSandbox }, // TODO: remove this from restricted
-
     { NULL, NULL }
     // clang-format on
 };
 
-// TODO: should we block raw* functions?
+/**
-// TODO: Will we ever need to block a subset of functions more than one level deep?
+ * Load allowed Lua libraries into the state.
-static const BlockedFunction BlockedFunctions[] = {
+ *
-    // clang-format off
+ * Functions from each library that are not in the allowed list are
-    { LUA_GNAME, "collectgarbage" },
+ * replaced with LuaBlockedFunction.
-    { LUA_GNAME, "dofile" },
+ */
-    { LUA_GNAME, "getmetatable" },
+void SCLuaSbLoadLibs(lua_State *L)
-    { LUA_GNAME, "loadfile" },
-    { LUA_GNAME, "load" }, 
-    { LUA_GNAME, "pcall" },
-    { LUA_GNAME, "setmetatable" },
-    { LUA_GNAME, "xpcall" },
-
-    /* TODO:  probably don't need to block this for normal restricted
-     * since we have memory limit */
-    { LUA_STRLIBNAME, "rep" }, 
-    { NULL, NULL }
-    // clang-format on
-};
-
-static void LoadAllowedLibs(lua_State *L, const luaL_Reg *libs)
 {
     const luaL_Reg *lib;
-    /* "require" functions from 'loadedlibs' and set results to global table */
+
-    for (lib = libs; lib->func; lib++) {
+    for (lib = AllowedLibs; lib->func; lib++) {
         luaL_requiref(L, lib->name, lib->func, 1);
-        lua_pop(L, 1); /* remove lib */
+        lua_pop(L, 1);
+        /* Iterate over all the functions in the just loaded table and
+         * replace functions now on the allow list with our blocked
+         * function placeholder. */
+        lua_getglobal(L, lib->name);
+        lua_pushnil(L);
+        while (lua_next(L, -2)) {
+            if (lua_type(L, -1) == LUA_TFUNCTION) {
+                const char *name = lua_tostring(L, -2);
+                if (!IsAllowed(lib->name, name)) {
+                    SCLogDebug("Blocking Lua function %s.%s", lib->name, name);
+                    lua_pushstring(L, name);
+                    lua_pushcfunction(L, LuaBlockedFunction);
+                    lua_settable(L, -5);
+                } else {
+                    SCLogDebug("Allowing Lua function %s.%s", lib->name, name);
+                }
+            }
+            lua_pop(L, 1);
+        }
+        lua_pop(L, 1);
     }
 }
 
 /**
- * Apply function blocking by replacing blocked functions with a nil.
+ * \brief Allocate a new Lua sandbox.
+ *
+ * \returns An allocated sandbox state or NULL if memory allocation
+ *     fails.
  */
-static void ApplyBlockedFunctions(lua_State *L, const BlockedFunction *funcs)
-{
-    const BlockedFunction *func;
-
-    // set target functions to nil
-    lua_pushglobaltable(L);
-    for (func = funcs; func->module; func++) {
-        lua_pushstring(L, func->module);
-        lua_gettable(L, -2); // load module to stack
-        lua_pushstring(L, func->name);
-        lua_pushnil(L);
-        lua_settable(L, -3);
-        lua_pop(L, 1); // remove module from the stack
-    }
-    lua_pop(L, 1); // remove global table
-}
-
-void SCLuaSbLoadRestricted(lua_State *L)
-{
-    LoadAllowedLibs(L, AllowedLibs);
-    ApplyBlockedFunctions(L, BlockedFunctions);
-}
-
 lua_State *SCLuaSbStateNew(uint64_t alloclimit, uint64_t instructionlimit)
 {
     SCLuaSbState *sb = SCCalloc(1, sizeof(SCLuaSbState));
     if (sb == NULL) {
-        // Out of memory.  Error code?
         return NULL;
     }
 

src/util-lua-sandbox.h

@@ -74,6 +74,6 @@ void SCLuaSbResetInstructionCounter(lua_State *sb);
  *  Replaces luaL_openlibs.  Only opens allowed packages for the sandbox and
  *  masks out dangerous functions from the base.
  */
-void SCLuaSbLoadRestricted(lua_State *L);
+void SCLuaSbLoadLibs(lua_State *L);
 
 #endif /*  SURICATA_UTIL_LUA_SANDBOX_H */