Pull request #5046: appid: check for Lua table errors during initialization and cleanup

Merge in SNORT/snort3 from ~OSTEPANO/snort3:lua_table_error_handle to master

Squashed commit of the following:

commit 7367e3e513f0b6f362544791fd763f4a1ded975e
Author: Oleksandr Stepanov <ostepano@cisco.com>
Date:   Wed Dec 10 07:45:22 2025 -0500

    appid: check for Lua table errors during initialization and cleanup

diff --git a/src/network_inspectors/appid/lua_detector_api.cc b/src/network_inspectors/appid/lua_detector_api.cc
index af93127f0120ccb8416decb8bd977fbae81ff5e5..5a39e748f2402e0fdd9b088756d7d13a76ffd8cd 100644 (file)
--- a/src/network_inspectors/appid/lua_detector_api.cc
+++ b/src/network_inspectors/appid/lua_detector_api.cc
@@ -3375,6 +3375,26 @@ static int set_user_detector_data_item(lua_State *L)
     return result;
 }
 
+#ifdef REG_TEST
+int lua_remove_registry_table_test(lua_State* L)
+{
+    const char* table_entry = lua_tostring(L, 2);
+    if (!table_entry)
+    {
+        return 0;
+    }
+    lua_getfield(L, LUA_REGISTRYINDEX, table_entry);
+    if (lua_isnil(L, -1))
+    {
+        APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "appid: Registry table %s not found \n", table_entry);
+        return 0;
+    }
+    lua_pushnil(L);
+    lua_setfield(L, LUA_REGISTRYINDEX, table_entry);
+    return 1;
+}
+#endif
+
 static const luaL_Reg detector_methods[] =
 {
     /* Obsolete API names.  No longer use these!  They are here for backward
@@ -3507,6 +3527,10 @@ static const luaL_Reg detector_methods[] =
     {"addCipService",            detector_add_cip_service},
     {"addEnipCommand",           detector_add_enip_command},
 
+#ifdef REG_TEST
+    {"luaRemoveRegistryTableTest", lua_remove_registry_table_test},
+#endif
+
     { nullptr, nullptr }
 };
 
@@ -3603,7 +3627,7 @@ int LuaStateDescriptor::lua_validate(AppIdDiscoveryArgs& args)
     if (lua_pcall(my_lua_state, 0, 1, 0))
     {
         // Runtime Lua errors are suppressed in production code since detectors are written for
-        // efficiency and with defensive minimum checks. Errors are dealt as exceptions
+        // efficiency and with minimum defensive checks. Errors are dealt as exceptions
         // that don't impact processing by other detectors or future packets by the same detector.
         APPID_LOG(args.pkt, TRACE_ERROR_LEVEL, "lua detector %s: error validating %s\n",
             package_info.name.c_str(), lua_tostring(my_lua_state, -1));

diff --git a/src/network_inspectors/appid/lua_detector_module.cc b/src/network_inspectors/appid/lua_detector_module.cc
index 8d891139217aaeb3dc6aacf50100f3fcb59e52c7..377f9058596567e4bdeae98972a9e3d5842e63c1 100644 (file)
--- a/src/network_inspectors/appid/lua_detector_module.cc
+++ b/src/network_inspectors/appid/lua_detector_module.cc
@@ -197,27 +197,43 @@ LuaDetectorManager::~LuaDetectorManager()
     if (L)
     {
         if (lua_gettop(L))
+        {
             APPID_LOG(nullptr, TRACE_WARNING_LEVEL, "appid: leak of %d lua stack elements before detector unload\n",
                 lua_gettop(L));
+            lua_settop(L, 0);
+        }
 
-        for ( auto& lua_object : allocated_objects )
+        for (auto& lua_object : allocated_objects)
         {
             LuaStateDescriptor* lsd = lua_object->validate_lua_state(false);
 
-            lua_getfield(L, LUA_REGISTRYINDEX, lsd->package_info.name.c_str());
-            lua_getfield(L, -1, lsd->package_info.cleanFunctionName.c_str());
-            if ( lua_isfunction(L, -1) )
+            if (!lsd->package_info.cleanFunctionName.empty())
             {
-                string name = lsd->package_info.name + "_";
-                lua_getglobal(L, name.c_str());
-
-                if ( lua_pcall(L, 1, 1, 0) )
+                lua_getfield(L, LUA_REGISTRYINDEX, lsd->package_info.name.c_str());
+                if (lua_istable(L, -1))
                 {
-                    APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "Could not cleanup the %s client app element: %s\n",
-                        lsd->package_info.name.c_str(), lua_tostring(L, -1));
+                    lua_getfield(L, -1, lsd->package_info.cleanFunctionName.c_str());
+                    if (lua_isfunction(L, -1))
+                    {
+                        string name = lsd->package_info.name + "_";
+                        lua_getglobal(L, name.c_str());
+
+                        if ( lua_pcall(L, 1, 1, 0) )
+                        {
+                            APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "Error - appid: Could not cleanup the %s client app element: %s\n",
+                                lsd->package_info.name.c_str(), lua_tostring(L, -1));
+                        }
+                    }
                 }
+                else
+                {
+                    APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "Error - appid: Could not find the %s detector table for cleanup\n",
+                        lsd->package_info.name.c_str());
+                }
+                
+                lua_settop(L, 0);
             }
-            lua_settop(L, 0);
+
             delete lua_object;
         }
         lua_close(L);
@@ -321,6 +337,14 @@ LuaObject* LuaDetectorManager::create_lua_detector(const char* detector_name,
 
     Lua::ManageStack mgr(L);
     lua_getfield(L, LUA_REGISTRYINDEX, detector_name);
+    if (!lua_istable(L, -1))
+    {
+        if (init(L))
+            APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "Error - appid: Could not find detector table %s for initialization\n",
+                detector_name);
+        lua_pop(L, 1);
+        return nullptr;
+    }
 
     lua_getfield(L, -1, "DetectorPackageInfo");
     if (!lua_istable(L, -1))
@@ -466,8 +490,11 @@ bool LuaDetectorManager::load_detector(char* detector_filename, bool is_custom,
 void LuaDetectorManager::activate_lua_detectors(const SnortConfig* sc)
 {
     if (lua_gettop(L))
+    {
         APPID_LOG(nullptr, TRACE_WARNING_LEVEL, "appid: leak of %d lua stack elements before detector activate\n",
             lua_gettop(L));
+        lua_settop(L, 0);
+    }
 
     uint32_t lua_tracker_size = compute_lua_tracker_size(MAX_MEMORY_FOR_LUA_DETECTORS, allocated_objects.size());
     list<LuaObject*>::iterator lo = allocated_objects.begin();
@@ -475,6 +502,18 @@ void LuaDetectorManager::activate_lua_detectors(const SnortConfig* sc)
     {
         LuaStateDescriptor* lsd = (*lo)->validate_lua_state(false);
         lua_getfield(L, LUA_REGISTRYINDEX, lsd->package_info.name.c_str());
+        if (!lua_istable(L, -1))
+        {
+            if (init(L))
+                APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "Error - appid: Could not find detector table %s for initialization\n",
+                    (*lo)->get_detector()->get_name().c_str());
+            if (!(*lo)->get_detector()->is_custom_detector())
+                num_odp_detectors--;
+            lua_settop(L, 0);
+            delete *lo;
+            lo = allocated_objects.erase(lo);
+            continue;
+        }
         lua_getfield(L, -1, lsd->package_info.initFunctionName.c_str());
         if (!lua_isfunction(L, -1))
         {
@@ -605,8 +644,11 @@ void ControlLuaDetectorManager::load_lua_detectors(const char* path, bool is_cus
     if (rval == 0 )
     {
         if (lua_gettop(L))
+        {
             APPID_LOG(nullptr, TRACE_WARNING_LEVEL, "appid: leak of %d lua stack elements before detector load\n",
                 lua_gettop(L));
+            lua_settop(L, 0);
+        }
 
         for (unsigned n = 0; n < globs.gl_pathc; n++)
         {