From ec7d2db400bb91d4efabb1de3e2f3bb6ec4674a8 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Mon, 16 Nov 2020 11:39:26 -0300 Subject: [PATCH] Avoids to cleanup GeoIp on ModSecurity destructor GeoIp is already being cleaned elsewhere. Fix #2041 --- CHANGES | 2 ++ src/modsecurity.cc | 3 --- src/utils/geo_lookup.cc | 16 ++++++++++------ src/utils/geo_lookup.h | 7 +++++-- test/cppcheck_suppressions.txt | 4 ++-- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/CHANGES b/CHANGES index 9f834faf..fd5f79b7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v3.x.y - YYYY-MMM-DD (to be released) ------------------------------------- + - Avoids to cleanup GeoIp on ModSecurity destructor + [#2041 - @zimmerle, @jptosso, @victorhora] - Fix memory leak of RuleMessages objects [#2376 - @WGH-, @martinhsv] - Produce not-supported error for ctl:forceRequestBodyVariable diff --git a/src/modsecurity.cc b/src/modsecurity.cc index 55728a86..75c6d6d1 100644 --- a/src/modsecurity.cc +++ b/src/modsecurity.cc @@ -95,9 +95,6 @@ ModSecurity::~ModSecurity() { #ifdef MSC_WITH_CURL curl_global_cleanup(); #endif -#ifdef WITH_GEOIP - Utils::GeoLookup::getInstance().cleanUp(); -#endif #ifdef WITH_LIBXML2 xmlCleanupParser(); #endif diff --git a/src/utils/geo_lookup.cc b/src/utils/geo_lookup.cc index e8b37c5c..411da337 100644 --- a/src/utils/geo_lookup.cc +++ b/src/utils/geo_lookup.cc @@ -63,22 +63,26 @@ bool GeoLookup::setDataBase(const std::string& filePath, std::string intGeo; #endif + if (m_version != NOT_LOADED) { + cleanUp(); + } + #ifdef WITH_MAXMIND int status = MMDB_open(filePath.c_str(), MMDB_MODE_MMAP, &mmdb); - if (status != MMDB_SUCCESS) { - intMax.assign("libMaxMind: Can't open: " + std::string(MMDB_strerror(status)) + "."); - } else { + if (status == MMDB_SUCCESS) { m_version = VERSION_MAXMIND; + } else { + intMax.assign("libMaxMind: Can't open: " + std::string(MMDB_strerror(status)) + "."); } #endif #ifdef WITH_GEOIP if (m_version == NOT_LOADED) { m_gi = GeoIP_open(filePath.c_str(), GEOIP_MEMORY_CACHE); - if (m_gi == NULL) { - intGeo.append("GeoIP: Can't open: " + filePath + "."); - } else { + if (m_gi) { m_version = VERSION_GEOIP; + } else { + intGeo.append("GeoIP: Can't open: " + filePath + "."); } } #endif diff --git a/src/utils/geo_lookup.h b/src/utils/geo_lookup.h index d8fd3541..35fa9822 100644 --- a/src/utils/geo_lookup.h +++ b/src/utils/geo_lookup.h @@ -55,13 +55,16 @@ class GeoLookup { private: GeoLookup() : m_version(NOT_LOADED) +#if WITH_MAXMIND + ,mmdb() +#endif #if WITH_GEOIP ,m_gi(NULL) #endif { } ~GeoLookup(); - GeoLookup(GeoLookup const&); - void operator=(GeoLookup const&); + GeoLookup(GeoLookup const&) = delete; + void operator=(GeoLookup const&) = delete; GeoLookupVersion m_version; #if WITH_MAXMIND diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index b03c4896..98e6bbd2 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -51,14 +51,14 @@ functionStatic:src/engine/lua.h:80 functionConst:src/utils/geo_lookup.h:49 useInitializationList:src/operators/rbl.h:69 constStatement:test/common/modsecurity_test.cc:82 -danglingTemporaryLifetime:src/modsecurity.cc:204 +danglingTemporaryLifetime:src/modsecurity.cc:201 functionStatic:src/operators/geo_lookup.h:35 duplicateBreak:src/operators/validate_utf8_encoding.cc duplicateBranch:src/request_body_processor/multipart.cc:91 syntaxError:src/transaction.cc:62 noConstructor:src/variables/variable.h:152 duplicateBranch:src/request_body_processor/multipart.cc:93 -danglingTempReference:src/modsecurity.cc:204 +danglingTempReference:src/modsecurity.cc:201 knownConditionTrueFalse:src/operators/validate_url_encoding.cc:79 knownConditionTrueFalse:src/operators/verify_svnr.cc:90 noConstructor:src/actions/rule_id.h:30