From: Oleksandr Stepanov -X (ostepano - SOFTSERVE INC at Cisco) Date: Thu, 18 Dec 2025 13:13:01 +0000 (+0000) Subject: Pull request #5046: appid: check for Lua table errors during initialization and cleanup X-Git-Tag: 3.10.1.0~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7ba02a1b41bc953a782d6301403489a5457cb750;p=thirdparty%2Fsnort3.git 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 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 af93127f0..5a39e748f 100644 --- 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 8d8911392..377f90585 100644 --- 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::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++) {