diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71ca3c7c57..2fe0010a0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -178,20 +178,21 @@ jobs: ctest -C ${{ matrix.configuration }} --output-on-failure cppcheck: - runs-on: [ubuntu-22.04] + runs-on: [macos-14] steps: - name: Setup Dependencies run: | - sudo apt-get update -y -qq - sudo apt-get install -y cppcheck - - name: Checkout source - uses: actions/checkout@v4 + brew install autoconf \ + automake \ + libtool \ + cppcheck + - uses: actions/checkout@v4 with: submodules: true fetch-depth: 0 - - name: Configure libModSecurity + - name: configure run: | ./build.sh ./configure - - name: Run cppcheck on libModSecurity + - name: cppcheck run: make check-static diff --git a/Makefile.am b/Makefile.am index ab22102dcd..4468ba7154 100644 --- a/Makefile.am +++ b/Makefile.am @@ -63,9 +63,11 @@ cppcheck: --enable=warning,style,performance,portability,unusedFunction,missingInclude \ --inconclusive \ --template="warning: {file},{line},{severity},{id},{message}" \ - -I headers -I . -I others -I src -I others/mbedtls/include -I src/parser \ + -I headers -I . -I others -I src -I others/mbedtls/include \ --error-exitcode=1 \ -i "src/parser/seclang-parser.cc" -i "src/parser/seclang-scanner.cc" \ + -i others \ + --std=c++17 \ --force --verbose . diff --git a/examples/multithread/multithread.cc b/examples/multithread/multithread.cc index 4233bc71fa..b679e40623 100644 --- a/examples/multithread/multithread.cc +++ b/examples/multithread/multithread.cc @@ -40,7 +40,7 @@ int main (int argc, char *argv[]) { modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha (Simple " \ "example on how to use ModSecurity API"); - char main_rule_uri[] = "basic_rules.conf"; + const char main_rule_uri[] = "basic_rules.conf"; auto rules = std::make_unique(); if (rules->loadFromUri(main_rule_uri) < 0) { std::cerr << "Problems loading the rules..." << std::endl; diff --git a/headers/modsecurity/anchored_set_variable_translation_proxy.h b/headers/modsecurity/anchored_set_variable_translation_proxy.h index 10f3f7f2d2..37767b980c 100644 --- a/headers/modsecurity/anchored_set_variable_translation_proxy.h +++ b/headers/modsecurity/anchored_set_variable_translation_proxy.h @@ -42,7 +42,7 @@ class AnchoredSetVariableTranslationProxy { : m_name(name), m_fount(fount) { - m_translate = [](std::string *name, std::vector *l) { + m_translate = [](const std::string *name, std::vector *l) { for (int i = 0; i < l->size(); ++i) { VariableValue *newVariableValue = new VariableValue(name, &l->at(i)->getKey(), &l->at(i)->getKey()); const VariableValue *oldVariableValue = l->at(i); diff --git a/headers/modsecurity/rule_with_actions.h b/headers/modsecurity/rule_with_actions.h index aee16d8c04..541eb3f4bf 100644 --- a/headers/modsecurity/rule_with_actions.h +++ b/headers/modsecurity/rule_with_actions.h @@ -79,7 +79,7 @@ class RuleWithActions : public Rule { bool chainedParentNull = false) const; std::vector getActionsByName(const std::string& name, - Transaction *t); + const Transaction *t); bool containsTag(const std::string& name, Transaction *t); bool containsMsg(const std::string& name, Transaction *t); diff --git a/headers/modsecurity/rule_with_operator.h b/headers/modsecurity/rule_with_operator.h index d0297409f0..7fb9b2d280 100644 --- a/headers/modsecurity/rule_with_operator.h +++ b/headers/modsecurity/rule_with_operator.h @@ -62,7 +62,7 @@ class RuleWithOperator : public RuleWithActions { static void cleanMatchedVars(Transaction *trasn); - std::string getOperatorName() const; + const std::string& getOperatorName() const; virtual std::string getReference() override { return std::to_string(m_ruleId); diff --git a/headers/modsecurity/rules.h b/headers/modsecurity/rules.h index a9bfb80d3e..cb83dedb39 100644 --- a/headers/modsecurity/rules.h +++ b/headers/modsecurity/rules.h @@ -50,7 +50,7 @@ class Rules { int append(Rules *from, const std::vector &ids, std::ostringstream *err) { size_t j = 0; for (; j < from->size(); j++) { - RuleWithOperator *rule = dynamic_cast(from->at(j).get()); + const RuleWithOperator *rule = dynamic_cast(from->at(j).get()); if (rule && std::binary_search(ids.begin(), ids.end(), rule->m_ruleId)) { if (err != NULL) { *err << "Rule id: " << std::to_string(rule->m_ruleId) \ @@ -68,7 +68,7 @@ class Rules { } bool insert(std::shared_ptr rule, const std::vector *ids, std::ostringstream *err) { - RuleWithOperator *r = dynamic_cast(rule.get()); + const RuleWithOperator *r = dynamic_cast(rule.get()); if (r && ids != nullptr && std::binary_search(ids->begin(), ids->end(), r->m_ruleId)) { if (err != nullptr) { *err << "Rule id: " << std::to_string(r->m_ruleId) \ diff --git a/headers/modsecurity/rules_set.h b/headers/modsecurity/rules_set.h index c5616cc426..e1574594d4 100644 --- a/headers/modsecurity/rules_set.h +++ b/headers/modsecurity/rules_set.h @@ -93,7 +93,7 @@ extern "C" { #endif RulesSet *msc_create_rules_set(void); -void msc_rules_dump(RulesSet *rules); +void msc_rules_dump(const RulesSet *rules); int msc_rules_merge(RulesSet *rules_dst, RulesSet *rules_from, const char **error); int msc_rules_add_remote(RulesSet *rules, const char *key, const char *uri, const char **error); diff --git a/headers/modsecurity/rules_set_properties.h b/headers/modsecurity/rules_set_properties.h index 643abce834..386a252d75 100644 --- a/headers/modsecurity/rules_set_properties.h +++ b/headers/modsecurity/rules_set_properties.h @@ -70,7 +70,7 @@ class ConfigInt { bool m_set; int m_value; - void merge(ConfigInt *from) { + void merge(const ConfigInt *from) { if (m_set == true || from->m_set == false) { return; } @@ -87,7 +87,7 @@ class ConfigDouble { bool m_set; double m_value; - void merge(ConfigDouble *from) { + void merge(const ConfigDouble *from) { if (m_set == true || from->m_set == false) { return; } @@ -104,7 +104,7 @@ class ConfigString { bool m_set; std::string m_value; - void merge(ConfigString *from) { + void merge(const ConfigString *from) { if (m_set == true || from->m_set == false) { return; } @@ -150,7 +150,7 @@ class ConfigUnicodeMap { static void loadConfig(std::string f, double codePage, RulesSetProperties *driver, std::string *errg); - void merge(ConfigUnicodeMap *from) { + void merge(const ConfigUnicodeMap *from) { if (from->m_set == false) { return; } diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index 488e6c5a7c..f294c246a8 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; // cppcheck-suppress functionStatic + void debug(int, const std::string &) const; #endif void serverLog(const RuleMessage &rm); @@ -713,7 +713,7 @@ int msc_process_uri(Transaction *transaction, const char *uri, const char *protocol, const char *http_version); /** @ingroup ModSecurity_C_API */ -const char *msc_get_response_body(Transaction *transaction); +const char *msc_get_response_body(const Transaction *transaction); /** @ingroup ModSecurity_C_API */ size_t msc_get_response_body_length(Transaction *transaction); diff --git a/src/actions/ctl/rule_remove_by_id.cc b/src/actions/ctl/rule_remove_by_id.cc index 869dc4f945..ff8673814f 100644 --- a/src/actions/ctl/rule_remove_by_id.cc +++ b/src/actions/ctl/rule_remove_by_id.cc @@ -84,10 +84,10 @@ bool RuleRemoveById::init(std::string *error) { } bool RuleRemoveById::evaluate(RuleWithActions *rule, Transaction *transaction) { - for (auto &i : m_ids) { + for (const auto &i : m_ids) { transaction->m_ruleRemoveById.push_back(i); } - for (auto &i : m_ranges) { + for (const auto &i : m_ranges) { transaction->m_ruleRemoveByIdRange.push_back(i); } diff --git a/src/actions/exec.h b/src/actions/exec.h index f899982182..d1f42f8531 100644 --- a/src/actions/exec.h +++ b/src/actions/exec.h @@ -31,10 +31,7 @@ namespace actions { class Exec : public Action { public: explicit Exec(const std::string &action) - : Action(action), - m_script("") { } - - ~Exec() { } + : Action(action) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; bool init(std::string *error) override; diff --git a/src/actions/expire_var.cc b/src/actions/expire_var.cc index 988bc6c68c..ffc89f4779 100644 --- a/src/actions/expire_var.cc +++ b/src/actions/expire_var.cc @@ -32,11 +32,6 @@ namespace modsecurity { namespace actions { -bool ExpireVar::init(std::string *error) { - return true; -} - - bool ExpireVar::evaluate(RuleWithActions *rule, Transaction *t) { std::string expireExpressionExpanded(m_string->evaluate(t)); diff --git a/src/actions/expire_var.h b/src/actions/expire_var.h index f0ca7496fe..3c15e9865e 100644 --- a/src/actions/expire_var.h +++ b/src/actions/expire_var.h @@ -40,7 +40,6 @@ class ExpireVar : public Action { m_string(std::move(z)) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; - bool init(std::string *error) override; private: diff --git a/src/actions/set_env.cc b/src/actions/set_env.cc index 2929065313..a14c0ad309 100644 --- a/src/actions/set_env.cc +++ b/src/actions/set_env.cc @@ -26,11 +26,6 @@ namespace modsecurity { namespace actions { -bool SetENV::init(std::string *error) { - return true; -} - - bool SetENV::evaluate(RuleWithActions *rule, Transaction *t) { std::string colNameExpanded(m_string->evaluate(t)); diff --git a/src/actions/set_env.h b/src/actions/set_env.h index cc24c28508..abdc6b5d13 100644 --- a/src/actions/set_env.h +++ b/src/actions/set_env.h @@ -40,7 +40,6 @@ class SetENV : public Action { m_string(std::move(z)) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; - bool init(std::string *error) override; private: std::unique_ptr m_string; diff --git a/src/actions/set_rsc.cc b/src/actions/set_rsc.cc index 8464a9b688..5b31c0a163 100644 --- a/src/actions/set_rsc.cc +++ b/src/actions/set_rsc.cc @@ -26,11 +26,6 @@ namespace modsecurity { namespace actions { -bool SetRSC::init(std::string *error) { - return true; -} - - bool SetRSC::evaluate(RuleWithActions *rule, Transaction *t) { std::string colNameExpanded(m_string->evaluate(t)); ms_dbg_a(t, 8, "RESOURCE initiated with value: \'" diff --git a/src/actions/set_rsc.h b/src/actions/set_rsc.h index 7830ee92c3..2264e82b88 100644 --- a/src/actions/set_rsc.h +++ b/src/actions/set_rsc.h @@ -40,7 +40,6 @@ class SetRSC : public Action { m_string(std::move(z)) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; - bool init(std::string *error) override; private: std::unique_ptr m_string; diff --git a/src/actions/set_sid.cc b/src/actions/set_sid.cc index c7a0db687a..4117d35120 100644 --- a/src/actions/set_sid.cc +++ b/src/actions/set_sid.cc @@ -26,11 +26,6 @@ namespace modsecurity { namespace actions { -bool SetSID::init(std::string *error) { - return true; -} - - bool SetSID::evaluate(RuleWithActions *rule, Transaction *t) { std::string colNameExpanded(m_string->evaluate(t)); ms_dbg_a(t, 8, "Session ID initiated with value: \'" diff --git a/src/actions/set_sid.h b/src/actions/set_sid.h index dca45433d1..1048caef1c 100644 --- a/src/actions/set_sid.h +++ b/src/actions/set_sid.h @@ -40,7 +40,6 @@ class SetSID : public Action { m_string(std::move(z)) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; - bool init(std::string *error) override; private: std::unique_ptr m_string; diff --git a/src/actions/set_uid.cc b/src/actions/set_uid.cc index 997df77b63..01f74b84b1 100644 --- a/src/actions/set_uid.cc +++ b/src/actions/set_uid.cc @@ -26,11 +26,6 @@ namespace modsecurity { namespace actions { -bool SetUID::init(std::string *error) { - return true; -} - - bool SetUID::evaluate(RuleWithActions *rule, Transaction *t) { std::string colNameExpanded(m_string->evaluate(t)); ms_dbg_a(t, 8, "User collection initiated with value: \'" diff --git a/src/actions/set_uid.h b/src/actions/set_uid.h index 76893999fc..49a36c1d7a 100644 --- a/src/actions/set_uid.h +++ b/src/actions/set_uid.h @@ -40,7 +40,6 @@ class SetUID : public Action { m_string(std::move(z)) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; - bool init(std::string *error) override; private: std::unique_ptr m_string; diff --git a/src/actions/set_var.cc b/src/actions/set_var.cc index f30ae18864..70c07d065d 100644 --- a/src/actions/set_var.cc +++ b/src/actions/set_var.cc @@ -35,11 +35,6 @@ namespace modsecurity { namespace actions { -bool SetVar::init(std::string *error) { - return true; -} - - bool SetVar::evaluate(RuleWithActions *rule, Transaction *t) { std::string targetValue; std::string resolvedPre; diff --git a/src/actions/set_var.h b/src/actions/set_var.h index 5fe9de0329..e89e8be74e 100644 --- a/src/actions/set_var.h +++ b/src/actions/set_var.h @@ -59,7 +59,6 @@ class SetVar : public Action { m_variable(std::move(variable)) { } bool evaluate(RuleWithActions *rule, Transaction *transaction) override; - bool init(std::string *error) override; private: SetVarOperation m_operation; diff --git a/src/actions/transformations/hex_decode.cc b/src/actions/transformations/hex_decode.cc index 1b43b6cdf2..ed3c76f608 100644 --- a/src/actions/transformations/hex_decode.cc +++ b/src/actions/transformations/hex_decode.cc @@ -26,7 +26,7 @@ static inline int inplace(std::string &value) { const auto len = value.length(); auto d = reinterpret_cast(value.data()); - const auto data = d; + const auto *data = d; for (int i = 0; i <= len - 2; i += 2) { *d++ = utils::string::x2c(&data[i]); diff --git a/src/actions/transformations/html_entity_decode.cc b/src/actions/transformations/html_entity_decode.cc index 8a0cc3e97d..9c6b989d57 100644 --- a/src/actions/transformations/html_entity_decode.cc +++ b/src/actions/transformations/html_entity_decode.cc @@ -118,7 +118,7 @@ static inline bool inplace(std::string &value) { j++; } if (j > k) { /* Do we have at least one digit? */ - const auto x = reinterpret_cast(&input[k]); + const auto *x = reinterpret_cast(&input[k]); /* Decode the entity. */ /* ENH What about others? */ diff --git a/src/actions/transformations/normalise_path.cc b/src/actions/transformations/normalise_path.cc index 91c0402729..5c67ddc150 100644 --- a/src/actions/transformations/normalise_path.cc +++ b/src/actions/transformations/normalise_path.cc @@ -29,9 +29,6 @@ bool NormalisePath::transform(std::string &value, const Transaction *trans) cons * IMP1 Assumes NUL-terminated */ bool NormalisePath::normalize_path_inplace(std::string &val, const bool win) { - unsigned char *src; - unsigned char *dst; - unsigned char *end; int hitroot = 0; int done = 0; int relative; @@ -49,13 +46,13 @@ bool NormalisePath::normalize_path_inplace(std::string &val, const bool win) { * ENH: Deal with UNC and drive letters? */ - src = dst = input; - end = input + (input_len - 1); + auto src = input; + auto dst = input; + const auto *end = input + (input_len - 1); relative = ((*input == '/') || (win && (*input == '\\'))) ? 0 : 1; trailing = ((*end == '/') || (win && (*end == '\\'))) ? 1 : 0; - while (!done && (src <= end) && (dst <= end)) { /* Convert backslash to forward slash on Windows only. */ if (win) { @@ -152,7 +149,7 @@ bool NormalisePath::normalize_path_inplace(std::string &val, const bool win) { /* Skip to the last forward slash when multiple are used. */ if (*src == '/') { - unsigned char *oldsrc = src; + const unsigned char *oldsrc = src; while ((src < end) && ((*(src + 1) == '/') || (win && (*(src + 1) == '\\'))) ) { diff --git a/src/actions/transformations/parity_zero_7bit.cc b/src/actions/transformations/parity_zero_7bit.cc index 77ad05145b..54972b93b4 100644 --- a/src/actions/transformations/parity_zero_7bit.cc +++ b/src/actions/transformations/parity_zero_7bit.cc @@ -22,7 +22,7 @@ namespace modsecurity::actions::transformations { static inline bool inplace(std::string &value) { if (value.empty()) return false; - for(auto &c : value) { + for(auto &c : value) { // cppcheck-suppress constVariableReference ; false positive ((unsigned char&)c) &= 0x7f; } diff --git a/src/actions/transformations/sql_hex_decode.cc b/src/actions/transformations/sql_hex_decode.cc index 0ce2408726..de0ea26b6e 100644 --- a/src/actions/transformations/sql_hex_decode.cc +++ b/src/actions/transformations/sql_hex_decode.cc @@ -38,7 +38,7 @@ static inline bool inplace(std::string &value) { auto d = reinterpret_cast(value.data()); const unsigned char *data = d; - const auto end = data + value.size(); + const auto *end = data + value.size(); bool changed = false; diff --git a/src/collection/backend/in_memory-per_process.h b/src/collection/backend/in_memory-per_process.h index 2e283d2582..4aa7b1d076 100644 --- a/src/collection/backend/in_memory-per_process.h +++ b/src/collection/backend/in_memory-per_process.h @@ -74,7 +74,7 @@ class InMemoryPerProcess : public Collection { public: explicit InMemoryPerProcess(const std::string &name); - ~InMemoryPerProcess(); + ~InMemoryPerProcess() override; void store(const std::string &key, const std::string &value); bool storeOrUpdateFirst(const std::string &key, diff --git a/src/collection/backend/lmdb.cc b/src/collection/backend/lmdb.cc index 3fe68061e5..ffad0c7675 100644 --- a/src/collection/backend/lmdb.cc +++ b/src/collection/backend/lmdb.cc @@ -559,7 +559,7 @@ void LMDB::resolveMultiMatches(const std::string& var, continue; } - char *a = reinterpret_cast(key.mv_data); + const char *a = reinterpret_cast(key.mv_data); if (strncmp(var.c_str(), a, keySize) == 0) { std::string key_to_insert(reinterpret_cast(key.mv_data), key.mv_size); l->insert(l->begin(), new VariableValue(&m_name, &key_to_insert, &collectionData.getValue())); diff --git a/src/compat/msvc.h b/src/compat/msvc.h index ce9a14c0a4..75630691cc 100644 --- a/src/compat/msvc.h +++ b/src/compat/msvc.h @@ -14,6 +14,7 @@ #define pclose _pclose inline tm* localtime_r(const time_t* tin, tm* tout) { + // cppcheck-suppress[uninitvar, ctuuninitvar] if (!localtime_s(tout, tin)) return tout; return nullptr; diff --git a/src/engine/lua.cc b/src/engine/lua.cc index 1bed904442..9313a2ec7f 100644 --- a/src/engine/lua.cc +++ b/src/engine/lua.cc @@ -114,14 +114,14 @@ int Lua::blob_keeper(lua_State *L, const void *p, size_t sz, void *ud) { const char *Lua::blob_reader(lua_State *L, void *ud, size_t *size) { - LuaScriptBlob *lsb = static_cast(ud); + const LuaScriptBlob *lsb = static_cast(ud); const char *data = lsb->read(size); return data; } #endif -int Lua::run(Transaction *t, const std::string &str) { +int Lua::run(Transaction *t, const std::string &str) { // cppcheck-suppress constParameterPointer #ifdef WITH_LUA std::string luaRet; const char *a = NULL; diff --git a/src/engine/lua.h b/src/engine/lua.h index 452c7f4989..37a150b0ef 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); // cppcheck-suppress functionStatic ; triggered when compiling without LUA - int run(Transaction *t, const std::string &str = ""); // cppcheck-suppress functionStatic ; triggered when compiling without LUA + bool load(const std::string &script, std::string *err); + int run(Transaction *t, const std::string &str = ""); static bool isCompatible(const std::string &script, Lua *l, std::string *error); #ifdef WITH_LUA diff --git a/src/operators/fuzzy_hash.h b/src/operators/fuzzy_hash.h index b555eff55f..10a0ca6dc0 100644 --- a/src/operators/fuzzy_hash.h +++ b/src/operators/fuzzy_hash.h @@ -42,7 +42,7 @@ class FuzzyHash : public Operator { : Operator("FuzzyHash", std::move(param)), m_threshold(0), m_head(NULL) { } - ~FuzzyHash(); + ~FuzzyHash() override; bool evaluate(Transaction *transaction, const std::string &std) override; diff --git a/src/operators/geo_lookup.cc b/src/operators/geo_lookup.cc index 5717b7afb3..6bd7918594 100644 --- a/src/operators/geo_lookup.cc +++ b/src/operators/geo_lookup.cc @@ -30,8 +30,13 @@ #include "src/utils/geo_lookup.h" -namespace modsecurity { -namespace operators { +namespace modsecurity::operators { + + +static bool debug(const Transaction *transaction, int x, const std::string &a) { + ms_dbg_a(transaction, x, a); + return true; +} bool GeoLookup::evaluate(Transaction *trans, const std::string &exp) { @@ -41,9 +46,9 @@ bool GeoLookup::evaluate(Transaction *trans, const std::string &exp) { if (trans) { ret = Utils::GeoLookup::getInstance().lookup(exp, trans, - std::bind(&GeoLookup::debug, this, trans, _1, _2)); + std::bind(debug, trans, _1, _2)); } else { - ret = Utils::GeoLookup::getInstance().lookup(exp, NULL, + ret = Utils::GeoLookup::getInstance().lookup(exp, nullptr, nullptr); } @@ -51,5 +56,4 @@ bool GeoLookup::evaluate(Transaction *trans, const std::string &exp) { } -} // namespace operators -} // namespace modsecurity +} // namespace modsecurity::operators diff --git a/src/operators/geo_lookup.h b/src/operators/geo_lookup.h index 187a2f317e..b12b031aa7 100644 --- a/src/operators/geo_lookup.h +++ b/src/operators/geo_lookup.h @@ -21,8 +21,8 @@ #include "src/operators/operator.h" -namespace modsecurity { -namespace operators { +namespace modsecurity::operators { + class GeoLookup : public Operator { public: @@ -30,17 +30,10 @@ class GeoLookup : public Operator { GeoLookup() : Operator("GeoLookup") { } 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; - } }; -} // namespace operators -} // namespace modsecurity + +} // namespace modsecurity::operators #endif // SRC_OPERATORS_GEO_LOOKUP_H_ diff --git a/src/operators/operator.h b/src/operators/operator.h index ba94a1b466..636ad5e00c 100644 --- a/src/operators/operator.h +++ b/src/operators/operator.h @@ -136,7 +136,7 @@ class Operator { std::string m_match_message; bool m_negation; - std::string m_op; + const std::string m_op; std::string m_param; std::unique_ptr m_string; bool m_couldContainsMacro; diff --git a/src/operators/pm.h b/src/operators/pm.h index 1b45fff79e..3ac1ffec7f 100644 --- a/src/operators/pm.h +++ b/src/operators/pm.h @@ -40,7 +40,7 @@ class Pm : public Operator { : Operator(n, std::move(param)) { m_p = acmp_create(0); } - ~Pm(); + ~Pm() override; bool evaluate(Transaction *transaction, RuleWithActions *rule, const std::string &str, RuleMessage &ruleMessage) override; diff --git a/src/operators/rbl.cc b/src/operators/rbl.cc index 35bbc5f934..4b06f337ae 100644 --- a/src/operators/rbl.cc +++ b/src/operators/rbl.cc @@ -73,8 +73,8 @@ std::string Rbl::mapIpToAddress(const std::string &ipStr, Transaction *trans) co void Rbl::futherInfo_httpbl(struct sockaddr_in *sin, const std::string &ipStr, - Transaction *trans) { - char *respBl; + const Transaction *trans) { + const char *respBl; int first, days, score, type; #ifndef NO_LOGS std::string ptype; @@ -131,7 +131,7 @@ void Rbl::futherInfo_httpbl(struct sockaddr_in *sin, const std::string &ipStr, void Rbl::futherInfo_spamhaus(unsigned int high8bits, const std::string &ipStr, - Transaction *trans) { + const Transaction *trans) { switch (high8bits) { case 2: case 3: @@ -158,7 +158,7 @@ void Rbl::futherInfo_spamhaus(unsigned int high8bits, const std::string &ipStr, void Rbl::futherInfo_uribl(unsigned int high8bits, const std::string &ipStr, - Transaction *trans) { + const Transaction *trans) { switch (high8bits) { case 2: ms_dbg_a(trans, 4, "RBL lookup of " + ipStr + " succeeded (BLACK)."); @@ -185,7 +185,7 @@ void Rbl::futherInfo_uribl(unsigned int high8bits, const std::string &ipStr, void Rbl::furtherInfo(struct sockaddr_in *sin, const std::string &ipStr, - Transaction *trans, RblProvider provider) { + const Transaction *trans, RblProvider provider) { unsigned int high8bits = sin->sin_addr.s_addr >> 24; switch (provider) { diff --git a/src/operators/rbl.h b/src/operators/rbl.h index e7d9538cd3..a36b0d9d66 100644 --- a/src/operators/rbl.h +++ b/src/operators/rbl.h @@ -33,8 +33,8 @@ #include "src/operators/operator.h" -namespace modsecurity { -namespace operators { +namespace modsecurity::operators { + class Rbl : public Operator { public: @@ -66,21 +66,18 @@ class Rbl : public Operator { /** @ingroup ModSecurity_Operator */ explicit Rbl(std::unique_ptr param) - : m_service(), - m_demandsPassword(false), - m_provider(RblProvider::UnknownProvider), - Operator("Rbl", std::move(param)) { - m_service = m_string->evaluate(); // cppcheck-suppress useInitializationList + : 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; - } else if (m_service.find("uribl.com") != std::string::npos) { - m_provider = RblProvider::uribl; - } else if (m_service.find("spamhaus.org") != std::string::npos) { - m_provider = RblProvider::spamhaus; - } + } else if (m_service.find("uribl.com") != std::string::npos) { + m_provider = RblProvider::uribl; + } else if (m_service.find("spamhaus.org") != std::string::npos) { + m_provider = RblProvider::spamhaus; } + } bool evaluate(Transaction *transaction, RuleWithActions *rule, const std::string& input, RuleMessage &ruleMessage) override; @@ -88,22 +85,22 @@ class Rbl : public Operator { std::string mapIpToAddress(const std::string &ipStr, Transaction *trans) const; static void futherInfo_httpbl(struct sockaddr_in *sin, const std::string &ipStr, - Transaction *trans); + const Transaction *trans); static void futherInfo_spamhaus(unsigned int high8bits, const std::string &ipStr, - Transaction *trans); + const Transaction *trans); static void futherInfo_uribl(unsigned int high8bits, const std::string &ipStr, - Transaction *trans); + const Transaction *trans); static void furtherInfo(struct sockaddr_in *sin, const std::string &ipStr, - Transaction *trans, RblProvider provider); + const Transaction *trans, RblProvider provider); private: std::string m_service; - bool m_demandsPassword; - RblProvider m_provider; + bool m_demandsPassword = false; + RblProvider m_provider = RblProvider::UnknownProvider; }; -} // namespace operators -} // namespace modsecurity + +} // namespace modsecurity::operators #endif // SRC_OPERATORS_RBL_H_ diff --git a/src/operators/rx.h b/src/operators/rx.h index 86a12d523a..bc08969005 100644 --- a/src/operators/rx.h +++ b/src/operators/rx.h @@ -42,7 +42,7 @@ class Rx : public Operator { m_couldContainsMacro = true; } - ~Rx() { + ~Rx() override { if (m_string->m_containsMacro == false && m_re != NULL) { delete m_re; m_re = NULL; diff --git a/src/operators/rx_global.h b/src/operators/rx_global.h index 5b6ed8757b..82684eea28 100644 --- a/src/operators/rx_global.h +++ b/src/operators/rx_global.h @@ -42,7 +42,7 @@ class RxGlobal : public Operator { m_couldContainsMacro = true; } - ~RxGlobal() { + ~RxGlobal() override { if (m_string->m_containsMacro == false && m_re != NULL) { delete m_re; m_re = NULL; diff --git a/src/operators/validate_dtd.h b/src/operators/validate_dtd.h index 9de1ea1da0..4056c38377 100644 --- a/src/operators/validate_dtd.h +++ b/src/operators/validate_dtd.h @@ -28,6 +28,7 @@ #include #include "src/operators/operator.h" +#include "validate_schema.h" namespace modsecurity { @@ -57,47 +58,27 @@ class ValidateDTD : public Operator { explicit ValidateDTD(std::unique_ptr param) : Operator("ValidateDTD", std::move(param)) { } #ifdef WITH_LIBXML2 - ~ValidateDTD() { } - bool evaluate(Transaction *transaction, const std::string &str) override; bool init(const std::string &file, std::string *error) override; static void error_runtime(void *ctx, const char *msg, ...) { - Transaction *t = reinterpret_cast(ctx); - char buf[1024]; - std::string s; va_list args; - va_start(args, msg); - int len = vsnprintf(buf, sizeof(buf), msg, args); + ValidateSchema::callback_func(ctx, ValidateSchema::log_msg, ValidateSchema::PREFIX_ERROR, msg, args); va_end(args); - - if (len > 0) { - s = "XML Error: " + std::string(buf); - } - ms_dbg_a(t, 4, s); } static void warn_runtime(void *ctx, const char *msg, ...) { - Transaction *t = reinterpret_cast(ctx); - char buf[1024]; - std::string s; va_list args; - va_start(args, msg); - int len = vsnprintf(buf, sizeof(buf), msg, args); + ValidateSchema::callback_func(ctx, ValidateSchema::log_msg, ValidateSchema::PREFIX_WARNING, msg, args); va_end(args); - - if (len > 0) { - s = "XML Warning: " + std::string(buf); - } - ms_dbg_a(t, 4, s); } - static void null_error(void *ctx, const char *msg, ...) { + static void null_error(void *, const char *, ...) { // cppcheck-suppress[constParameterPointer,constParameterCallback] } private: diff --git a/src/operators/validate_schema.h b/src/operators/validate_schema.h index 02f07e4dc3..5708cfae18 100644 --- a/src/operators/validate_schema.h +++ b/src/operators/validate_schema.h @@ -38,7 +38,6 @@ class ValidateSchema : public Operator { /** @ingroup ModSecurity_Operator */ explicit ValidateSchema(std::unique_ptr param) : Operator("ValidateSchema", std::move(param)) { } - ~ValidateSchema() { } #ifdef WITH_LIBXML2 bool evaluate(Transaction *transaction, const std::string &str) override; @@ -46,71 +45,61 @@ class ValidateSchema : public Operator { static void error_load(void *ctx, const char *msg, ...) { - std::string *t = reinterpret_cast(ctx); - char buf[1024]; va_list args; - va_start(args, msg); - int len = vsnprintf(buf, sizeof(buf), msg, args); + callback_func(ctx, append_msg, PREFIX_ERROR, msg, args); va_end(args); - - if (len > 0) { - t->append("XML Error: " + std::string(buf)); - } } static void warn_load(void *ctx, const char *msg, ...) { - std::string *t = reinterpret_cast(ctx); - char buf[1024]; va_list args; - va_start(args, msg); - int len = vsnprintf(buf, sizeof(buf), msg, args); + callback_func(ctx, append_msg, PREFIX_WARNING, msg, args); va_end(args); - - if (len > 0) { - t->append("XML Warning: " + std::string(buf)); - } } static void error_runtime(void *ctx, const char *msg, ...) { - Transaction *t = reinterpret_cast(ctx); - char buf[1024]; - std::string s; va_list args; - va_start(args, msg); - int len = vsnprintf(buf, sizeof(buf), msg, args); + callback_func(ctx, log_msg, PREFIX_ERROR, msg, args); va_end(args); - - if (len > 0) { - s = "XML Error: " + std::string(buf); - } - ms_dbg_a(t, 4, s); } static void warn_runtime(void *ctx, const char *msg, ...) { - Transaction *t = reinterpret_cast(ctx); - char buf[1024]; - std::string s; va_list args; - va_start(args, msg); - int len = vsnprintf(buf, sizeof(buf), msg, args); + callback_func(ctx, log_msg, PREFIX_WARNING, msg, args); va_end(args); + } - if (len > 0) { - s = "XML Warning: " + std::string(buf); - } - ms_dbg_a(t, 4, s); + static void null_error(void *, const char *, ...) { // cppcheck-suppress[constParameterPointer,constParameterCallback] } - static void null_error(void *ctx, const char *msg, ...) { + template + static void callback_func(void *ctx, Pred pred, const char *base_msg, const char *msg, va_list args) { + char buf[1024]; + const auto len = vsnprintf(buf, sizeof(buf), msg, args); + + if (len > 0) + pred(ctx, base_msg + std::string(buf)); } + static void log_msg(const void *ctx, const std::string &msg) { + auto t = reinterpret_cast(ctx); + ms_dbg_a(t, 4, msg); + } + + static void append_msg(void *ctx, const std::string &msg) { + auto s = reinterpret_cast(ctx); + s->append(msg); + } + + static constexpr auto PREFIX_WARNING = "XML Warning: "; + static constexpr auto PREFIX_ERROR = "XML Error: "; + private: std::string m_resource; std::string m_err; diff --git a/src/operators/validate_url_encoding.cc b/src/operators/validate_url_encoding.cc index 7ca71b221c..65a3a328b8 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; // cppcheck-suppress knownConditionTrueFalse + return res; } int rc = validate_url_encoding(input.c_str(), input.size(), &offset); diff --git a/src/operators/verify_cc.h b/src/operators/verify_cc.h index e14f728af0..05d4cdec7e 100644 --- a/src/operators/verify_cc.h +++ b/src/operators/verify_cc.h @@ -45,7 +45,7 @@ class VerifyCC : public Operator { m_pc(NULL), m_pce(NULL) { } #endif - ~VerifyCC(); + ~VerifyCC() override; bool evaluate(Transaction *t, RuleWithActions *rule, const std::string& input, diff --git a/src/operators/verify_cpf.cc b/src/operators/verify_cpf.cc index b012eac826..07ebe74515 100644 --- a/src/operators/verify_cpf.cc +++ b/src/operators/verify_cpf.cc @@ -38,7 +38,7 @@ int VerifyCPF::convert_to_int(const char c) { } -bool VerifyCPF::verify(const char *cpfnumber, int len) { +bool VerifyCPF::verify(const char *cpfnumber, int len) const { int factor, part_1, part_2, var_len = len; unsigned int sum = 0, i = 0, cpf_len = 11, c; int cpf[11]; @@ -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); // cppcheck-suppress uninitvar + sum += (cpf[i] * --c); } factor = (sum % cpf_len); diff --git a/src/operators/verify_cpf.h b/src/operators/verify_cpf.h index ccb9988d17..22e260265b 100644 --- a/src/operators/verify_cpf.h +++ b/src/operators/verify_cpf.h @@ -35,13 +35,8 @@ class VerifyCPF : public Operator { public: /** @ingroup ModSecurity_Operator */ explicit VerifyCPF(std::unique_ptr param) - : Operator("VerifyCPF", std::move(param)) { - m_re = new Regex(m_param); - } - - ~VerifyCPF() { - delete m_re; - } + : Operator("VerifyCPF", std::move(param)) + , m_re(std::make_unique(m_param)) { } bool operator=(const VerifyCPF &a) = delete; VerifyCPF(const VerifyCPF &a) = delete; @@ -50,11 +45,11 @@ class VerifyCPF : public Operator { const std::string& input, RuleMessage &ruleMessage) override; - bool verify(const char *ssnumber, int len); + bool verify(const char *ssnumber, int len) const; private: static int convert_to_int(const char c); - Regex *m_re; + std::unique_ptr m_re; const char bad_cpf[12][12] = { "00000000000", "01234567890", "11111111111", diff --git a/src/operators/verify_ssn.h b/src/operators/verify_ssn.h index 7bb40c33c2..5b54492be2 100644 --- a/src/operators/verify_ssn.h +++ b/src/operators/verify_ssn.h @@ -35,13 +35,8 @@ class VerifySSN : public Operator { public: /** @ingroup ModSecurity_Operator */ explicit VerifySSN(std::unique_ptr param) - : Operator("VerifySSN", std::move(param)) { - m_re = new Regex(m_param); - } - - ~VerifySSN() { - delete m_re; - } + : Operator("VerifySSN", std::move(param)) + , m_re(std::make_unique(m_param)) { } bool operator=(const VerifySSN &a) = delete; VerifySSN(const VerifySSN &a) = delete; @@ -56,7 +51,7 @@ class VerifySSN : public Operator { static bool verify(const char *ssnumber, int len); static int convert_to_int(const char c); - Regex *m_re; + std::unique_ptr m_re; }; } // namespace operators diff --git a/src/operators/verify_svnr.cc b/src/operators/verify_svnr.cc index f869411d34..ce3147af99 100644 --- a/src/operators/verify_svnr.cc +++ b/src/operators/verify_svnr.cc @@ -24,7 +24,7 @@ int VerifySVNR::convert_to_int(const char c) { } -bool VerifySVNR::verify(const char *svnrnumber, int len) { +bool VerifySVNR::verify(const char *svnrnumber, int len) const { int var_len = len; int sum = 0; unsigned int i = 0, svnr_len = 10; @@ -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; // cppcheck-suppress uninitvar + 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 %= 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; // cppcheck-suppress knownConditionTrueFalse + return is_svnr; } for (i = 0; i < input.size() - 1 && is_svnr == false; i++) { diff --git a/src/operators/verify_svnr.h b/src/operators/verify_svnr.h index ba4358343e..ee693346d4 100644 --- a/src/operators/verify_svnr.h +++ b/src/operators/verify_svnr.h @@ -21,13 +21,8 @@ class VerifySVNR : public Operator { public: /** @ingroup ModSecurity_Operator */ explicit VerifySVNR(std::unique_ptr param) - : Operator("VerifySVNR", std::move(param)) { - m_re = new Regex(m_param); - } - - ~VerifySVNR() { - delete m_re; - } + : Operator("VerifySVNR", std::move(param)) + , m_re(std::make_unique(m_param)) { } bool operator=(const VerifySVNR &a) = delete; VerifySVNR(const VerifySVNR &a) = delete; @@ -36,10 +31,10 @@ class VerifySVNR : public Operator { const std::string& input, RuleMessage &ruleMessage) override; - bool verify(const char *ssnumber, int len); + bool verify(const char *ssnumber, int len) const; private: - Regex *m_re; + std::unique_ptr m_re; static int convert_to_int(const char c); const char bad_svnr[12][11] = { "0000000000", "0123456789", diff --git a/src/parser/driver.cc b/src/parser/driver.cc index a42942fbdf..b86f4c57f8 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -107,9 +107,9 @@ int Driver::addSecRule(std::unique_ptr r) { } for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { - Rules *rules = m_rulesSetPhases[i]; + const Rules *rules = m_rulesSetPhases[i]; for (int j = 0; j < rules->size(); j++) { - RuleWithOperator *lr = dynamic_cast(rules->at(j).get()); + const RuleWithOperator *lr = dynamic_cast(rules->at(j).get()); if (lr && lr->m_ruleId == rule->m_ruleId) { m_parserError << "Rule id: " << std::to_string(rule->m_ruleId) \ << " is duplicated" << std::endl; diff --git a/src/request_body_processor/json.cc b/src/request_body_processor/json.cc index 585976b48a..f56704effa 100644 --- a/src/request_body_processor/json.cc +++ b/src/request_body_processor/json.cc @@ -131,7 +131,7 @@ int JSON::addArgument(const std::string& value) { std::string path; for (size_t i = 0; i < m_containers.size(); i++) { - JSONContainerArray *a = dynamic_cast( + const JSONContainerArray *a = dynamic_cast( m_containers[i]); path = path + m_containers[i]->m_name; if (a != NULL) { diff --git a/src/request_body_processor/json.h b/src/request_body_processor/json.h index 19c469ee9f..961ea94ea8 100644 --- a/src/request_body_processor/json.h +++ b/src/request_body_processor/json.h @@ -79,7 +79,7 @@ class JSON { static int yajl_end_array(void *ctx); bool isPreviousArray() const { - JSONContainerArray *prev = NULL; + const JSONContainerArray *prev = NULL; if (m_containers.size() < 1) { return false; } diff --git a/src/request_body_processor/xml.h b/src/request_body_processor/xml.h index 1d29f62fea..fa8dc786a3 100644 --- a/src/request_body_processor/xml.h +++ b/src/request_body_processor/xml.h @@ -53,7 +53,7 @@ class XML { static xmlParserInputBufferPtr unloadExternalEntity(const char *URI, xmlCharEncoding enc); - static void null_error(void *ctx, const char *msg, ...) { + static void null_error(void *ctx, const char *msg, ...) { // cppcheck-suppress[constParameterPointer,constParameterCallback] } diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc index 373acf1ef3..f6642b67e6 100644 --- a/src/rule_with_actions.cc +++ b/src/rule_with_actions.cc @@ -102,9 +102,9 @@ RuleWithActions::RuleWithActions( delete a; } else if (auto sa = dynamic_cast(a)) { m_severity = sa; - } else if (auto lda = dynamic_cast(a)) { + } else if (auto lda = dynamic_cast(a)) { // cppcheck-suppress unreadVariable ; false positive m_logData = lda; - } else if (auto ma = dynamic_cast(a)) { + } else if (auto ma = dynamic_cast(a)) { // cppcheck-suppress unreadVariable ; false positive m_msg = ma; } else if (auto sva = dynamic_cast(a)) { m_actionsSetVar.push_back(sva); @@ -363,7 +363,7 @@ void RuleWithActions::executeTransformations( std::make_shared(path)); } - for (Action *a : m_transformations) { + for (const Action *a : m_transformations) { if (a->m_isNone) { none++; } @@ -457,7 +457,7 @@ bool RuleWithActions::containsMsg(const std::string& name, Transaction *t) { std::vector RuleWithActions::getActionsByName(const std::string& name, - Transaction *trans) { + const Transaction *trans) { std::vector ret; for (auto &z : m_actionsRuntimePos) { if (*z->m_name.get() == name) { diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index a3e35bf0ba..a7f7918a6f 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -176,7 +176,7 @@ inline void RuleWithOperator::getFinalVars(variables::Variables *vars, } if (std::find_if(trans->m_ruleRemoveTargetById.begin(), trans->m_ruleRemoveTargetById.end(), - [&, variable, this](std::pair &m) -> bool { + [&, variable, this](const auto &m) -> bool { return m.first == m_ruleId && m.second == *variable->m_fullName.get(); }) != trans->m_ruleRemoveTargetById.end()) { @@ -185,7 +185,7 @@ inline void RuleWithOperator::getFinalVars(variables::Variables *vars, if (std::find_if(trans->m_ruleRemoveTargetByTag.begin(), trans->m_ruleRemoveTargetByTag.end(), [&, variable, trans, this]( - std::pair &m) -> bool { + const auto &m) -> bool { return containsTag(m.first, trans) && m.second == *variable->m_fullName.get(); }) != trans->m_ruleRemoveTargetByTag.end()) { @@ -203,7 +203,6 @@ inline void RuleWithOperator::getFinalVars(variables::Variables *vars, bool RuleWithOperator::evaluate(Transaction *trans, RuleMessage &ruleMessage) { bool globalRet = false; - variables::Variables *variables = this->m_variables; bool recursiveGlobalRet; bool containsBlock = hasBlockAction(); std::string eparam; @@ -246,12 +245,12 @@ bool RuleWithOperator::evaluate(Transaction *trans, + "\" with param " \ + eparam \ + " against " \ - + variables + "."); + + m_variables + "."); } else { ms_dbg_a(trans, 4, "(Rule: " + std::to_string(m_ruleId) \ + ") Executing operator \"" + getOperatorName() \ + " against " \ - + variables + "."); + + m_variables + "."); } @@ -270,23 +269,23 @@ bool RuleWithOperator::evaluate(Transaction *trans, if (exclusion.contains(v) || std::find_if(trans->m_ruleRemoveTargetById.begin(), trans->m_ruleRemoveTargetById.end(), - [&, v, this](std::pair &m) -> bool { + [&, v, this](const auto &m) -> bool { return m.first == m_ruleId && m.second == v->getKeyWithCollection(); }) != trans->m_ruleRemoveTargetById.end() ) { delete v; - v = NULL; + v = nullptr; continue; } if (exclusion.contains(v) || std::find_if(trans->m_ruleRemoveTargetByTag.begin(), trans->m_ruleRemoveTargetByTag.end(), - [&, v, trans, this](std::pair &m) -> bool { + [&, v, trans, this](const auto &m) -> bool { return containsTag(m.first, trans) && m.second == v->getKeyWithCollection(); }) != trans->m_ruleRemoveTargetByTag.end() ) { delete v; - v = NULL; + v = nullptr; continue; } @@ -360,7 +359,7 @@ bool RuleWithOperator::evaluate(Transaction *trans, } -std::string RuleWithOperator::getOperatorName() const { return m_operator->m_op; } +const std::string& RuleWithOperator::getOperatorName() const { return m_operator->m_op; } } // namespace modsecurity diff --git a/src/rules_exceptions.cc b/src/rules_exceptions.cc index 71cf28c223..1545571c18 100644 --- a/src/rules_exceptions.cc +++ b/src/rules_exceptions.cc @@ -254,11 +254,11 @@ bool RulesExceptions::merge(RulesExceptions *from) { p.second)); } - for (auto &p : from->m_remove_rule_by_msg) { + for (const auto &p : from->m_remove_rule_by_msg) { m_remove_rule_by_msg.push_back(p); } - for (auto &p : from->m_remove_rule_by_tag) { + for (const auto &p : from->m_remove_rule_by_tag) { m_remove_rule_by_tag.push_back(p); } diff --git a/src/rules_set.cc b/src/rules_set.cc index 025abe69d9..19f31d8952 100644 --- a/src/rules_set.cc +++ b/src/rules_set.cc @@ -266,7 +266,7 @@ extern "C" RulesSet *msc_create_rules_set(void) { } -extern "C" void msc_rules_dump(RulesSet *rules) { +extern "C" void msc_rules_dump(const RulesSet *rules) { rules->dump(); } diff --git a/src/rules_set_properties.cc b/src/rules_set_properties.cc index 80078b3dd2..d65d9a95b0 100644 --- a/src/rules_set_properties.cc +++ b/src/rules_set_properties.cc @@ -30,13 +30,11 @@ void ConfigUnicodeMap::loadConfig(std::string f, double configCodePage, RulesSetProperties *driver, std::string *errg) { char *buf = NULL; char *hmap = NULL; - char *p = NULL; + const char *p = NULL; char *savedptr = NULL; - char *ucode = NULL; - int code = 0; + const char *ucode = NULL; int found = 0; int length = 0; - int Map = 0; int processing = 0; driver->m_unicodeMapTable.m_set = true; @@ -102,10 +100,10 @@ void ConfigUnicodeMap::loadConfig(std::string f, double configCodePage, if (mapping != NULL) { ucode = strtok_r(mapping, ":", &hmap); - sscanf(ucode, "%x", &code); // cppcheck-suppress invalidScanfArgType_int - sscanf(hmap, "%x", &Map); // cppcheck-suppress invalidScanfArgType_int + int code = strtol(ucode, nullptr, 16); + int map = strtol(hmap, nullptr, 16); if (code >= 0 && code <= 65535) { - driver->m_unicodeMapTable.m_unicodeMapTable->change(code, Map); + driver->m_unicodeMapTable.m_unicodeMapTable->change(code, map); } free(mapping); diff --git a/src/transaction.cc b/src/transaction.cc index f5ded52464..55e2dd3cf5 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -1175,7 +1175,7 @@ int Transaction::processResponseBody() { int Transaction::appendResponseBody(const unsigned char *buf, size_t len) { int current_size = this->m_responseBody.tellp(); - std::set &bi = \ + const std::set &bi = \ this->m_rules->m_responseBodyTypeToBeInspected.m_value; auto t = bi.find(m_variableResponseContentType.m_value); if (t == bi.end() && bi.empty() == false) { @@ -1677,7 +1677,7 @@ std::string Transaction::toJSON(int parts) { strlen("components")); yajl_gen_array_open(g); - for (auto a : m_rules->m_components) { + for (const auto &a : m_rules->m_components) { yajl_gen_string(g, reinterpret_cast (a.c_str()), a.length()); @@ -2197,7 +2197,7 @@ extern "C" void msc_intervention_cleanup(ModSecurityIntervention *it) { * @retval NULL Nothing was updated. * */ -extern "C" const char *msc_get_response_body(Transaction *transaction) { +extern "C" const char *msc_get_response_body(const Transaction *transaction) { return transaction->getResponseBody(); } diff --git a/src/utils/base64.cc b/src/utils/base64.cc index 7b1f4b07e6..e27cace943 100644 --- a/src/utils/base64.cc +++ b/src/utils/base64.cc @@ -24,7 +24,7 @@ #include "mbedtls/base64.h" template -inline std::string base64Helper(const char *data, const unsigned int len, Operation op) { +inline std::string base64Helper(const char *data, const unsigned int len, Operation op) { // cppcheck-suppress syntaxError ; false positive size_t out_len = 0; op(nullptr, 0, &out_len, diff --git a/src/utils/regex.cc b/src/utils/regex.cc index 5facd2b195..731ffc9795 100644 --- a/src/utils/regex.cc +++ b/src/utils/regex.cc @@ -154,7 +154,7 @@ std::list Regex::searchAll(const std::string& s) const { rc = pcre2_match(m_pc, pcre2_s, s.length(), offset, PCRE2_NO_JIT, match_data, NULL); } - PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(match_data); + const PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(match_data); #else const char *subject = s.c_str(); int ovector[OVECCOUNT]; @@ -207,7 +207,7 @@ RegexResult Regex::searchOneMatch(const std::string& s, std::vector } int rc = pcre2_match(m_pc, pcre2_s, s.length(), startOffset, pcre2_options, match_data, match_context); - PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(match_data); + const PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(match_data); #else const char *subject = s.c_str(); diff --git a/src/utils/sha1.h b/src/utils/sha1.h index 2dac3ea2c4..a40d7fa1c8 100644 --- a/src/utils/sha1.h +++ b/src/utils/sha1.h @@ -57,7 +57,7 @@ class DigestImpl { ConvertOp convertOp) -> auto { char digest[DigestSize]; - auto ret = digestOp(reinterpret_cast(input.c_str()), + const auto ret = (*digestOp)(reinterpret_cast(input.c_str()), input.size(), reinterpret_cast(digest)); assert(ret == 0); diff --git a/src/utils/string.h b/src/utils/string.h index f5794686bf..ca2967aa5f 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -257,7 +257,7 @@ inline std::string string_to_hex(std::string_view input) { template -inline std::string toCaseHelper(std::string str, Operation op) { +inline std::string toCaseHelper(std::string str, Operation op) { // cppcheck-suppress syntaxError ; false positive std::transform(str.begin(), str.end(), str.begin(), diff --git a/src/variables/env.cc b/src/variables/env.cc index bf40954a86..95f0f766b3 100644 --- a/src/variables/env.cc +++ b/src/variables/env.cc @@ -54,7 +54,7 @@ void Env::evaluate(Transaction *transaction, } const auto hasName = m_name.length() > 0; - for (auto& x : transaction->m_variableEnvs) { + for (const auto& x : transaction->m_variableEnvs) { #ifndef WIN32 if (hasName && x.first != m_name) { #else diff --git a/src/variables/variable.cc b/src/variables/variable.cc index 043b8ede72..caf8f6fd88 100644 --- a/src/variables/variable.cc +++ b/src/variables/variable.cc @@ -48,23 +48,23 @@ Variable::Variable(const std::string &name) } -Variable::Variable(Variable *var) : +Variable::Variable(const Variable *var) : m_name(var->m_name), m_collectionName(var->m_collectionName), m_fullName(var->m_fullName) { } -void Variable::addsKeyExclusion(Variable *v) { +void Variable::addsKeyExclusion(const Variable *v) { std::unique_ptr r; - VariableModificatorExclusion *ve = \ - dynamic_cast(v); - VariableRegex *vr; + const auto *ve = \ + dynamic_cast(v); + const VariableRegex *vr; if (!ve) { return; } - vr = dynamic_cast(ve->m_base.get()); + vr = dynamic_cast(ve->m_base.get()); if (vr == NULL) { r.reset(new KeyExclusionString(v->m_name)); @@ -76,12 +76,12 @@ void Variable::addsKeyExclusion(Variable *v) { } -std::string operator+(const std::string &a, Variable *v) { +std::string operator+(const std::string &a, const Variable *v) { return a + *v->m_fullName.get(); } -std::string operator+(const std::string &a, Variables *v) { +std::string operator+(const std::string &a, const Variables *v) { std::string test; for (const auto &b : *v) { if (test.empty()) { diff --git a/src/variables/variable.h b/src/variables/variable.h index 2d8c6ec025..5d740e1097 100644 --- a/src/variables/variable.h +++ b/src/variables/variable.h @@ -132,7 +132,7 @@ class KeyExclusionRegex : public KeyExclusion { class KeyExclusionString : public KeyExclusion { public: - explicit KeyExclusionString(std::string &a) + explicit KeyExclusionString(const std::string &a) : m_key(utils::string::toupper(a)) { } ~KeyExclusionString() override { } @@ -589,7 +589,7 @@ class VariableMonkeyResolution { class Variable : public VariableMonkeyResolution { public: explicit Variable(const std::string &name); - explicit Variable(Variable *_name); + explicit Variable(const Variable *_name); virtual ~Variable() { } @@ -608,7 +608,7 @@ class Variable : public VariableMonkeyResolution { } - void addsKeyExclusion(Variable *v); + void addsKeyExclusion(const Variable *v); bool operator==(const Variable& b) const { @@ -718,8 +718,8 @@ class VariableModificatorCount : public Variable { }; -std::string operator+(const std::string &a, modsecurity::variables::Variable *v); -std::string operator+(const std::string &a, modsecurity::variables::Variables *v); +std::string operator+(const std::string &a, const modsecurity::variables::Variable *v); +std::string operator+(const std::string &a, const modsecurity::variables::Variables *v); } // namespace variables diff --git a/test/benchmark/benchmark.cc b/test/benchmark/benchmark.cc index 8b36d368aa..a502150eaf 100644 --- a/test/benchmark/benchmark.cc +++ b/test/benchmark/benchmark.cc @@ -44,7 +44,7 @@ char rules_file[] = "basic_rules.conf"; const char* const help_message = "Usage: benchmark [num_iterations|-h|-?|--help]"; -int main(int argc, char *argv[]) { +int main(int argc, const char *argv[]) { unsigned long long NUM_REQUESTS(1000000); diff --git a/test/common/modsecurity_test.cc b/test/common/modsecurity_test.cc index 1e0585d475..21af285d55 100644 --- a/test/common/modsecurity_test.cc +++ b/test/common/modsecurity_test.cc @@ -89,7 +89,7 @@ template void ModSecurityTest::load_tests(const std::string &path) { DIR *dir; - struct dirent *ent; + const struct dirent *ent; struct stat buffer; if ((dir = opendir(path.c_str())) == nullptr) { diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index 81d47b1d67..2bdc15e85e 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -1,3 +1,5 @@ +normalCheckLevelMaxBranches:* + // // Ignore libinjection related stuff. // @@ -27,6 +29,8 @@ noExplicitConstructor:seclang-parser.hh constParameter:seclang-parser.hh accessMoved:seclang-parser.hh returnTempReference:seclang-parser.hh +duplInheritedMember:seclang-parser.hh +constVariableReference:seclang-parser.hh unreadVariable:src/operators/rx.cc unreadVariable:src/operators/rx_global.cc diff --git a/test/optimization/optimization.cc b/test/optimization/optimization.cc index 19298b0b9a..9affd29095 100644 --- a/test/optimization/optimization.cc +++ b/test/optimization/optimization.cc @@ -54,7 +54,7 @@ int main(int argc, char **argv) { } - for (auto &x : files) { + for (const auto &x : files) { std::cout << "Loading file: " << x << std::endl; if (modsecRules->loadFromUri(x.c_str()) < 0) { std::cout << "Not able to load the rules" << std::endl; @@ -96,8 +96,8 @@ int main(int argc, char **argv) { } } - if (auto rwo = dynamic_cast(z.get())) { - std::string op = rwo->getOperatorName(); + if (const auto *rwo = dynamic_cast(z.get())) { + const auto &op = rwo->getOperatorName(); if (operators.count(op) > 0) { operators[op] = 1 + operators[op]; } else { diff --git a/test/regression/regression.cc b/test/regression/regression.cc index 6d7b9dc3a1..f8acffb190 100644 --- a/test/regression/regression.cc +++ b/test/regression/regression.cc @@ -26,6 +26,7 @@ #include #include #include +#include #include "modsecurity/rules_set.h" #include "modsecurity/modsecurity.h" @@ -110,8 +111,8 @@ void actions(ModSecurityTestResults *r, } } -void perform_unit_test(ModSecurityTest *test, - std::vector> &tests, +void perform_unit_test(const ModSecurityTest &test, + const std::vector> &tests, ModSecurityTestResults *res, int *count) { for (auto &t : tests) { @@ -131,7 +132,7 @@ void perform_unit_test(ModSecurityTest *test, filename = t->filename; } - if (!test->m_automake_output) { + if (!test.m_automake_output) { std::cout << std::setw(3) << std::right << std::to_string(*count) << " "; std::cout << std::setw(50) << std::left << filename; @@ -139,7 +140,7 @@ void perform_unit_test(ModSecurityTest *test, } if (t->enabled == 0) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: SKIP" << filename \ << ":" << t->name << std::endl; } else { @@ -173,7 +174,7 @@ void perform_unit_test(ModSecurityTest *test, testRes->reason << KCYN << "compiled with support " << std::endl; testRes->reason << KCYN << "to: " << t->resource << std::endl; testRes->reason << RESET << std::endl; - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: SKIP " << filename \ << ":" << t->name << std::endl; } else { @@ -192,7 +193,7 @@ void perform_unit_test(ModSecurityTest *test, * Not expecting any error, thus return the error to * the user. */ - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << ":" << *count << std::endl; } else { @@ -213,7 +214,7 @@ void perform_unit_test(ModSecurityTest *test, const auto s = context.m_modsec_rules.getParserError(); if (regex_search(s, &match, re)) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: PASS " << filename \ << ":" << t->name << std::endl; } else { @@ -227,7 +228,7 @@ void perform_unit_test(ModSecurityTest *test, continue; } else { /* Parser error was expected, but with a different content */ - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << ":" << *count << std::endl; } else { @@ -249,7 +250,7 @@ void perform_unit_test(ModSecurityTest *test, } else { /* Parser error was expected but never happened */ if (t->parser_error.empty() == false) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << ":" << *count << std::endl; } else { @@ -318,7 +319,7 @@ void perform_unit_test(ModSecurityTest *test, const auto *d = static_cast(context.m_modsec_rules.m_debugLog); if (!d->contains(t->debug_log)) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << ":" << *count << std::endl; } else { @@ -330,7 +331,7 @@ void perform_unit_test(ModSecurityTest *test, << t->debug_log + ""; testRes->passed = false; } else if (r.status != t->http_code) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << ":" << *count << std::endl; } else { @@ -341,7 +342,7 @@ void perform_unit_test(ModSecurityTest *test, " got: " + std::to_string(r.status) + "\n"; testRes->passed = false; } else if (!contains(context.m_server_log.str(), t->error_log)) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << std::endl; } else { @@ -353,7 +354,7 @@ void perform_unit_test(ModSecurityTest *test, << t->error_log + ""; testRes->passed = false; } else if (!t->audit_log.empty() && !contains(getAuditLogContent(modsec_transaction.m_rules->m_auditLog->m_path1), t->audit_log)) { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: FAIL " << filename \ << ":" << t->name << ":" << *count << std::endl; } else { @@ -365,7 +366,7 @@ void perform_unit_test(ModSecurityTest *test, << t->audit_log + ""; testRes->passed = false; } else { - if (test->m_automake_output) { + if (test.m_automake_output) { std::cout << ":test-result: PASS " << filename \ << ":" << t->name << std::endl; } else { @@ -471,8 +472,8 @@ int main(int argc, char **argv) test_number++; if ((test.m_test_number == 0) || (test_number == test.m_test_number)) { - auto &tests = test[a]; - perform_unit_test(&test, tests, &res, &counter); + const auto &tests = test[a]; + perform_unit_test(test, tests, &res, &counter); } } diff --git a/test/unit/unit.cc b/test/unit/unit.cc index a1c6b0b3c6..8bf5954d27 100644 --- a/test/unit/unit.cc +++ b/test/unit/unit.cc @@ -92,7 +92,7 @@ struct TransformationTest { return tfn; } - static UnitTestResult eval(const ItemType &tfn, const UnitTest &t, modsecurity::Transaction &transaction) { + static UnitTestResult eval(const ItemType &tfn, const UnitTest &t, const modsecurity::Transaction &transaction) { auto ret = t.input; tfn.transform(ret, &transaction); return {1, ret}; @@ -105,7 +105,7 @@ struct TransformationTest { template -UnitTestResult perform_unit_test_once(const UnitTest &t, modsecurity::Transaction &transaction) { +UnitTestResult perform_unit_test_once(const UnitTest &t, modsecurity::Transaction &transaction) { // cppcheck-suppress constParameterReference ; transaction can be const for transformations but not for operators std::unique_ptr item(TestType::init(t)); assert(item.get() != nullptr); diff --git a/test/unit/unit_test.cc b/test/unit/unit_test.cc index b00607325c..d7c86c538e 100644 --- a/test/unit/unit_test.cc +++ b/test/unit/unit_test.cc @@ -75,7 +75,7 @@ void json2bin(std::string *str) { } -std::string UnitTest::print() { +std::string UnitTest::print() const { std::stringstream i; i << KRED << "Test failed." << RESET; diff --git a/test/unit/unit_test.h b/test/unit/unit_test.h index 81d99d1424..326a16c994 100644 --- a/test/unit/unit_test.h +++ b/test/unit/unit_test.h @@ -35,7 +35,7 @@ class UnitTest { public: static UnitTest *from_yajl_node(const yajl_val &); - std::string print(); + std::string print() const; std::string param; std::string input;