Merge pull request #3134 from eduar-hte/inline-cppcheck-suppressions

airween · web-flow · commit c8056483f7db · 2024-05-03T14:43:51.000+02:00
Remove cppcheck suppressions with line numbers in test/cppcheck_suppressions.txt
diff --git 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}" \
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
@@ -124,35 +124,31 @@ class ReadingLogsViaRuleMessage {
             m_rules(rules)
         { }
 
-    int process() {
+    int process() const {
         pthread_t threads[NUM_THREADS];
         int i;
         struct data_ms dms;
         void *status;
 
-        modsecurity::ModSecurity *modsec;
-        modsecurity::RulesSet *rules;
-
-        modsec = new modsecurity::ModSecurity();
+        auto modsec = std::make_unique<modsecurity::ModSecurity>();
         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<modsecurity::RulesSet>();
         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<void *>(&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/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
         }
     }
 
@@ -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<RuleMessage> rm);
 
diff --git 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/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
@@ -83,7 +83,7 @@ class MDBEnvProvider {
     }
     MDB_env* GetEnv();
     MDB_dbi* GetDBI();
-    bool isValid();
+    bool isValid() const;
 
     ~MDBEnvProvider();
  private:
diff --git 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/modsecurity.cc b/src/modsecurity.cc
@@ -202,7 +202,7 @@ void ModSecurity::serverLog(void *data, std::shared_ptr<RuleMessage> rm) {
     }
 
     if (m_logProperties & TextLogProperty) {
-        std::string &&d = rm->log();
+        auto d = rm->log();
         const void *a = static_cast<const void *>(d.c_str());
         m_logCb(data, a);
         return;
diff --git 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
@@ -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
@@ -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/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/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
@@ -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
@@ -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;
@@ -241,7 +241,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans,
     bool containsBlock, std::shared_ptr<RuleMessage> 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
@@ -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 (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;
         }
@@ -146,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;
         }
@@ -160,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;
         }
diff --git 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/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/common/modsecurity_test.cc b/test/common/modsecurity_test.cc
@@ -46,7 +46,7 @@ std::string ModSecurityTest<T>::header() {
 }
 
 template <class T>
-bool ModSecurityTest<T>::load_test_json(std::string file) {
+bool ModSecurityTest<T>::load_test_json(const std::string &file) {
     char errbuf[1024];
     yajl_val node;
 
@@ -76,13 +76,12 @@ bool ModSecurityTest<T>::load_test_json(std::string file) {
         u->filename = file;
 
         if (this->count(u->filename + ":" + u->name) == 0) {
-            std::vector<T *> *vector = new std::vector<T *>;
-            vector->push_back(u);
+            auto vec = new std::vector<T *>;
+            vec->push_back(u);
             std::string filename(u->filename + ":" + u->name);
-            std::pair<std::string, std::vector<T*>*> a(filename, vector);
-            this->insert(a);
+            this->insert({filename, vec});
         } else {
-            std::vector<T *> *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<T>::load_test_json(std::string file) {
 
 template <class T>
 std::pair<std::string, std::vector<T *>>*
-ModSecurityTest<T>::load_tests(std::string path) {
+ModSecurityTest<T>::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
@@ -39,8 +39,8 @@ template <class T> class ModSecurityTest :
     std::string header();
     void cmd_options(int, char **);
     std::pair<std::string, std::vector<T *>>* load_tests();
-    std::pair<std::string, std::vector<T *>>* load_tests(std::string path);
-    bool load_test_json(std::string);
+    std::pair<std::string, std::vector<T *>>* 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
@@ -25,48 +25,11 @@
 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...
 // 
-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
-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
-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
-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
-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
 
@@ -101,5 +64,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

Original file line number	Diff line number	Diff line change
`@@ -310,7 +310,7 @@ class TransactionSecMarkerManagement {`
`310`	`310`	`if (m_marker) {`
`311`	`311`	`return m_marker;`
`312`	`312`	`} else {`
`313`		`- throw;`
	`313`	`+ throw; // cppcheck-suppress rethrowNoCurrentException`
`314`	`314`	`}`
`315`	`315`	`}`
`316`	`316`
`@@ -405,7 +405,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa`
`405`	`405`	`size_t getRequestBodyLength();`
`406`	`406`
`407`	`407`	`#ifndef NO_LOGS`
`408`		`- void debug(int, const std::string&) const;`
	`408`	`+ void debug(int, const std::string &) const; // cppcheck-suppress functionStatic`
`409`	`409`	`#endif`
`410`	`410`	`void serverLog(std::shared_ptr<RuleMessage> rm);`
`411`	`411`
Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@ bool Parallel::write(Transaction transaction, int parts, std::string error) {`
`118`	`118`	`log = transaction->toOldAuditLogFormat(parts, "-" + boundary + "--");`
`119`	`119`	`}`
`120`	`120`
`121`		`- std::string logPath = m_audit->m_storage_dir;`
	`121`	`+ const auto &logPath = m_audit->m_storage_dir;`
`122`	`122`	`fileName = logPath + fileName + "-" + *transaction->m_id.get();`
`123`	`123`
`124`	`124`	`if (logPath.empty()) {`
Original file line number	Diff line number	Diff line change
`@@ -659,7 +659,7 @@ MDB_dbi* MDBEnvProvider::GetDBI() {`
`659`	`659`	`return &m_dbi;`
`660`	`660`	`}`
`661`	`661`
`662`		`-bool MDBEnvProvider::isValid() {`
	`662`	`+bool MDBEnvProvider::isValid() const {`
`663`	`663`	`return valid;`
`664`	`664`	`}`
`665`	`665`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ class MDBEnvProvider {`
`83`	`83`	`}`
`84`	`84`	`MDB_env* GetEnv();`
`85`	`85`	`MDB_dbi* GetDBI();`
`86`		`- bool isValid();`
	`86`	`+ bool isValid() const;`
`87`	`87`
`88`	`88`	`~MDBEnvProvider();`
`89`	`89`	`private:`
Original file line number	Diff line number	Diff line change
`@@ -202,7 +202,7 @@ void ModSecurity::serverLog(void *data, std::shared_ptr<RuleMessage> rm) {`
`202`	`202`	`}`
`203`	`203`
`204`	`204`	`if (m_logProperties & TextLogProperty) {`
`205`		`- std::string &&d = rm->log();`
	`205`	`+ auto d = rm->log();`
`206`	`206`	`const void a = static_cast<const void >(d.c_str());`
`207`	`207`	`m_logCb(data, a);`
`208`	`208`	`return;`