From 4288f5a009b3c64c9baf23637efd2837ff3bd8dc Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sat, 27 Apr 2024 23:25:53 -0300 Subject: [PATCH 01/12] Enable inline suppressions in cppcheck --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 3a6b23e55..501fcfdf4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -59,6 +59,7 @@ cppcheck: @cppcheck -U YYSTYPE -U MBEDTLS_MD5_ALT -U MBEDTLS_SHA1_ALT \ -D MS_CPPCHECK_DISABLED_FOR_PARSER -U YY_USER_INIT \ --suppressions-list=./test/cppcheck_suppressions.txt \ + --inline-suppr \ --enable=warning,style,performance,portability,unusedFunction,missingInclude \ --inconclusive \ --template="warning: {file},{line},{severity},{id},{message}" \ From b872f11f68c09d9c252fba043901e3b5c087c0dd Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 11:42:05 -0300 Subject: [PATCH 02/12] Fixed memory leak in examples/reading_logs_via_rule_message --- .../reading_logs_via_rule_message.h | 14 ++++---------- test/cppcheck_suppressions.txt | 1 - 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h index 29e4d6622..3fb6ef1eb 100644 --- a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h +++ b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h @@ -130,29 +130,25 @@ class ReadingLogsViaRuleMessage { struct data_ms dms; void *status; - modsecurity::ModSecurity *modsec; - modsecurity::RulesSet *rules; - - modsec = new modsecurity::ModSecurity(); + auto modsec = std::make_unique(); modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha" \ " (ModSecurity test)"); modsec->setServerLogCb(logCb, modsecurity::RuleMessageLogProperty | modsecurity::IncludeFullHighlightLogProperty); - rules = new modsecurity::RulesSet(); + auto rules = std::make_unique(); if (rules->loadFromUri(m_rules.c_str()) < 0) { std::cout << "Problems loading the rules..." << std::endl; std::cout << rules->m_parserError.str() << std::endl; return -1; } - dms.modsec = modsec; - dms.rules = rules; + dms.modsec = modsec.get(); + dms.rules = rules.get(); for (i = 0; i < NUM_THREADS; i++) { pthread_create(&threads[i], NULL, process_request, reinterpret_cast(&dms)); - // process_request((void *)&dms); } usleep(10000); @@ -162,8 +158,6 @@ class ReadingLogsViaRuleMessage { std::cout << "Main: completed thread id :" << i << std::endl; } - delete rules; - delete modsec; return 0; } diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index ebbc665ee..77479be26 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -101,5 +101,4 @@ stlcstrStream uselessCallsSubstr // Examples -memleak:examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h:147 memleak:examples/using_bodies_in_chunks/simple_request.cc From fde9d279b02f928a09bea5bb3840bedadf905d15 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sat, 27 Apr 2024 18:00:28 -0300 Subject: [PATCH 03/12] Removed unnecessary cppcheck suppression and r-value reference as copy should be avoidded by RVO --- src/modsecurity.cc | 2 +- test/cppcheck_suppressions.txt | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/modsecurity.cc b/src/modsecurity.cc index 854ec31ea..4b48b7995 100644 --- a/src/modsecurity.cc +++ b/src/modsecurity.cc @@ -202,7 +202,7 @@ void ModSecurity::serverLog(void *data, std::shared_ptr rm) { } if (m_logProperties & TextLogProperty) { - std::string &&d = rm->log(); + auto d = rm->log(); const void *a = static_cast(d.c_str()); m_logCb(data, a); return; diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index 77479be26..6668c6f26 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -44,12 +44,10 @@ functionStatic:src/engine/lua.h:71 functionConst:src/utils/geo_lookup.h:49 useInitializationList:src/operators/rbl.h:69 constStatement:test/common/modsecurity_test.cc:82 -danglingTemporaryLifetime:src/modsecurity.cc:206 functionStatic:src/operators/geo_lookup.h:35 duplicateBreak:src/operators/validate_utf8_encoding.cc syntaxError:src/transaction.cc:62 noConstructor:src/variables/variable.h:152 -danglingTempReference:src/modsecurity.cc:206 knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77 knownConditionTrueFalse:src/operators/verify_svnr.cc:87 rethrowNoCurrentException:headers/modsecurity/transaction.h:313 From 94b68b2514053836e431c15128b4e7f3ade2d235 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 12:01:20 -0300 Subject: [PATCH 04/12] Minor updates to simplify code and remove cppcheck suppressions --- test/common/modsecurity_test.cc | 13 ++++++------- test/common/modsecurity_test.h | 4 ++-- test/cppcheck_suppressions.txt | 3 --- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/test/common/modsecurity_test.cc b/test/common/modsecurity_test.cc index 5b63580f6..227571919 100644 --- a/test/common/modsecurity_test.cc +++ b/test/common/modsecurity_test.cc @@ -46,7 +46,7 @@ std::string ModSecurityTest::header() { } template -bool ModSecurityTest::load_test_json(std::string file) { +bool ModSecurityTest::load_test_json(const std::string &file) { char errbuf[1024]; yajl_val node; @@ -76,13 +76,12 @@ bool ModSecurityTest::load_test_json(std::string file) { u->filename = file; if (this->count(u->filename + ":" + u->name) == 0) { - std::vector *vector = new std::vector; - vector->push_back(u); + auto vec = new std::vector; + vec->push_back(u); std::string filename(u->filename + ":" + u->name); - std::pair*> a(filename, vector); - this->insert(a); + this->insert({filename, vec}); } else { - std::vector *vec = this->at(u->filename + ":" + u->name); + auto vec = this->at(u->filename + ":" + u->name); vec->push_back(u); } } @@ -95,7 +94,7 @@ bool ModSecurityTest::load_test_json(std::string file) { template std::pair>* -ModSecurityTest::load_tests(std::string path) { +ModSecurityTest::load_tests(const std::string &path) { DIR *dir; struct dirent *ent; struct stat buffer; diff --git a/test/common/modsecurity_test.h b/test/common/modsecurity_test.h index 83e06ab8f..79a168f71 100644 --- a/test/common/modsecurity_test.h +++ b/test/common/modsecurity_test.h @@ -39,8 +39,8 @@ template class ModSecurityTest : std::string header(); void cmd_options(int, char **); std::pair>* load_tests(); - std::pair>* load_tests(std::string path); - bool load_test_json(std::string); + std::pair>* load_tests(const std::string &path); + bool load_test_json(const std::string &file); std::string target; bool verbose = false; diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index 6668c6f26..15234c701 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -43,7 +43,6 @@ functionStatic:src/engine/lua.h:70 functionStatic:src/engine/lua.h:71 functionConst:src/utils/geo_lookup.h:49 useInitializationList:src/operators/rbl.h:69 -constStatement:test/common/modsecurity_test.cc:82 functionStatic:src/operators/geo_lookup.h:35 duplicateBreak:src/operators/validate_utf8_encoding.cc syntaxError:src/transaction.cc:62 @@ -55,8 +54,6 @@ rethrowNoCurrentException:src/rule_with_actions.cc:127 ctunullpointer:src/rule_with_actions.cc:244 ctunullpointer:src/rule_with_operator.cc:135 ctunullpointer:src/rule_with_operator.cc:95 -passedByValue:test/common/modsecurity_test.cc:49 -passedByValue:test/common/modsecurity_test.cc:98 unreadVariable:src/rule_with_operator.cc:219 uninitvar:src/operators/verify_cpf.cc:77 From 0cd2f459f319eeec1f53545146d4a1a2ad2dc8d2 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 12:43:02 -0300 Subject: [PATCH 05/12] Address cppcheck suppressions in lmdb --- src/collection/backend/lmdb.cc | 2 +- src/collection/backend/lmdb.h | 2 +- test/cppcheck_suppressions.txt | 3 --- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/collection/backend/lmdb.cc b/src/collection/backend/lmdb.cc index 9197fda83..040ebdff1 100644 --- a/src/collection/backend/lmdb.cc +++ b/src/collection/backend/lmdb.cc @@ -659,7 +659,7 @@ MDB_dbi* MDBEnvProvider::GetDBI() { return &m_dbi; } -bool MDBEnvProvider::isValid() { +bool MDBEnvProvider::isValid() const { return valid; } diff --git a/src/collection/backend/lmdb.h b/src/collection/backend/lmdb.h index fb88b0038..15c4fa9f7 100644 --- a/src/collection/backend/lmdb.h +++ b/src/collection/backend/lmdb.h @@ -83,7 +83,7 @@ class MDBEnvProvider { } MDB_env* GetEnv(); MDB_dbi* GetDBI(); - bool isValid(); + bool isValid() const; ~MDBEnvProvider(); private: diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index 15234c701..bd467b6f6 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -59,9 +59,6 @@ unreadVariable:src/rule_with_operator.cc:219 uninitvar:src/operators/verify_cpf.cc:77 uninitvar:src/operators/verify_svnr.cc:67 -functionConst:src/collection/backend/lmdb.h:86 -unusedLabel:src/collection/backend/lmdb.cc:297 - variableScope:src/operators/rx.cc variableScope:src/operators/rx_global.cc From cd2dded659ae965d798653688dd6f52943870918 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 12:50:58 -0300 Subject: [PATCH 06/12] Removed unnecessary break after return --- src/operators/validate_utf8_encoding.cc | 5 ----- test/cppcheck_suppressions.txt | 1 - 2 files changed, 6 deletions(-) diff --git a/src/operators/validate_utf8_encoding.cc b/src/operators/validate_utf8_encoding.cc index e95061af3..3e17686fc 100644 --- a/src/operators/validate_utf8_encoding.cc +++ b/src/operators/validate_utf8_encoding.cc @@ -132,7 +132,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r std::to_string(i) + "\"]"); } return true; - break; case UNICODE_ERROR_INVALID_ENCODING : if (transaction) { ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: " @@ -142,7 +141,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r logOffset(ruleMessage, i, str.size()); } return true; - break; case UNICODE_ERROR_OVERLONG_CHARACTER : if (transaction) { ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: " @@ -152,7 +150,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r logOffset(ruleMessage, i, str.size()); } return true; - break; case UNICODE_ERROR_RESTRICTED_CHARACTER : if (transaction) { ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: " @@ -162,7 +159,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r logOffset(ruleMessage, i, str.size()); } return true; - break; case UNICODE_ERROR_DECODING_ERROR : if (transaction) { ms_dbg_a(transaction, 8, "Error validating UTF-8 decoding " @@ -171,7 +167,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r logOffset(ruleMessage, i, str.size()); } return true; - break; } if (rc <= 0) { diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index bd467b6f6..bd5e6bf85 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -44,7 +44,6 @@ functionStatic:src/engine/lua.h:71 functionConst:src/utils/geo_lookup.h:49 useInitializationList:src/operators/rbl.h:69 functionStatic:src/operators/geo_lookup.h:35 -duplicateBreak:src/operators/validate_utf8_encoding.cc syntaxError:src/transaction.cc:62 noConstructor:src/variables/variable.h:152 knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77 From 0c38023b2114bc804908f74bf2fe38c568183b6a Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 13:08:39 -0300 Subject: [PATCH 07/12] Removed unmatchedSuppression entries --- test/cppcheck_suppressions.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index bd5e6bf85..c3a4cf498 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -32,9 +32,7 @@ invalidScanfArgType_int:src/rules_set_properties.cc:102 // // ModSecurity v3 code... // -unmatchedSuppression:src/utils/geo_lookup.cc:82 useInitializationList:src/utils/shared_files.h:87 -unmatchedSuppression:src/utils/msc_tree.cc functionStatic:headers/modsecurity/transaction.h:408 duplicateBranch:src/audit_log/audit_log.cc:226 unreadVariable:src/request_body_processor/multipart.cc:435 From 4aad8e0d061e0f6303c765d1b6605abba6985e8b Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 11:34:37 -0300 Subject: [PATCH 08/12] Inline cppcheck suppressions --- headers/modsecurity/transaction.h | 2 +- src/engine/lua.h | 4 ++-- src/operators/geo_lookup.h | 1 + src/operators/rbl.h | 9 +++++---- src/operators/validate_url_encoding.cc | 2 +- src/operators/verify_cpf.cc | 2 +- src/operators/verify_svnr.cc | 4 ++-- src/rule_with_actions.cc | 2 +- src/rule_with_operator.cc | 3 ++- src/rules_set_properties.cc | 4 ++-- test/cppcheck_suppressions.txt | 17 ----------------- 11 files changed, 18 insertions(+), 32 deletions(-) diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index b0dc4b714..83d850122 100644 --- a/headers/modsecurity/transaction.h +++ b/headers/modsecurity/transaction.h @@ -405,7 +405,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa size_t getRequestBodyLength(); #ifndef NO_LOGS - void debug(int, const std::string&) const; + void debug(int, const std::string &) const; // cppcheck-suppress functionStatic #endif void serverLog(std::shared_ptr rm); diff --git a/src/engine/lua.h b/src/engine/lua.h index 9a678fd74..a33f28f81 100644 --- a/src/engine/lua.h +++ b/src/engine/lua.h @@ -67,8 +67,8 @@ class Lua { public: Lua() { } - bool load(const std::string &script, std::string *err); - int run(Transaction *t, const std::string &str=""); + bool load(const std::string &script, std::string *err); // cppcheck-suppress functionStatic ; triggered when compiling without LUA + int run(Transaction *t, const std::string &str = ""); // cppcheck-suppress functionStatic ; triggered when compiling without LUA static bool isCompatible(const std::string &script, Lua *l, std::string *error); #ifdef WITH_LUA diff --git a/src/operators/geo_lookup.h b/src/operators/geo_lookup.h index e01937825..187a2f317 100644 --- a/src/operators/geo_lookup.h +++ b/src/operators/geo_lookup.h @@ -32,6 +32,7 @@ class GeoLookup : public Operator { bool evaluate(Transaction *transaction, const std::string &exp) override; protected: + // cppcheck-suppress functionStatic bool debug(Transaction *transaction, int x, const std::string &a) { ms_dbg_a(transaction, x, a); return true; diff --git a/src/operators/rbl.h b/src/operators/rbl.h index a35a6a7b5..fb57d2d96 100644 --- a/src/operators/rbl.h +++ b/src/operators/rbl.h @@ -66,10 +66,11 @@ class Rbl : public Operator { m_demandsPassword(false), m_provider(RblProvider::UnknownProvider), Operator("Rbl", std::move(param)) { - m_service = m_string->evaluate(); - if (m_service.find("httpbl.org") != std::string::npos) { - m_demandsPassword = true; - m_provider = RblProvider::httpbl; + m_service = m_string->evaluate(); // cppcheck-suppress useInitializationList + if (m_service.find("httpbl.org") != std::string::npos) + { + m_demandsPassword = true; + m_provider = RblProvider::httpbl; } else if (m_service.find("uribl.com") != std::string::npos) { m_provider = RblProvider::uribl; } else if (m_service.find("spamhaus.org") != std::string::npos) { diff --git a/src/operators/validate_url_encoding.cc b/src/operators/validate_url_encoding.cc index 20a86eb62..502aa3d49 100644 --- a/src/operators/validate_url_encoding.cc +++ b/src/operators/validate_url_encoding.cc @@ -74,7 +74,7 @@ bool ValidateUrlEncoding::evaluate(Transaction *transaction, RuleWithActions *ru bool res = false; if (input.empty() == true) { - return res; + return res; // cppcheck-suppress knownConditionTrueFalse } int rc = validate_url_encoding(input.c_str(), input.size(), &offset); diff --git a/src/operators/verify_cpf.cc b/src/operators/verify_cpf.cc index 778584db6..1dcf18444 100644 --- a/src/operators/verify_cpf.cc +++ b/src/operators/verify_cpf.cc @@ -74,7 +74,7 @@ bool VerifyCPF::verify(const char *cpfnumber, int len) { c = cpf_len; for (i = 0; i < 9; i++) { - sum += (cpf[i] * --c); + sum += (cpf[i] * --c); // cppcheck-suppress uninitvar } factor = (sum % cpf_len); diff --git a/src/operators/verify_svnr.cc b/src/operators/verify_svnr.cc index 248e6b4ec..87834dd4d 100644 --- a/src/operators/verify_svnr.cc +++ b/src/operators/verify_svnr.cc @@ -64,7 +64,7 @@ bool VerifySVNR::verify(const char *svnrnumber, int len) { } //Laufnummer mit 3, 7, 9 //Geburtsdatum mit 5, 8, 4, 2, 1, 6 - sum = svnr[0] * 3 + svnr[1] * 7 + svnr[2] * 9 + svnr[4] * 5 + svnr[5] * 8 + svnr[6] * 4 + svnr[7] * 2 + svnr[8] * 1 + svnr[9] * 6; + sum = svnr[0] * 3 + svnr[1] * 7 + svnr[2] * 9 + svnr[4] * 5 + svnr[5] * 8 + svnr[6] * 4 + svnr[7] * 2 + svnr[8] * 1 + svnr[9] * 6; // cppcheck-suppress uninitvar sum %= 11; if(sum == 10){ sum = 0; @@ -84,7 +84,7 @@ bool VerifySVNR::evaluate(Transaction *t, RuleWithActions *rule, int i; if (m_param.empty()) { - return is_svnr; + return is_svnr; // cppcheck-suppress knownConditionTrueFalse } for (i = 0; i < input.size() - 1 && is_svnr == false; i++) { diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc index 04ee219c4..7c9471c19 100644 --- a/src/rule_with_actions.cc +++ b/src/rule_with_actions.cc @@ -241,7 +241,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans, bool containsBlock, std::shared_ptr ruleMessage) { bool disruptiveAlreadyExecuted = false; - for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) { + for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) { // cppcheck-suppress ctunullpointer if (a.get()->action_kind != actions::Action::RunTimeOnlyIfMatchKind) { continue; } diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index 21d936282..d308443a4 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -92,6 +92,7 @@ void RuleWithOperator::updateMatchedVars(Transaction *trans, const std::string & void RuleWithOperator::cleanMatchedVars(Transaction *trans) { ms_dbg_a(trans, 9, "Matched vars cleaned."); + // cppcheck-suppress ctunullpointer trans->m_variableMatchedVar.unset(); trans->m_variableMatchedVars.unset(); trans->m_variableMatchedVarName.unset(); @@ -132,7 +133,7 @@ bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string & void RuleWithOperator::getVariablesExceptions(Transaction *t, variables::Variables *exclusion, variables::Variables *addition) { - for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { + for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer if (containsTag(*a.first.get(), t) == false) { continue; } diff --git a/src/rules_set_properties.cc b/src/rules_set_properties.cc index ffb37e768..075e3e877 100644 --- a/src/rules_set_properties.cc +++ b/src/rules_set_properties.cc @@ -98,8 +98,8 @@ void ConfigUnicodeMap::loadConfig(std::string f, double configCodePage, if (mapping != NULL) { ucode = strtok_r(mapping, ":", &hmap); - sscanf(ucode, "%x", &code); - sscanf(hmap, "%x", &Map); + sscanf(ucode, "%x", &code); // cppcheck-suppress invalidScanfArgType_int + sscanf(hmap, "%x", &Map); // cppcheck-suppress invalidScanfArgType_int if (code >= 0 && code <= 65535) { driver->m_unicodeMapTable.m_unicodeMapTable->change(code, Map); } diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index c3a4cf498..be29218f6 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -25,36 +25,19 @@ shiftNegative:src/utils/msc_tree.cc *:src/utils/acmp.cc *:src/utils/msc_tree.cc -invalidScanfArgType_int:src/rules_set_properties.cc:101 -invalidScanfArgType_int:src/rules_set_properties.cc:102 // // ModSecurity v3 code... // useInitializationList:src/utils/shared_files.h:87 -functionStatic:headers/modsecurity/transaction.h:408 duplicateBranch:src/audit_log/audit_log.cc:226 unreadVariable:src/request_body_processor/multipart.cc:435 stlcstrParam:src/audit_log/writer/parallel.cc:145 -functionStatic:src/engine/lua.h:70 -functionStatic:src/engine/lua.h:71 -functionConst:src/utils/geo_lookup.h:49 -useInitializationList:src/operators/rbl.h:69 -functionStatic:src/operators/geo_lookup.h:35 syntaxError:src/transaction.cc:62 noConstructor:src/variables/variable.h:152 -knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77 -knownConditionTrueFalse:src/operators/verify_svnr.cc:87 rethrowNoCurrentException:headers/modsecurity/transaction.h:313 rethrowNoCurrentException:src/rule_with_actions.cc:127 -ctunullpointer:src/rule_with_actions.cc:244 -ctunullpointer:src/rule_with_operator.cc:135 -ctunullpointer:src/rule_with_operator.cc:95 -unreadVariable:src/rule_with_operator.cc:219 - -uninitvar:src/operators/verify_cpf.cc:77 -uninitvar:src/operators/verify_svnr.cc:67 variableScope:src/operators/rx.cc variableScope:src/operators/rx_global.cc From 7a9c0ab15fcb315b8976f3f1c4476e79c11b2e51 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 14:36:18 -0300 Subject: [PATCH 09/12] Removed unused suppresion and avoid copy of logPath --- src/audit_log/writer/parallel.cc | 2 +- test/cppcheck_suppressions.txt | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/audit_log/writer/parallel.cc b/src/audit_log/writer/parallel.cc index 0a9777bfa..e42d7871c 100644 --- a/src/audit_log/writer/parallel.cc +++ b/src/audit_log/writer/parallel.cc @@ -118,7 +118,7 @@ bool Parallel::write(Transaction *transaction, int parts, std::string *error) { log = transaction->toOldAuditLogFormat(parts, "-" + boundary + "--"); } - std::string logPath = m_audit->m_storage_dir; + const auto &logPath = m_audit->m_storage_dir; fileName = logPath + fileName + "-" + *transaction->m_id.get(); if (logPath.empty()) { diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index be29218f6..a1004873f 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -33,7 +33,6 @@ shiftNegative:src/utils/msc_tree.cc useInitializationList:src/utils/shared_files.h:87 duplicateBranch:src/audit_log/audit_log.cc:226 unreadVariable:src/request_body_processor/multipart.cc:435 -stlcstrParam:src/audit_log/writer/parallel.cc:145 syntaxError:src/transaction.cc:62 noConstructor:src/variables/variable.h:152 rethrowNoCurrentException:headers/modsecurity/transaction.h:313 From 95ce3a7db471bc39d4299c0fcec23a8edecea480 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 14:37:56 -0300 Subject: [PATCH 10/12] Removed unused suppressions --- test/cppcheck_suppressions.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index a1004873f..7f7c4526e 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -31,10 +31,6 @@ shiftNegative:src/utils/msc_tree.cc // ModSecurity v3 code... // useInitializationList:src/utils/shared_files.h:87 -duplicateBranch:src/audit_log/audit_log.cc:226 -unreadVariable:src/request_body_processor/multipart.cc:435 -syntaxError:src/transaction.cc:62 -noConstructor:src/variables/variable.h:152 rethrowNoCurrentException:headers/modsecurity/transaction.h:313 rethrowNoCurrentException:src/rule_with_actions.cc:127 From 9f5dc200ba28e7b729ddfc3621e6cb750eef3c32 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Mon, 29 Apr 2024 22:28:42 -0300 Subject: [PATCH 11/12] Replace final three suppressions entries with line numbers - These were initially not included in these changes, as they were other PRs (#3104 & #3132) that address them. --- headers/modsecurity/transaction.h | 2 +- src/rule_with_actions.cc | 2 +- src/utils/shared_files.h | 2 +- test/cppcheck_suppressions.txt | 4 ---- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index 83d850122..811a2c831 100644 --- a/headers/modsecurity/transaction.h +++ b/headers/modsecurity/transaction.h @@ -310,7 +310,7 @@ class TransactionSecMarkerManagement { if (m_marker) { return m_marker; } else { - throw; + throw; // cppcheck-suppress rethrowNoCurrentException } } diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc index 7c9471c19..df323c4bb 100644 --- a/src/rule_with_actions.cc +++ b/src/rule_with_actions.cc @@ -124,7 +124,7 @@ RuleWithActions::RuleWithActions( delete a; std::cout << "General failure, action: " << a->m_name; std::cout << " has an unknown type." << std::endl; - throw; + throw; // cppcheck-suppress rethrowNoCurrentException } } delete actions; diff --git a/src/utils/shared_files.h b/src/utils/shared_files.h index bec28f652..d0d8ef992 100644 --- a/src/utils/shared_files.h +++ b/src/utils/shared_files.h @@ -84,7 +84,7 @@ class SharedFiles { bool toBeCreated(false); bool err = false; - m_memKeyStructure = ftok(".", 1); + m_memKeyStructure = ftok(".", 1); // cppcheck-suppress useInitializationList if (m_memKeyStructure < 0) { err = true; goto err_mem_key; diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index 7f7c4526e..bff688d8f 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -30,10 +30,6 @@ shiftNegative:src/utils/msc_tree.cc // // ModSecurity v3 code... // -useInitializationList:src/utils/shared_files.h:87 -rethrowNoCurrentException:headers/modsecurity/transaction.h:313 -rethrowNoCurrentException:src/rule_with_actions.cc:127 - variableScope:src/operators/rx.cc variableScope:src/operators/rx_global.cc From 1f419bba8f6700fa67f6d0ce0c123e8aa4331b2e Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 18:12:49 -0300 Subject: [PATCH 12/12] Implement sonarcloud suggestions --- .../reading_logs_via_rule_message.h | 2 +- src/rule_with_operator.cc | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h index 3fb6ef1eb..9d80d9b1d 100644 --- a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h +++ b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h @@ -124,7 +124,7 @@ class ReadingLogsViaRuleMessage { m_rules(rules) { } - int process() { + int process() const { pthread_t threads[NUM_THREADS]; int i; struct data_ms dms; diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index d308443a4..5146c6d43 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -133,7 +133,7 @@ bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string & void RuleWithOperator::getVariablesExceptions(Transaction *t, variables::Variables *exclusion, variables::Variables *addition) { - for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer + for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer if (containsTag(*a.first.get(), t) == false) { continue; } @@ -147,7 +147,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t, } } - for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) { + for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) { if (containsMsg(*a.first.get(), t) == false) { continue; } @@ -161,7 +161,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t, } } - for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) { + for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) { if (m_ruleId != a.first) { continue; }