From: Jason Ish <jason.ish@oisf.net>
Date: Thu, 11 Apr 2024 17:06:29 +0000 (-0600)
Subject: lua: misc cleanups in sandbox implementation
X-Git-Tag: suricata-8.0.0-beta1~1261
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1f05a17fb99fb1cdb8e545d7b53e5749bd01d0b6;p=thirdparty%2Fsuricata.git

lua: misc cleanups in sandbox implementation

Including:
- rename guards
- SCMalloc to SCCalloc
- remove unused enum
- rename public functions to our naming standard
---

diff --git a/src/detect-lua.c b/src/detect-lua.c
index 78a94a4129..cab93693d4 100644
--- a/src/detect-lua.c
+++ b/src/detect-lua.c
@@ -150,7 +150,6 @@ void DetectLuaRegister(void)
 #define FLAG_DATATYPE_BUFFER                    BIT_U32(22)
 #define FLAG_ERROR_LOGGED                       BIT_U32(23)
 
-// TODO: move to config
 #define DEFAULT_LUA_ALLOC_LIMIT       500000
 #define DEFAULT_LUA_INSTRUCTION_LIMIT 500000
 
@@ -484,7 +483,7 @@ static void *DetectLuaThreadInit(void *data)
     t->alproto = lua->alproto;
     t->flags = lua->flags;
 
-    t->luastate = sb_newstate(lua->alloc_limit, lua->instruction_limit);
+    t->luastate = SCLuaSbStateNew(lua->alloc_limit, lua->instruction_limit);
     if (t->luastate == NULL) {
         SCLogError("luastate pool depleted");
         goto error;
@@ -493,7 +492,7 @@ static void *DetectLuaThreadInit(void *data)
     if (lua->allow_restricted_functions) {
         luaL_openlibs(t->luastate);
     } else {
-        sb_loadrestricted(t->luastate);
+        SCLuaSbLoadRestricted(t->luastate);
     }
 
     LuaRegisterExtensions(t->luastate);
@@ -534,7 +533,7 @@ static void *DetectLuaThreadInit(void *data)
 
 error:
     if (t->luastate != NULL)
-        sb_close(t->luastate);
+        SCLuaSbStateClose(t->luastate);
     SCFree(t);
     return NULL;
 }
@@ -544,7 +543,7 @@ static void DetectLuaThreadFree(void *ctx)
     if (ctx != NULL) {
         DetectLuaThreadData *t = (DetectLuaThreadData *)ctx;
         if (t->luastate != NULL)
-            sb_close(t->luastate);
+            SCLuaSbStateClose(t->luastate);
         SCFree(t);
     }
 }
