From: Bohdan Hryniv -X (bhryniv - SOFTSERVE INC at Cisco) Date: Thu, 18 Sep 2025 19:50:26 +0000 (+0000) Subject: Pull request #4904: appid: fixed crash in stats manager X-Git-Tag: 3.9.6.0~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aaede8305be98dd6daec496cc97c11b128e6e3fe;p=thirdparty%2Fsnort3.git Pull request #4904: appid: fixed crash in stats manager Merge in SNORT/snort3 from ~BHRYNIV/snort3:fix_uaf_reload to master Squashed commit of the following: commit 25e2f0fa875bb0b472cf43db411e6c695f7ea2ac Author: Bohdan Hryniv Date: Thu Sep 11 08:31:20 2025 -0400 appid: fixed crash in stats manager --- diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc index 2058c9cd1..d441dec1d 100644 --- a/src/network_inspectors/appid/appid_session.cc +++ b/src/network_inspectors/appid/appid_session.cc @@ -164,7 +164,14 @@ AppIdSession::~AppIdSession() if (!in_expected_cache) { if (config.log_stats) - AppIdStatistics::get_stats_manager()->update(*this); + { + AppIdStatistics::stats_manager_lock.lock(); + if (AppIdStatistics::get_stats_manager() != nullptr) + { + AppIdStatistics::get_stats_manager()->update(*this); + } + AppIdStatistics::stats_manager_lock.unlock(); + } // fail any service detection that is in process for this flow if (!get_session_flags(APPID_SESSION_SERVICE_DETECTED | @@ -604,9 +611,9 @@ void AppIdSession::examine_ssl_metadata(AppidChangeBits& change_bits, bool parti tsession->set_tls_host_published(false); tsession->set_matched_tls_type(MatchedTlsType::MATCHED_TLS_CNAME); } - scan_flags &= ~SCAN_SSL_CERTIFICATE_FLAG; + scan_flags &= ~SCAN_SSL_CERTIFICATE_FLAG; } - + if (!match_found and (scan_flags & SCAN_SSL_ORG_UNIT_FLAG) and (tls_str = tsession->get_tls_org_unit()) ) { size_t size = strlen(tls_str); @@ -647,7 +654,7 @@ void AppIdSession::examine_ssl_metadata(AppidChangeBits& change_bits, bool parti change_bits.set(APPID_TLSHOST_BIT); } } - else if (tsession->is_tls_host_mismatched() and + else if (tsession->is_tls_host_mismatched() and (api.get_tls_host() and (strcmp(api.get_tls_host(), tsession->get_tls_host()) != 0))) { api.set_tls_host(tsession->get_tls_host()); @@ -1303,28 +1310,28 @@ void AppIdSession::publish_shadow_traffic_event(const uint32_t& shadow_traffic_b if (app_name == nullptr) { - if ((shadow_traffic_bits & ShadowTraffic_Type_Domain_Fronting) && + if ((shadow_traffic_bits & ShadowTraffic_Type_Domain_Fronting) && !(shadow_traffic_bits & ~ShadowTraffic_Type_Domain_Fronting)) - { - app_name = "unknown"; + { + app_name = "unknown"; } - else + else { APPID_LOG(curr_packet, TRACE_DEBUG_LEVEL, "Appname is invalid, not publishing shadow traffic event without appname\n"); - return; + return; } } shadow_traffic_pub_id = DataBus::get_id(shadowtraffic_pub_key); - + ShadowTrafficEvent shadow_event(shadow_traffic_bits, "", "", app_name); - DataBus::publish(shadow_traffic_pub_id, ShadowTrafficEventIds::SHADOWTRAFFIC_FLOW_DETECTED, shadow_event, flow); + DataBus::publish(shadow_traffic_pub_id, ShadowTrafficEventIds::SHADOWTRAFFIC_FLOW_DETECTED, shadow_event, flow); if (appidDebug and appidDebug->is_active()) change_shadow_traffic_bits_to_string(shadow_traffic_bits, str_print); - - APPID_LOG(curr_packet, TRACE_DEBUG_LEVEL, - "AppID: ShadowTraffic Published event for: %s, application_name: %s(%d)\n", + + APPID_LOG(curr_packet, TRACE_DEBUG_LEVEL, + "AppID: ShadowTraffic Published event for: %s, application_name: %s(%d)\n", str_print.c_str(), app_name, publishing_appid); set_previous_shadow_traffic_bits(shadow_traffic_bits); @@ -1396,7 +1403,7 @@ void AppIdSession::check_shadow_traffic_bits(AppId id, uint32_t& shadow_bits, Ap { if (id > APP_ID_NONE) { - uint32_t attributeBits = api.asd->get_odp_ctxt().get_app_info_mgr().getAttributeBits(id); + uint32_t attributeBits = api.asd->get_odp_ctxt().get_app_info_mgr().getAttributeBits(id); if (attributeBits & ATTR_APPENCRYPTEDDNS) { shadow_bits |= ShadowTraffic_Type_Encrypted_DNS; @@ -1439,21 +1446,21 @@ void AppIdSession::process_shadow_traffic_appids() if (service_id > 0) check_shadow_traffic_bits(service_id, shadow_bits, publishing_appid, is_publishing_set); - if (payload_id > 0) + if (payload_id > 0) check_shadow_traffic_bits(payload_id, shadow_bits, publishing_appid, is_publishing_set); if (client_id > 0) check_shadow_traffic_bits(client_id, shadow_bits, publishing_appid, is_publishing_set); - if (misc_id > 0) - check_shadow_traffic_bits(misc_id, shadow_bits, publishing_appid, is_publishing_set); + if (misc_id > 0) + check_shadow_traffic_bits(misc_id, shadow_bits, publishing_appid, is_publishing_set); if (shadow_bits != 0) { set_shadow_traffic_bits(shadow_bits); set_shadow_traffic_publishing_appid(publishing_appid); - } + } } -void AppIdSession::check_domain_fronting_status(const std::string& host) +void AppIdSession::check_domain_fronting_status(const std::string& host) { TLSDomainFrontCheckEvent domain_front_event(api.asd->get_cert_key(), host); DataBus::publish(AppIdInspector::get_pub_id(), AppIdEventIds::DOMAIN_FRONTING, domain_front_event); diff --git a/src/network_inspectors/appid/appid_stats.cc b/src/network_inspectors/appid/appid_stats.cc index cbb3bdba4..2aad11bd2 100644 --- a/src/network_inspectors/appid/appid_stats.cc +++ b/src/network_inspectors/appid/appid_stats.cc @@ -33,6 +33,9 @@ #include "app_info_table.h" #include "appid_inspector.h" #include "appid_session.h" +#include + +std::mutex AppIdStatistics::stats_manager_lock; using namespace snort; @@ -159,6 +162,7 @@ AppIdStatistics* AppIdStatistics::initialize_manager(const AppIdConfig& config) if ( !config.log_stats ) return nullptr; + std::lock_guard lock(AppIdStatistics::stats_manager_lock); appid_stats_manager = new AppIdStatistics(config); return appid_stats_manager; } @@ -167,7 +171,11 @@ AppIdStatistics* AppIdStatistics::get_stats_manager() { return appid_stats_manager; } void AppIdStatistics::cleanup() -{ delete appid_stats_manager; } +{ + std::lock_guard lock(AppIdStatistics::stats_manager_lock); + delete appid_stats_manager; + appid_stats_manager = nullptr; +} static void update_stats(const AppIdSession& asd, AppId app_id, StatsBucket* bucket) { diff --git a/src/network_inspectors/appid/appid_stats.h b/src/network_inspectors/appid/appid_stats.h index 5ff16c2fb..9a32f93bd 100644 --- a/src/network_inspectors/appid/appid_stats.h +++ b/src/network_inspectors/appid/appid_stats.h @@ -25,6 +25,7 @@ #include #include #include +#include #include "utils/sflsq.h" #include "utils/util.h" @@ -69,6 +70,8 @@ public: void update(const AppIdSession&); void flush(); + static std::mutex stats_manager_lock; + private: AppIdStatistics(const AppIdConfig&);