@@ -590,13 +589,13 @@ static int DetectLuaSetupPrime(DetectEngineCtx *de_ctx, DetectLuaData *ld, const
 {
     int status;
 
-    lua_State *luastate = sb_newstate(ld->alloc_limit, ld->instruction_limit);
+    lua_State *luastate = SCLuaSbStateNew(ld->alloc_limit, ld->instruction_limit);
     if (luastate == NULL)
         return -1;
     if (ld->allow_restricted_functions) {
         luaL_openlibs(luastate);
     } else {
-        sb_loadrestricted(luastate);
+        SCLuaSbLoadRestricted(luastate);
     }
 
     /* hackish, needed to allow unittests to pass buffers as scripts instead of files */
@@ -876,10 +875,10 @@ static int DetectLuaSetupPrime(DetectEngineCtx *de_ctx, DetectLuaData *ld, const
 
     /* pop the table */
     lua_pop(luastate, 1);
-    sb_close(luastate);
+    SCLuaSbStateClose(luastate);
     return 0;
 error:
-    sb_close(luastate);
+    SCLuaSbStateClose(luastate);
     return -1;
 }
 
diff --git a/src/util-lua-sandbox.c b/src/util-lua-sandbox.c
index f0c1b7ea0f..2cb5ae2b7b 100644
--- a/src/util-lua-sandbox.c
+++ b/src/util-lua-sandbox.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2014-2023 Open Information Security Foundation
+/* Copyright (C) 2023-2024 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -36,19 +36,33 @@
 
 #include "util-lua-sandbox.h"
 
-typedef struct sb_block_function {
+#if !defined(SANDBOX_ALLOC_CTX)
+#define SANDBOX_CTX "SANDBOX_CTX"
+#endif
+
+typedef struct BlockedFunction {
     const char *module;
     const char *name;
-} sb_block_function;
+} BlockedFunction;
 
-static void sb_hook(lua_State *L, lua_Debug *ar);
-LUAMOD_API int luaopen_sandbox(lua_State *L);
+static void HookFunc(lua_State *L, lua_Debug *ar);
+static int OpenSandbox(lua_State *L);
 
-static void *sb_alloc(void *ud, void *ptr, size_t osize, size_t nsize)
+/**
+ * Lua allocator function provided to lua_newstate.
+ *
+ * \param ud The pointer passed to lua_newstate
+ * \param ptr Pointer to data being allocated/reallocated/freed
+ * \param osize Original size of the block
+ * \param nsize Size of the new block
+ *
+ * See: https://www.lua.org/manual/5.4/manual.html#lua_Alloc
+ */
+static void *LuaAlloc(void *ud, void *ptr, size_t osize, size_t nsize)
 {
     (void)ud;
     (void)osize; /* not used */
-    sb_lua_state *ctx = (sb_lua_state *)ud;
+    SCLuaSbState *ctx = (SCLuaSbState *)ud;
     if (nsize == 0) {
         if (ptr != NULL) {
             // ASSERT: alloc_bytes > osize
@@ -71,32 +85,53 @@ static void *sb_alloc(void *ud, void *ptr, size_t osize, size_t nsize)
     }
 }
 
-/*
-** These are the set of libs allowed in the restricted lua sandbox
-*/
-static const luaL_Reg sb_restrictedlibs[] = { { LUA_GNAME, luaopen_base },
-    //  {LUA_LOADLIBNAME, luaopen_package},
-    //  {LUA_COLIBNAME, luaopen_coroutine},
+/**
+ * Set of libs that are allowed and loaded into the Lua state.
+ */
+static const luaL_Reg AllowedLibs[] = {
+    // clang-format off
+    { LUA_GNAME, luaopen_base },
     { LUA_TABLIBNAME, luaopen_table },
-    //  {LUA_IOLIBNAME, luaopen_io},
-    //  {LUA_OSLIBNAME, luaopen_os},
-    { LUA_STRLIBNAME, luaopen_string }, { LUA_MATHLIBNAME, luaopen_math },
+    { LUA_STRLIBNAME, luaopen_string },
+    { LUA_MATHLIBNAME, luaopen_math },
     { LUA_UTF8LIBNAME, luaopen_utf8 },
-    { LUA_DBLIBNAME, luaopen_sandbox }, // TODO: remove this from restricted
-    { NULL, NULL } };
+
+    /* 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?
-static const sb_block_function sb_restrictedfuncs[] = { { LUA_GNAME, "collectgarbage" },
-    { LUA_GNAME, "dofile" }, { LUA_GNAME, "getmetatable" }, { LUA_GNAME, "loadfile" },
-    { LUA_GNAME, "load" }, { LUA_GNAME, "pcall" }, { LUA_GNAME, "setmetatable" },
+static const BlockedFunction BlockedFunctions[] = {
+    // clang-format off
+    { LUA_GNAME, "collectgarbage" },
+    { LUA_GNAME, "dofile" },
+    { LUA_GNAME, "getmetatable" },
+    { LUA_GNAME, "loadfile" },
+    { LUA_GNAME, "load" }, 
+    { LUA_GNAME, "pcall" },
+    { LUA_GNAME, "setmetatable" },
     { LUA_GNAME, "xpcall" },
 
-    { LUA_STRLIBNAME, "rep" }, // TODO:  probably don't need to block this for normal restricted
-                               // since we have memory limit
-    { NULL, NULL } };
+    /* 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 sb_loadlibs(lua_State *L, const luaL_Reg *libs)
+static void LoadAllowedLibs(lua_State *L, const luaL_Reg *libs)
 {
     const luaL_Reg *lib;
     /* "require" functions from 'loadedlibs' and set results to global table */
@@ -106,9 +141,12 @@ static void sb_loadlibs(lua_State *L, const luaL_Reg *libs)
     }
 }
 
-static void sb_block_functions(lua_State *L, const sb_block_function *funcs)
+/**
+ * Apply function blocking by replacing blocked functions with a nil.
+ */
+static void ApplyBlockedFunctions(lua_State *L, const BlockedFunction *funcs)
 {
-    const sb_block_function *func;
+    const BlockedFunction *func;
 
     // set target functions to nil
     lua_pushglobaltable(L);
@@ -123,15 +161,15 @@ static void sb_block_functions(lua_State *L, const sb_block_function *funcs)
     lua_pop(L, 1); // remove global table
 }
 
-LUALIB_API void sb_loadrestricted(lua_State *L)
+void SCLuaSbLoadRestricted(lua_State *L)
 {
-    sb_loadlibs(L, sb_restrictedlibs);
-    sb_block_functions(L, sb_restrictedfuncs);
+    LoadAllowedLibs(L, AllowedLibs);
+    ApplyBlockedFunctions(L, BlockedFunctions);
 }
 
-lua_State *sb_newstate(uint64_t alloclimit, uint64_t instructionlimit)
+lua_State *SCLuaSbStateNew(uint64_t alloclimit, uint64_t instructionlimit)
 {
-    sb_lua_state *sb = SCMalloc(sizeof(sb_lua_state));
+    SCLuaSbState *sb = SCCalloc(1, sizeof(SCLuaSbState));
     if (sb == NULL) {
         // Out of memory.  Error code?
         return NULL;
@@ -142,9 +180,8 @@ lua_State *sb_newstate(uint64_t alloclimit, uint64_t instructionlimit)
     sb->hook_instruction_count = 100;
     sb->instruction_limit = instructionlimit;
 
-    sb->L = lua_newstate(sb_alloc, sb); /* create state */
+    sb->L = lua_newstate(LuaAlloc, sb); /* create state */
     if (sb->L == NULL) {
-        // TODO: log or error code?
         SCFree(sb);
         return NULL;
     }
@@ -153,31 +190,34 @@ lua_State *sb_newstate(uint64_t alloclimit, uint64_t instructionlimit)
     lua_pushlightuserdata(sb->L, sb);
     lua_settable(sb->L, LUA_REGISTRYINDEX);
 
-    lua_sethook(sb->L, sb_hook, LUA_MASKCOUNT, sb->hook_instruction_count);
+    lua_sethook(sb->L, HookFunc, LUA_MASKCOUNT, sb->hook_instruction_count);
     return sb->L;
 }
 
-static sb_lua_state *sb_get_ctx(lua_State *L)
+static SCLuaSbState *GetContext(lua_State *L)
 {
     lua_pushstring(L, SANDBOX_CTX);
     lua_gettable(L, LUA_REGISTRYINDEX);
-    sb_lua_state *ctx = lua_touserdata(L, -1);
+    SCLuaSbState *ctx = lua_touserdata(L, -1);
     // TODO:  log if null?
     lua_pop(L, 1);
     return ctx;
 }
 
-void sb_close(lua_State *L)
+void SCLuaSbStateClose(lua_State *L)
 {
-    sb_lua_state *sb = sb_get_ctx(L);
+    SCLuaSbState *sb = GetContext(L);
     lua_close(sb->L);
     SCFree(sb);
 }
 
-static void sb_hook(lua_State *L, lua_Debug *ar)
+/**
+ * Lua debugging hook, but used here for instruction limit counting.
+ */
+static void HookFunc(lua_State *L, lua_Debug *ar)
 {
     (void)ar;
-    sb_lua_state *sb = sb_get_ctx(L);
+    SCLuaSbState *sb = GetContext(L);
 
     sb->instruction_count += sb->hook_instruction_count;
 
@@ -187,35 +227,38 @@ static void sb_hook(lua_State *L, lua_Debug *ar)
     }
 }
 
-void sb_resetinstructioncounter(lua_State *L)
+/**
+ * Reset the instruction counter for the provided state.
+ */
+void SCLuaSbResetInstructionCounter(lua_State *L)
 {
-    sb_lua_state *sb = sb_get_ctx(L);
+    SCLuaSbState *sb = GetContext(L);
     if (sb != NULL) {
         sb->instruction_count = 0;
-        lua_sethook(L, sb_hook, LUA_MASKCOUNT, sb->hook_instruction_count);
+        lua_sethook(L, HookFunc, LUA_MASKCOUNT, sb->hook_instruction_count);
     }
 }
 
-void sb_setinstructionlimit(lua_State *L, uint64_t instruction_limit)
+static void SetInstructionCount(lua_State *L, uint64_t instruction_limit)
 {
-    sb_lua_state *ctx = sb_get_ctx(L);
+    SCLuaSbState *ctx = GetContext(L);
     if (ctx != NULL) {
         ctx->instruction_limit = instruction_limit;
     }
 }
 
-static uint64_t sb_getinstructioncount(lua_State *L)
+static uint64_t GetInstructionCount(lua_State *L)
 {
-    sb_lua_state *ctx = sb_get_ctx(L);
+    SCLuaSbState *ctx = GetContext(L);
     if (ctx != NULL) {
         return ctx->instruction_count;
     }
     return 0;
 }
 
-static int sb_Ltotalalloc(lua_State *L)
+static int L_TotalAlloc(lua_State *L)
 {
-    sb_lua_state *ctx = sb_get_ctx(L);
+    SCLuaSbState *ctx = GetContext(L);
     if (ctx != NULL) {
         lua_pushinteger(L, ctx->alloc_bytes);
     } else {
@@ -224,9 +267,9 @@ static int sb_Ltotalalloc(lua_State *L)
     return 1;
 }
 
-static int sb_Lgetalloclimit(lua_State *L)
+static int L_GetAllocLimit(lua_State *L)
 {
-    sb_lua_state *ctx = sb_get_ctx(L);
+    SCLuaSbState *ctx = GetContext(L);
     if (ctx != NULL) {
         lua_pushinteger(L, ctx->alloc_limit);
     } else {
@@ -235,34 +278,24 @@ static int sb_Lgetalloclimit(lua_State *L)
     return 1;
 }
 
-static int sb_Lsetalloclimit(lua_State *L)
+static int L_SetAllocLimit(lua_State *L)
 {
-    sb_lua_state *ctx = sb_get_ctx(L);
+    SCLuaSbState *ctx = GetContext(L);
     if (ctx != NULL) {
         ctx->alloc_limit = luaL_checkinteger(L, 1);
     }
     return 0;
 }
 
-/*
-static int sb_Lsetlevel(lua_State *L)
+static int L_GetInstructionCount(lua_State *L)
 {
-    lua_Integer level = luaL_checkinteger(L, 1);
-    lua_pushnil(L);
-    lua_setglobal(L, "debug");
-    return 0;
-}
-*/
-
-static int sb_Lgetinstructioncount(lua_State *L)
-{
-    lua_pushinteger(L, sb_getinstructioncount(L));
+    lua_pushinteger(L, GetInstructionCount(L));
     return 1;
 }
 
-static int sb_Lgetinstructionlimit(lua_State *L)
+static int L_GetInstructionLimit(lua_State *L)
 {
-    sb_lua_state *ctx = sb_get_ctx(L);
+    SCLuaSbState *ctx = GetContext(L);
     if (ctx != NULL) {
         lua_pushinteger(L, ctx->instruction_limit);
     } else {
@@ -271,30 +304,35 @@ static int sb_Lgetinstructionlimit(lua_State *L)
     return 1;
 }
 
-static int sb_Lsetinstructionlimit(lua_State *L)
+static int L_SetInstructionLimit(lua_State *L)
 {
-    sb_setinstructionlimit(L, luaL_checkinteger(L, 1));
+    SetInstructionCount(L, luaL_checkinteger(L, 1));
     return 0;
 }
 
-static int sb_Lresetinstructioncount(lua_State *L)
+static int L_ResetInstructionCount(lua_State *L)
 {
-    sb_resetinstructioncounter(L);
+    SCLuaSbResetInstructionCounter(L);
     return 0;
 }
 
-static const luaL_Reg sblib[] = { { "totalalloc", sb_Ltotalalloc },
-    { "getalloclimit", sb_Lgetalloclimit }, { "setalloclimit", sb_Lsetalloclimit },
-    // { "setlevel", sb_Lsetlevel },
-    { "instructioncount", sb_Lgetinstructioncount },
-    { "getinstructionlimit", sb_Lgetinstructionlimit },
-    { "setinstructionlimit", sb_Lsetinstructionlimit },
-    { "resetinstructioncount", sb_Lresetinstructioncount }, { NULL, NULL } };
-
-LUAMOD_API int luaopen_sandbox(lua_State *L)
+static const luaL_Reg sblib[] = {
+    // clang-format off
+    { "totalalloc", L_TotalAlloc },
+    { "getalloclimit", L_GetAllocLimit },
+    { "setalloclimit", L_SetAllocLimit },
+    { "instructioncount", L_GetInstructionCount },
+    { "getinstructionlimit", L_GetInstructionLimit },
+    { "setinstructionlimit", L_SetInstructionLimit },
+    { "resetinstructioncount", L_ResetInstructionCount },
+    { NULL, NULL }
+    // clang-format on
+};
+
+static int OpenSandbox(lua_State *L)
 {
     luaL_newlib(L, sblib);
     return 1;
 }
 
-#endif
\ No newline at end of file
+#endif
diff --git a/src/util-lua-sandbox.h b/src/util-lua-sandbox.h
index 42ad7facf8..2d28550799 100644
--- a/src/util-lua-sandbox.h
+++ b/src/util-lua-sandbox.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2014-2023 Open Information Security Foundation
+/* Copyright (C) 2023-2024 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -21,45 +21,33 @@
  * \author Jo Johnson <pyrojoe314@gmail.com>
  */
 
-#ifndef __UTIL_LUA_SANDBOX_H__
-#define __UTIL_LUA_SANDBOX_H__
+#ifndef SURICATA_UTIL_LUA_SANDBOX_H
+#define SURICATA_UTIL_LUA_SANDBOX_H
 
-#ifndef HAVE_LUA
-/* If we don't have Lua, create a typedef for sb_lua_State so the
- * exported Lua functions don't fail the build. */
-typedef void sb_lua_state;
-#else
-#include <stdlib.h>
 #include "lua.h"
 
-#if !defined(SANDBOX_ALLOC_CTX)
-#define SANDBOX_CTX "SANDBOX_CTX"
-#endif
-
 /*
  *  Lua sandbox usage:  The only needed changes to use the sandboxed lua state are
- *      to replace calls to lua_newstate and lua_close with sb_newstate and sb_close
- *      Additionally, sb_loadrestricted can be used to load a restricted set of packages
+ *      to replace calls to lua_newstate and lua_close with SCLuaSbStateNew and SCLuaSbStateClose
+ *      Additionally, SCLuaSbLoadRestricted can be used to load a restricted set of packages
  *      that prevent side effecting outside of the lua runtime
  */
 
 /*
  *  Struct to store a lua_state and the additional metadata required to sandbox it
  */
-typedef struct sb_lua_state {
+typedef struct SCLuaSbState {
     lua_State *L;
 
-    // Allocation limits
+    /* Allocation limits */
     uint64_t alloc_bytes;
     uint64_t alloc_limit;
 
-    // Execution Limits
+    /* Execution Limits */
     uint64_t instruction_count;
     uint64_t instruction_limit;
     uint64_t hook_instruction_count;
-} sb_lua_state;
-
-enum sb_level { NONE, EXTERNAL_RESTRICTED, PERFORMANCE_RESTRICTED };
+} SCLuaSbState;
 
 /*
  *  Replaces luaL_newstate.  Sets an upper bound for allocations and bytecode
@@ -70,29 +58,22 @@ enum sb_level { NONE, EXTERNAL_RESTRICTED, PERFORMANCE_RESTRICTED };
  *  instructionlimit - maximum number of lua bytecode instructions before an error is thrown
  *      A value of zero will not limit the number of instructions
  */
-lua_State *sb_newstate(uint64_t alloclimit, uint64_t instructionlimit);
+lua_State *SCLuaSbStateNew(uint64_t alloclimit, uint64_t instructionlimit);
 
 /*
- *  Replaces lua_close.  Handles freeing the sb_lua_state
+ *  Replaces lua_close.  Handles freeing the SCLuaSbState
  */
-void sb_close(lua_State *sb);
+void SCLuaSbStateClose(lua_State *sb);
 
 /*
  *  Resets the instruction counter for the sandbox to 0
  */
-void sb_resetinstructioncounter(lua_State *sb);
-
-/*
- *  Sets the maximum number of lua instructions before erroring out
- */
-void sb_setinstructionlimit(lua_State *L, uint64_t instruction_limit);
+void SCLuaSbResetInstructionCounter(lua_State *sb);
 
 /*
- *  Replaces luaL_openlibs.  Only opens allowed paackages for the sandbox and
+ *  Replaces luaL_openlibs.  Only opens allowed packages for the sandbox and
  *  masks out dangerous functions from the base.
  */
-LUALIB_API void sb_loadrestricted(lua_State *L);
-
-#endif /* HAVE_LUA */
+void SCLuaSbLoadRestricted(lua_State *L);
 
-#endif /*  __UTIL_LUA_SANDBOX_H__ */
\ No newline at end of file
+#endif /*  SURICATA_UTIL_LUA_SANDBOX_H */