From fe98ce4c7dafcf2b4390ed825fab521da7f5773d Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Mon, 27 Jan 2020 18:11:08 -0300 Subject: [PATCH] Cosmetics: address cppcheck warnings --- Makefile.am | 20 +-- build/lua.m4 | 2 +- .../reading_logs_via_rule_message.h | 5 +- .../using_bodies_in_chunks/simple_request.cc | 1 - headers/modsecurity/anchored_set_variable.h | 2 +- headers/modsecurity/anchored_variable.h | 19 +-- headers/modsecurity/audit_log.h | 16 ++- headers/modsecurity/collection/collection.h | 2 +- headers/modsecurity/collection/collections.h | 3 + headers/modsecurity/debug_log.h | 10 +- headers/modsecurity/modsecurity.h | 9 +- headers/modsecurity/rule.h | 6 +- headers/modsecurity/rule_message.h | 12 +- headers/modsecurity/rules.h | 4 +- headers/modsecurity/rules_properties.h | 2 + headers/modsecurity/transaction.h | 6 +- headers/modsecurity/variable_value.h | 18 +-- src/anchored_set_variable.cc | 3 +- src/anchored_variable.cc | 2 +- src/audit_log/audit_log.cc | 10 +- src/audit_log/writer/parallel.h | 2 +- src/audit_log/writer/writer.h | 2 +- .../backend/in_memory-per_process.cc | 2 +- .../backend/in_memory-per_process.h | 2 +- src/collection/collections.cc | 7 +- src/debug_log/debug_log_writer.h | 6 +- src/engine/lua.cc | 4 +- src/engine/lua.h | 6 +- src/modsecurity.cc | 17 +-- src/operators/pm.cc | 2 +- src/operators/rbl.h | 2 +- src/parser/driver.cc | 1 - src/parser/driver.h | 2 + src/request_body_processor/json.cc | 12 +- src/request_body_processor/json.h | 10 +- src/request_body_processor/multipart.cc | 4 +- src/request_body_processor/multipart.h | 22 ++- src/request_body_processor/xml.cc | 6 +- src/rule.cc | 94 +++++++------ src/rule_script.h | 4 +- src/rules.cc | 10 +- src/rules_exceptions.cc | 4 +- src/run_time_string.cc | 2 +- src/run_time_string.h | 4 +- src/transaction.cc | 133 +++++++++++++----- src/unique_id.cc | 4 +- src/unique_id.h | 4 +- src/variables/variable.h | 5 + test/common/modsecurity_test.cc | 4 +- test/cppcheck_suppressions.txt | 102 ++++++-------- test/regression/regression_test.cc | 2 +- test/regression/regression_test.h | 2 +- test/unit/unit_test.cc | 2 +- 53 files changed, 358 insertions(+), 279 deletions(-) diff --git a/Makefile.am b/Makefile.am index a5f484e4..bf5b2026 100644 --- a/Makefile.am +++ b/Makefile.am @@ -56,15 +56,17 @@ parser: cppcheck: - @cppcheck -U YYSTYPE \ - --suppressions-list=./test/cppcheck_suppressions.txt \ - --enable=all \ - --inconclusive \ - --template="warning: {file},{line},{severity},{id},{message}" \ - -I headers -I . -I others -I src -I others/mbedtls -I src/parser \ - --error-exitcode=0 \ - -i "src/parser/seclang-parser.cc" -i "src/parser/seclang-scanner.cc" \ - . + @cppcheck -U YYSTYPE -U MBEDTLS_MD5_ALT -U MBEDTLS_SHA1_ALT \ + -D MS_CPPCHECK_DISABLED_FOR_PARSER \ + --suppressions-list=./test/cppcheck_suppressions.txt \ + --enable=all \ + --inconclusive \ + --template="warning: {file},{line},{severity},{id},{message}" \ + -I headers -I . -I others -I src -I others/mbedtls -I src/parser \ + --error-exitcode=1 \ + -i "src/parser/seclang-parser.cc" -i "src/parser/seclang-scanner.cc" \ + --force --verbose . + check-static: cppcheck diff --git a/build/lua.m4 b/build/lua.m4 index 49a54b48..b30b5bd5 100644 --- a/build/lua.m4 +++ b/build/lua.m4 @@ -6,7 +6,7 @@ AC_DEFUN([CHECK_LUA], [dnl # Possible names for the lua library/package (pkg-config) -LUA_POSSIBLE_LIB_NAMES="luajit luajit-5.1 lua53 lua5.3 lua-5.3 lua52 lua5.2 lua-5.2 lua51 lua5.1 lua-5.1 lua" +LUA_POSSIBLE_LIB_NAMES="lua53 lua5.3 lua-5.3 lua52 lua5.2 lua-5.2 lua51 lua5.1 lua-5.1 lua" # Possible extensions for the library LUA_POSSIBLE_EXTENSIONS="so so0 la sl dll dylib so.0.0.0" 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 52ce11ed..5c2252dc 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 @@ -115,7 +115,7 @@ class ReadingLogsViaRuleMessage { char *response_headers, char *response_body, char *ip, - std::string rules) : + const std::string &rules) : m_request_header(request_header), m_request_uri(request_uri), m_request_body(request_body), @@ -133,7 +133,6 @@ class ReadingLogsViaRuleMessage { modsecurity::ModSecurity *modsec; modsecurity::Rules *rules; - modsecurity::ModSecurityIntervention it; modsec = new modsecurity::ModSecurity(); modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha" \ @@ -168,8 +167,6 @@ class ReadingLogsViaRuleMessage { delete modsec; pthread_exit(NULL); return 0; -end: - return -1; } static void logCb(void *data, const void *ruleMessagev) { diff --git a/examples/using_bodies_in_chunks/simple_request.cc b/examples/using_bodies_in_chunks/simple_request.cc index 0a766bc2..5df6434c 100644 --- a/examples/using_bodies_in_chunks/simple_request.cc +++ b/examples/using_bodies_in_chunks/simple_request.cc @@ -126,7 +126,6 @@ int process_intervention(modsecurity::Transaction *transaction) { int main(int argc, char **argv) { modsecurity::ModSecurity *modsec; modsecurity::Rules *rules; - modsecurity::ModSecurityIntervention it; if (argc < 2) { std::cout << "Use " << *argv << " test-case-file.conf"; diff --git a/headers/modsecurity/anchored_set_variable.h b/headers/modsecurity/anchored_set_variable.h index f2020494..5c3d8fcd 100644 --- a/headers/modsecurity/anchored_set_variable.h +++ b/headers/modsecurity/anchored_set_variable.h @@ -71,7 +71,7 @@ struct MyHash{ class AnchoredSetVariable : public std::unordered_multimap { public: - AnchoredSetVariable(Transaction *t, std::string name); + AnchoredSetVariable(Transaction *t, const std::string &name); ~AnchoredSetVariable(); void unset(); diff --git a/headers/modsecurity/anchored_variable.h b/headers/modsecurity/anchored_variable.h index 071b9f19..4cf67ecc 100644 --- a/headers/modsecurity/anchored_variable.h +++ b/headers/modsecurity/anchored_variable.h @@ -42,15 +42,18 @@ class Transaction; class AnchoredVariable { public: - AnchoredVariable(Transaction* t, std::string name); + AnchoredVariable(Transaction* t, const std::string &name); - AnchoredVariable(const AnchoredVariable &a) { - m_transaction = a.m_transaction; - m_offset = a.m_offset; - m_name = a.m_name; - m_value = a.m_value; - m_var = a.m_var; - } + AnchoredVariable(const AnchoredVariable &a) = delete; + AnchoredVariable &operator= (const AnchoredVariable &a) = delete; + + /* + : m_transaction(a.m_transaction), + m_offset(a.m_offset), + m_name(a.m_name), + m_value(a.m_value), + m_var(a.m_var) { } + */ ~AnchoredVariable(); diff --git a/headers/modsecurity/audit_log.h b/headers/modsecurity/audit_log.h index 924739dd..54518959 100644 --- a/headers/modsecurity/audit_log.h +++ b/headers/modsecurity/audit_log.h @@ -37,7 +37,9 @@ class Writer; class AuditLog { public: AuditLog(); - ~AuditLog(); + virtual ~AuditLog(); + + AuditLog(const AuditLog &a) = delete; enum AuditLogType { NotSetAuditLogType, @@ -158,22 +160,22 @@ class AuditLog { bool setStorageDir(const std::basic_string& path); bool setFormat(AuditLogFormat fmt); - int getDirectoryPermission(); - int getFilePermission(); - int getParts(); + int getDirectoryPermission() const; + int getFilePermission() const; + int getParts() const; bool setParts(const std::basic_string& new_parts); bool setType(AuditLogType audit_type); bool init(std::string *error); - bool close(); + virtual bool close(); bool saveIfRelevant(Transaction *transaction); bool saveIfRelevant(Transaction *transaction, int parts); bool isRelevant(int status); - int addParts(int parts, const std::string& new_parts); - int removeParts(int parts, const std::string& new_parts); + static int addParts(int parts, const std::string& new_parts); + static int removeParts(int parts, const std::string& new_parts); bool merge(AuditLog *from, std::string *error); diff --git a/headers/modsecurity/collection/collection.h b/headers/modsecurity/collection/collection.h index 4c93495c..65e073a5 100644 --- a/headers/modsecurity/collection/collection.h +++ b/headers/modsecurity/collection/collection.h @@ -44,7 +44,7 @@ namespace collection { class Collection { public: - explicit Collection(std::string a) : m_name(a) { } + explicit Collection(const std::string &a) : m_name(a) { } virtual ~Collection() { } virtual void store(std::string key, std::string value) = 0; diff --git a/headers/modsecurity/collection/collections.h b/headers/modsecurity/collection/collections.h index 9961a04f..ed7ae3f1 100644 --- a/headers/modsecurity/collection/collections.h +++ b/headers/modsecurity/collection/collections.h @@ -49,6 +49,9 @@ class Collections { Collection *user, Collection *resource); ~Collections(); + Collections(const Collections &c) = delete; + Collections& operator =(const Collections &c) = delete; + std::string m_global_collection_key; std::string m_ip_collection_key; std::string m_session_collection_key; diff --git a/headers/modsecurity/debug_log.h b/headers/modsecurity/debug_log.h index c2501cf1..485aeb00 100644 --- a/headers/modsecurity/debug_log.h +++ b/headers/modsecurity/debug_log.h @@ -43,11 +43,11 @@ class DebugLog { virtual void write(int level, const std::string &msg); virtual void write(int level, const std::string &id, const std::string &uri, const std::string &msg); - bool isLogFileSet(); - bool isLogLevelSet(); - void setDebugLogLevel(int level); - void setDebugLogFile(const std::string &fileName, std::string *error); - const std::string& getDebugLogFile(); + virtual bool isLogFileSet(); + virtual bool isLogLevelSet(); + virtual void setDebugLogLevel(int level); + virtual void setDebugLogFile(const std::string &fileName, std::string *error); + virtual const std::string& getDebugLogFile(); virtual int getDebugLogLevel(); int m_debugLevel; diff --git a/headers/modsecurity/modsecurity.h b/headers/modsecurity/modsecurity.h index 4e4055be..d7abcc28 100644 --- a/headers/modsecurity/modsecurity.h +++ b/headers/modsecurity/modsecurity.h @@ -278,8 +278,11 @@ class ModSecurity { ModSecurity(); ~ModSecurity(); + ModSecurity(const ModSecurity &m) = delete; + ModSecurity& operator= (const ModSecurity &m) = delete; + const std::string& whoAmI(); - void setConnectorInformation(std::string connector); + void setConnectorInformation(const std::string &connector); void setServerLogCb(ModSecLogCb cb); /** * @@ -291,9 +294,9 @@ class ModSecurity { void serverLog(void *data, std::shared_ptr rm); - const std::string& getConnectorInformation(); + const std::string& getConnectorInformation() const; - int processContentOffset(const char *content, size_t len, + static int processContentOffset(const char *content, size_t len, const char *matchString, std::string *json, const char **err); collection::Collection *m_global_collection; diff --git a/headers/modsecurity/rule.h b/headers/modsecurity/rule.h index f9552e73..27bc6c51 100644 --- a/headers/modsecurity/rule.h +++ b/headers/modsecurity/rule.h @@ -74,7 +74,7 @@ class Rule { std::list, std::shared_ptr>> *ret, std::string *path, - int *nth); + int *nth) const; void getVariablesExceptions(Transaction *t, variables::Variables *exclusion, variables::Variables *addition); @@ -91,9 +91,9 @@ class Rule { std::string value, std::shared_ptr rm); void executeActionsIndependentOfChainedRuleResult(Transaction *trasn, bool *b, std::shared_ptr ruleMessage); - inline void updateMatchedVars(Transaction *trasn, const std::string &key, + static inline void updateMatchedVars(Transaction *trasn, const std::string &key, const std::string &value); - inline void cleanMatchedVars(Transaction *trasn); + static inline void cleanMatchedVars(Transaction *trasn); std::vector getActionsByName(const std::string& name, Transaction *t); diff --git a/headers/modsecurity/rule_message.h b/headers/modsecurity/rule_message.h index 2362a607..24ab615f 100644 --- a/headers/modsecurity/rule_message.h +++ b/headers/modsecurity/rule_message.h @@ -67,25 +67,25 @@ class RuleMessage { std::string log() { - return RuleMessage::log(this, 0); + return log(this, 0); } std::string log(int props) { - return RuleMessage::log(this, props); + return log(this, props); } std::string log(int props, int responseCode) { - return RuleMessage::log(this, props, responseCode); + return log(this, props, responseCode); } std::string errorLog() { - return RuleMessage::log(this, + return log(this, ClientLogMessageInfo | ErrorLogTailLogMessageInfo); } static std::string log(const RuleMessage *rm, int props, int code); static std::string log(const RuleMessage *rm, int props) { - return RuleMessage::log(rm, props, -1); + return log(rm, props, -1); } static std::string log(const RuleMessage *rm) { - return RuleMessage::log(rm, 0); + return log(rm, 0); } static std::string _details(const RuleMessage *rm); diff --git a/headers/modsecurity/rules.h b/headers/modsecurity/rules.h index d78b7708..5b7d4e10 100644 --- a/headers/modsecurity/rules.h +++ b/headers/modsecurity/rules.h @@ -70,7 +70,7 @@ class Rules : public RulesProperties { int load(const char *rules); int load(const char *rules, const std::string &ref); - void dump(); + void dump() const; int merge(Parser::Driver *driver); int merge(Rules *rules); @@ -84,10 +84,10 @@ class Rules : public RulesProperties { int64_t unicode_codepage; private: - int m_referenceCount; #ifndef NO_LOGS uint8_t m_secmarker_skipped; #endif + int m_referenceCount; }; #endif diff --git a/headers/modsecurity/rules_properties.h b/headers/modsecurity/rules_properties.h index c43578fa..b815eafb 100644 --- a/headers/modsecurity/rules_properties.h +++ b/headers/modsecurity/rules_properties.h @@ -197,6 +197,8 @@ class RulesProperties { m_remoteRulesActionOnFailed(PropertyNotSetRemoteRulesAction), m_secRuleEngine(PropertyNotSetRuleEngine) { } + RulesProperties(const RulesProperties &r) = delete; + RulesProperties &operator =(const RulesProperties &r) = delete; ~RulesProperties() { int i = 0; diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index b6161aba..4019ea5a 100644 --- a/headers/modsecurity/transaction.h +++ b/headers/modsecurity/transaction.h @@ -295,7 +295,7 @@ class Transaction : public TransactionAnchoredVariables { Transaction ( const Transaction & ) = delete; bool operator ==(const Transaction &b) const { return false; }; - Transaction operator =(const Transaction &b) const = delete; + Transaction &operator =(const Transaction &b) const = delete; /** TODO: Should be an structure that fits an IP address */ int processConnection(const char *client, int cPort, @@ -359,7 +359,7 @@ class Transaction : public TransactionAnchoredVariables { bool extractArguments(const std::string &orig, const std::string& buf, size_t offset); - const char *getResponseBody(); + const char *getResponseBody() const; size_t getResponseBodyLength(); size_t getRequestBodyLength(); @@ -368,7 +368,7 @@ class Transaction : public TransactionAnchoredVariables { #endif void serverLog(std::shared_ptr rm); - int getRuleEngineState(); + int getRuleEngineState() const; std::string toJSON(int parts); std::string toOldAuditLogFormat(int parts, const std::string &trailer); diff --git a/headers/modsecurity/variable_value.h b/headers/modsecurity/variable_value.h index 7c36170b..ca9c3f33 100644 --- a/headers/modsecurity/variable_value.h +++ b/headers/modsecurity/variable_value.h @@ -39,28 +39,28 @@ class VariableValue { public: using Origins = std::list>; - VariableValue(const std::string *key, + explicit VariableValue(const std::string *key, const std::string *value = nullptr) - : m_key(*key), + : m_collection(""), + m_key(*key), m_keyWithCollection(*key), - m_collection(""), m_value(value != nullptr?*value:"") { } VariableValue(const std::string *collection, const std::string *key, const std::string *value) - : m_key(*key), + : m_collection(*collection), + m_key(*key), m_keyWithCollection(*collection + ":" + *key), - m_collection(*collection), m_value(*value) { } explicit VariableValue(const VariableValue *o) : - m_key(o->m_key), - m_value(o->m_value), m_collection(o->m_collection), - m_keyWithCollection(o->m_keyWithCollection) + m_key(o->m_key), + m_keyWithCollection(o->m_keyWithCollection), + m_value(o->m_value) { for (auto &i : o->m_orign) { std::unique_ptr origin(new VariableOrigin()); @@ -70,6 +70,8 @@ class VariableValue { } } + VariableValue(const VariableValue &v) = delete; + const std::string& getKey() const { return m_key; diff --git a/src/anchored_set_variable.cc b/src/anchored_set_variable.cc index 622d977c..e05a16a0 100644 --- a/src/anchored_set_variable.cc +++ b/src/anchored_set_variable.cc @@ -28,7 +28,8 @@ namespace modsecurity { -AnchoredSetVariable::AnchoredSetVariable(Transaction *t, std::string name) +AnchoredSetVariable::AnchoredSetVariable(Transaction *t, + const std::string &name) : m_transaction(t), m_name(name) { reserve(10); diff --git a/src/anchored_variable.cc b/src/anchored_variable.cc index 1f834e01..9908dcc6 100644 --- a/src/anchored_variable.cc +++ b/src/anchored_variable.cc @@ -28,7 +28,7 @@ namespace modsecurity { AnchoredVariable::AnchoredVariable(Transaction *t, - std::string name) + const std::string &name) : m_transaction(t), m_offset(0), m_name(""), diff --git a/src/audit_log/audit_log.cc b/src/audit_log/audit_log.cc index fb7fe625..ac6fd086 100644 --- a/src/audit_log/audit_log.cc +++ b/src/audit_log/audit_log.cc @@ -54,12 +54,12 @@ AuditLog::AuditLog() : m_path1(""), m_path2(""), m_storage_dir(""), + m_format(NotSetAuditLogFormat), + m_parts(-1), m_filePermission(-1), m_directoryPermission(-1), - m_parts(-1), m_status(NotSetLogStatus), m_type(NotSetAuditLogType), - m_format(NotSetAuditLogFormat), m_relevant(""), m_writer(NULL), m_refereceCount(1) { } @@ -85,7 +85,7 @@ bool AuditLog::setFileMode(int permission) { } -int AuditLog::getFilePermission() { +int AuditLog::getFilePermission() const { if (m_filePermission == -1) { return m_defaultFilePermission; } @@ -93,7 +93,7 @@ int AuditLog::getFilePermission() { return m_filePermission; } -int AuditLog::getDirectoryPermission() { +int AuditLog::getDirectoryPermission() const { if (m_directoryPermission == -1) { return m_defaultDirectoryPermission; } @@ -192,7 +192,7 @@ bool AuditLog::setParts(const std::basic_string& new_parts) { } -int AuditLog::getParts() { +int AuditLog::getParts() const { if (m_parts == -1) { return m_defaultParts; } diff --git a/src/audit_log/writer/parallel.h b/src/audit_log/writer/parallel.h index 3a24a559..d5d8951b 100644 --- a/src/audit_log/writer/parallel.h +++ b/src/audit_log/writer/parallel.h @@ -65,7 +65,7 @@ class Parallel : public Writer { YearMonthDayAndTimeFileName = 8, }; - inline std::string logFilePath(time_t *t, int part); + static inline std::string logFilePath(time_t *t, int part); }; } // namespace writer diff --git a/src/audit_log/writer/writer.h b/src/audit_log/writer/writer.h index a83c7e28..1b1ddd4a 100644 --- a/src/audit_log/writer/writer.h +++ b/src/audit_log/writer/writer.h @@ -51,7 +51,7 @@ class Writer { virtual bool write(Transaction *transaction, int parts, std::string *error) = 0; - void generateBoundary(std::string *boundary); + static void generateBoundary(std::string *boundary); void refCountIncrease() { m_refereceCount++; diff --git a/src/collection/backend/in_memory-per_process.cc b/src/collection/backend/in_memory-per_process.cc index a3796486..4f3710ec 100644 --- a/src/collection/backend/in_memory-per_process.cc +++ b/src/collection/backend/in_memory-per_process.cc @@ -36,7 +36,7 @@ namespace collection { namespace backend { -InMemoryPerProcess::InMemoryPerProcess(std::string name) : +InMemoryPerProcess::InMemoryPerProcess(const std::string &name) : Collection(name) { this->reserve(1000); pthread_mutex_init(&m_lock, NULL); diff --git a/src/collection/backend/in_memory-per_process.h b/src/collection/backend/in_memory-per_process.h index 98b6f7b9..03f822cf 100644 --- a/src/collection/backend/in_memory-per_process.h +++ b/src/collection/backend/in_memory-per_process.h @@ -72,7 +72,7 @@ class InMemoryPerProcess : /*std::hash*/MyHash, MyEqual>, public Collection { public: - explicit InMemoryPerProcess(std::string name); + explicit InMemoryPerProcess(const std::string &name); ~InMemoryPerProcess(); void store(std::string key, std::string value) override; diff --git a/src/collection/collections.cc b/src/collection/collections.cc index c03d0d35..b6ed9d4c 100644 --- a/src/collection/collections.cc +++ b/src/collection/collections.cc @@ -36,14 +36,17 @@ namespace collection { Collections::Collections(Collection *global, Collection *ip, Collection *session, Collection *user, - Collection *resource) : m_global_collection_key(""), + Collection *resource) + : m_global_collection_key(""), m_ip_collection_key(""), + m_session_collection_key(""), + m_user_collection_key(""), m_resource_collection_key(""), m_global_collection(global), - m_resource_collection(resource), m_ip_collection(ip), m_session_collection(session), m_user_collection(user), + m_resource_collection(resource), m_tx_collection(new backend::InMemoryPerProcess("TX")) { } diff --git a/src/debug_log/debug_log_writer.h b/src/debug_log/debug_log_writer.h index 0fdba01b..17447307 100644 --- a/src/debug_log/debug_log_writer.h +++ b/src/debug_log/debug_log_writer.h @@ -40,9 +40,9 @@ class DebugLogWriter { return instance; } - void write_log(const std::string& file, const std::string& msg); - void close(const std::string& m_fileName); - int open(const std::string& m_fileName, std::string *error); + static void write_log(const std::string& file, const std::string& msg); + static void close(const std::string& m_fileName); + static int open(const std::string& m_fileName, std::string *error); private: DebugLogWriter() : m_first(NULL) { } diff --git a/src/engine/lua.cc b/src/engine/lua.cc index 12f1ba45..8735dbf9 100644 --- a/src/engine/lua.cc +++ b/src/engine/lua.cc @@ -39,7 +39,7 @@ namespace modsecurity { namespace engine { -bool Lua::isCompatible(std::string script, Lua *l, std::string *error) { +bool Lua::isCompatible(const std::string &script, Lua *l, std::string *error) { #ifdef WITH_LUA std::string lua(".lua"); std::string err; @@ -63,7 +63,7 @@ bool Lua::isCompatible(std::string script, Lua *l, std::string *error) { } -bool Lua::load(std::string script, std::string *err) { +bool Lua::load(const std::string &script, std::string *err) { #ifdef WITH_LUA lua_State *L = NULL; L = luaL_newstate(); diff --git a/src/engine/lua.h b/src/engine/lua.h index 9a0c0c47..b05a99bc 100644 --- a/src/engine/lua.h +++ b/src/engine/lua.h @@ -53,7 +53,7 @@ class LuaScriptBlob { } - const char *read(size_t *len) { + const char *read(size_t *len) const { *len = m_len; return (const char *)m_data; } @@ -68,9 +68,9 @@ class Lua { public: Lua() { } - bool load(std::string script, std::string *err); + bool load(const std::string &script, std::string *err); int run(Transaction *t, const std::string &str=""); - static bool isCompatible(std::string script, Lua *l, std::string *error); + static bool isCompatible(const std::string &script, Lua *l, std::string *error); #ifdef WITH_LUA static int blob_keeper(lua_State *L, const void *p, size_t sz, void *ud); diff --git a/src/modsecurity.cc b/src/modsecurity.cc index ab9bba2e..52c765df 100644 --- a/src/modsecurity.cc +++ b/src/modsecurity.cc @@ -60,8 +60,7 @@ namespace modsecurity { * @endcode */ ModSecurity::ModSecurity() - : m_connector(""), - m_whoami(""), + : #ifdef WITH_LMDB m_global_collection(new collection::backend::LMDB("GLOBAL")), m_resource_collection(new collection::backend::LMDB("RESOURCE")), @@ -70,14 +69,17 @@ ModSecurity::ModSecurity() m_user_collection(new collection::backend::LMDB("USER")), #else m_global_collection(new collection::backend::InMemoryPerProcess("GLOBAL")), - m_ip_collection(new collection::backend::InMemoryPerProcess("IP")), m_resource_collection( new collection::backend::InMemoryPerProcess("RESOURCE")), + m_ip_collection(new collection::backend::InMemoryPerProcess("IP")), m_session_collection( new collection::backend::InMemoryPerProcess("SESSION")), m_user_collection(new collection::backend::InMemoryPerProcess("USER")), #endif - m_logCb(NULL) { + m_connector(""), + m_whoami(""), + m_logCb(NULL), + m_logProperties(0) { UniqueId::uniqueId(); srand(time(NULL)); #ifdef MSC_WITH_CURL @@ -167,7 +169,7 @@ const std::string& ModSecurity::whoAmI() { * @param connector Information about the connector. * */ -void ModSecurity::setConnectorInformation(std::string connector) { +void ModSecurity::setConnectorInformation(const std::string &connector) { m_connector = connector; } @@ -182,7 +184,7 @@ void ModSecurity::setConnectorInformation(std::string connector) { * @retval "" Nothing was informed about the connector. * @retval !="" Connector information. */ -const std::string& ModSecurity::getConnectorInformation() { +const std::string& ModSecurity::getConnectorInformation() const { return m_connector; } @@ -224,7 +226,6 @@ int ModSecurity::processContentOffset(const char *content, size_t len, Utils::Regex transformations("t:(?:(?!t:).)+"); yajl_gen g; std::string varValue; - std::string opValue; const unsigned char *buf; size_t jsonSize; @@ -391,11 +392,11 @@ int ModSecurity::processContentOffset(const char *content, size_t len, json->append("\n"); yajl_gen_free(g); + return 0; #else *err = "Without YAJL support, we cannot generate JSON."; return -1; #endif - return 0; } diff --git a/src/operators/pm.cc b/src/operators/pm.cc index 86bac240..06a15da5 100644 --- a/src/operators/pm.cc +++ b/src/operators/pm.cc @@ -83,7 +83,7 @@ void Pm::postOrderTraversal(acmp_btree_node_t *node) { bool Pm::evaluate(Transaction *transaction, Rule *rule, const std::string &input, std::shared_ptr ruleMessage) { - int rc = -1; + int rc; ACMPT pt; pt.parser = m_p; pt.ptr = NULL; diff --git a/src/operators/rbl.h b/src/operators/rbl.h index 26a50b77..638b560f 100644 --- a/src/operators/rbl.h +++ b/src/operators/rbl.h @@ -62,7 +62,7 @@ class Rbl : public Operator { /** @ingroup ModSecurity_Operator */ explicit Rbl(std::unique_ptr param) - : m_service(""), + : m_service(), m_demandsPassword(false), m_provider(RblProvider::UnknownProvider), Operator("Rbl", std::move(param)) { diff --git a/src/parser/driver.cc b/src/parser/driver.cc index f9a729a0..e2c14064 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -15,7 +15,6 @@ #include "src/parser/driver.h" -#include "src/parser/seclang-parser.hh" #include "modsecurity/audit_log.h" #include "modsecurity/rules_properties.h" diff --git a/src/parser/driver.h b/src/parser/driver.h index 32d51fee..6d522911 100644 --- a/src/parser/driver.h +++ b/src/parser/driver.h @@ -30,7 +30,9 @@ #include "modsecurity/rules_properties.h" #include "modsecurity/audit_log.h" #include "src/rule_script.h" +#ifndef MS_CPPCHECK_DISABLED_FOR_PARSER #include "src/parser/seclang-parser.hh" +#endif using modsecurity::Rule; using modsecurity::Rules; diff --git a/src/request_body_processor/json.cc b/src/request_body_processor/json.cc index a6dd4ad4..194bc0c0 100644 --- a/src/request_body_processor/json.cc +++ b/src/request_body_processor/json.cc @@ -237,10 +237,10 @@ int JSON::yajl_end_array(void *ctx) { tthis->m_containers.pop_back(); delete a; if (tthis->m_containers.size() > 0) { - JSONContainerArray *a = dynamic_cast( + JSONContainerArray *ja = dynamic_cast( tthis->m_containers.back()); - if (a) { - a->m_elementCounter++; + if (ja) { + ja->m_elementCounter++; } } @@ -272,10 +272,10 @@ int JSON::yajl_end_map(void *ctx) { delete a; if (tthis->m_containers.size() > 0) { - JSONContainerArray *a = dynamic_cast( + JSONContainerArray *ja = dynamic_cast( tthis->m_containers.back()); - if (a) { - a->m_elementCounter++; + if (ja) { + ja->m_elementCounter++; } } diff --git a/src/request_body_processor/json.h b/src/request_body_processor/json.h index 8c4c502d..42d714d0 100644 --- a/src/request_body_processor/json.h +++ b/src/request_body_processor/json.h @@ -36,7 +36,7 @@ namespace RequestBodyProcessor { class JSONContainer { public: - explicit JSONContainer(std::string name) : m_name(name) { } + explicit JSONContainer(const std::string &name) : m_name(name) { } virtual ~JSONContainer() { } std::string m_name; }; @@ -44,7 +44,7 @@ class JSONContainer { class JSONContainerArray : public JSONContainer { public: - explicit JSONContainerArray(std::string name) : JSONContainer(name), + explicit JSONContainerArray(const std::string &name) : JSONContainer(name), m_elementCounter(0) { } size_t m_elementCounter; }; @@ -52,7 +52,7 @@ class JSONContainerArray : public JSONContainer { class JSONContainerMap : public JSONContainer { public: - explicit JSONContainerMap(std::string name) : JSONContainer(name) { } + explicit JSONContainerMap(const std::string &name) : JSONContainer(name) { } }; @@ -61,7 +61,7 @@ class JSON { explicit JSON(Transaction *transaction); ~JSON(); - bool init(); + static bool init(); bool processChunk(const char *buf, unsigned int size, std::string *err); bool complete(std::string *err); @@ -79,7 +79,7 @@ class JSON { static int yajl_start_array(void *ctx); static int yajl_end_array(void *ctx); - bool isPreviousArray() { + bool isPreviousArray() const { JSONContainerArray *prev = NULL; if (m_containers.size() < 1) { return false; diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index b38d0986..a6d3373d 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -36,7 +36,7 @@ namespace modsecurity { namespace RequestBodyProcessor { -Multipart::Multipart(std:: string header, Transaction *transaction) +Multipart::Multipart(const std::string &header, Transaction *transaction) : m_reqbody_no_files_length(0), m_nfiles(0), m_boundary_count(0), @@ -1277,7 +1277,7 @@ bool Multipart::init(std::string *error) { /* Quoted. */ m_boundary.assign(std::string(b + 1, len - 2)); if (m_boundary.empty()) { - return -1; + return false; } m_flag_boundary_quoted = 1; } else { diff --git a/src/request_body_processor/multipart.h b/src/request_body_processor/multipart.h index 896d11b7..d05b2fe6 100644 --- a/src/request_body_processor/multipart.h +++ b/src/request_body_processor/multipart.h @@ -58,11 +58,19 @@ class MultipartPart { public: MultipartPart() : m_type(MULTIPART_FORMDATA), - m_tmp_file_fd(0), - m_offset(0), - m_filenameOffset(0), + m_name(""), m_nameOffset(0), + m_value(""), m_valueOffset(0), + m_value_parts(), + m_tmp_file_name(""), + m_tmp_file_fd(0), + m_tmp_file_size(), + m_filename(""), + m_filenameOffset(0), + m_last_header_name(""), + m_headers(), + m_offset(0), m_length(0) { m_tmp_file_size.first = 0; m_tmp_file_size.second = 0; @@ -109,14 +117,14 @@ class MultipartPart { class Multipart { public: - Multipart(std::string header, Transaction *transaction); + Multipart(const std::string &header, Transaction *transaction); ~Multipart(); bool init(std::string *err); - int boundary_characters_valid(const char *boundary); - int count_boundary_params(const std::string& str_header_value); - int is_token_char(unsigned char c); + static int boundary_characters_valid(const char *boundary); + static int count_boundary_params(const std::string& str_header_value); + static int is_token_char(unsigned char c); int multipart_complete(std::string *err); int parse_content_disposition(const char *c_d_value, int offset); diff --git a/src/request_body_processor/xml.cc b/src/request_body_processor/xml.cc index 9483a0f4..db92922c 100644 --- a/src/request_body_processor/xml.cc +++ b/src/request_body_processor/xml.cc @@ -46,13 +46,13 @@ XML::~XML() { bool XML::init() { - xmlParserInputBufferCreateFilenameFunc entity; + //xmlParserInputBufferCreateFilenameFunc entity; if (m_transaction->m_rules->m_secXMLExternalEntity == RulesProperties::TrueConfigBoolean) { - entity = xmlParserInputBufferCreateFilenameDefault( + /*entity = */xmlParserInputBufferCreateFilenameDefault( __xmlParserInputBufferCreateFilename); } else { - entity = xmlParserInputBufferCreateFilenameDefault( + /*entity = */xmlParserInputBufferCreateFilenameDefault( this->unloadExternalEntity); } diff --git a/src/rule.cc b/src/rule.cc index 561ef537..51de9d7d 100644 --- a/src/rule.cc +++ b/src/rule.cc @@ -52,33 +52,34 @@ using actions::transformations::None; Rule::Rule(const std::string &marker) - : m_accuracy(0), + : m_theDisruptiveAction(nullptr), + m_logData(nullptr), + m_msg(nullptr), + m_severity(nullptr), + m_chained(false), + m_containsCaptureAction(false), + m_containsMultiMatchAction(false), + m_containsStaticBlockAction(false), + m_secMarker(true), + m_ruleId(0), + m_accuracy(0), + m_lineNumber(0), + m_maturity(0), + m_phase(-1), + m_variables(NULL), + m_op(NULL), + m_chainedRuleChild(NULL), + m_chainedRuleParent(NULL), + m_fileName(""), + m_marker(marker), + m_rev(""), + m_ver(""), m_actionsRuntimePos(), m_actionsRuntimePre(), m_actionsSetVar(), m_actionsTag(), - m_chained(false), - m_chainedRuleChild(NULL), - m_fileName(""), - m_lineNumber(0), - m_marker(marker), - m_maturity(0), - m_op(NULL), - m_phase(-1), - m_rev(""), - m_ruleId(0), - m_secMarker(true), - m_variables(NULL), - m_ver(""), m_unconditional(false), - m_referenceCount(1), - m_theDisruptiveAction(nullptr), - m_containsStaticBlockAction(false), - m_containsCaptureAction(false), - m_containsMultiMatchAction(false), - m_severity(nullptr), - m_logData(nullptr), - m_msg(nullptr) { } + m_referenceCount(1) { } Rule::Rule(Operator *_op, @@ -86,34 +87,35 @@ Rule::Rule(Operator *_op, std::vector *actions, std::string fileName, int lineNumber) - : m_accuracy(0), + : m_theDisruptiveAction(nullptr), + m_logData(nullptr), + m_msg(nullptr), + m_severity(nullptr), + m_chained(false), + m_containsCaptureAction(false), + m_containsMultiMatchAction(false), + m_containsStaticBlockAction(false), + m_secMarker(false), + m_ruleId(0), + m_accuracy(0), + m_lineNumber(lineNumber), + m_maturity(0), + m_phase(-1), + m_variables(_variables), + m_op(_op), + m_chainedRuleChild(NULL), + m_chainedRuleParent(NULL), + m_fileName(fileName), + m_marker(""), + m_rev(""), + m_ver(""), m_actionsRuntimePos(), m_actionsRuntimePre(), m_actionsSetVar(), m_actionsTag(), - m_chained(false), - m_chainedRuleChild(NULL), - m_chainedRuleParent(NULL), - m_fileName(fileName), - m_lineNumber(lineNumber), - m_marker(""), - m_maturity(0), - m_op(_op), - m_phase(-1), - m_rev(""), - m_ruleId(0), - m_secMarker(false), - m_variables(_variables), - m_ver(""), m_unconditional(false), - m_referenceCount(1), - m_theDisruptiveAction(nullptr), - m_containsStaticBlockAction(false), - m_containsCaptureAction(false), - m_containsMultiMatchAction(false), - m_severity(nullptr), - m_logData(nullptr), - m_msg(nullptr) { + m_referenceCount(1) + { /* */ organizeActions(actions); @@ -338,7 +340,7 @@ inline void Rule::executeTransformation(actions::Action *a, std::list, std::shared_ptr>> *ret, std::string *path, - int *nth) { + int *nth) const { std::string *oldValue = (*value).get(); std::string newValue = a->evaluate(*oldValue, trans); diff --git a/src/rule_script.h b/src/rule_script.h index 9b446632..db3f8a79 100644 --- a/src/rule_script.h +++ b/src/rule_script.h @@ -44,9 +44,9 @@ using actions::Action; /** @ingroup ModSecurity_CPP_API */ class RuleScript : public Rule { public: - RuleScript(std::string name, + RuleScript(const std::string &name, std::vector *actions, - std::string fileName, + const std::string &fileName, int lineNumber) : Rule(NULL, NULL, actions, fileName, lineNumber), m_name(name) { } diff --git a/src/rules.cc b/src/rules.cc index eff766f9..6d4d94c0 100644 --- a/src/rules.cc +++ b/src/rules.cc @@ -173,9 +173,9 @@ int Rules::evaluate(int phase, Transaction *t) { "through the utilization of an `allow' action."); return true; } - if (t->m_allowType != actions::disruptive::NoneAllowType) { - t->m_allowType = actions::disruptive::NoneAllowType; - } + //if (t->m_allowType != actions::disruptive::NoneAllowType) { + t->m_allowType = actions::disruptive::NoneAllowType; + //} for (int i = 0; i < rules.size(); i++) { Rule *rule = rules[i]; @@ -255,7 +255,7 @@ int Rules::evaluate(int phase, Transaction *t) { } rule->evaluate(t, NULL); - if (t->m_it.disruptive == true) { + if (t->m_it.disruptive > 0) { ms_dbg_a(t, 8, "Skipping this phase as this " \ "request was already intercepted."); break; @@ -296,7 +296,7 @@ void Rules::debug(int level, const std::string &id, } -void Rules::dump() { +void Rules::dump() const { std::cout << "Rules: " << std::endl; for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { std::vector rules = m_rules[i]; diff --git a/src/rules_exceptions.cc b/src/rules_exceptions.cc index dd057402..c7b54b8f 100644 --- a/src/rules_exceptions.cc +++ b/src/rules_exceptions.cc @@ -122,8 +122,8 @@ bool RulesExceptions::loadUpdateTargetById(double id, bool RulesExceptions::load(const std::string &a, std::string *error) { bool added = false; std::vector toRemove = utils::string::ssplit(a, ' '); - for (std::string &a : toRemove) { - std::string b = modsecurity::utils::string::parserSanitizer(a); + for (std::string &r : toRemove) { + std::string b = modsecurity::utils::string::parserSanitizer(r); if (b.size() == 0) { continue; } diff --git a/src/run_time_string.cc b/src/run_time_string.cc index 9d60cfc4..7c64e8ba 100644 --- a/src/run_time_string.cc +++ b/src/run_time_string.cc @@ -30,7 +30,7 @@ namespace modsecurity { -void RunTimeString::appendText(std::string text) { +void RunTimeString::appendText(const std::string &text) { std::unique_ptr r(new RunTimeElementHolder); r->m_string = text; m_elements.push_back(std::move(r)); diff --git a/src/run_time_string.h b/src/run_time_string.h index 20535c9a..96b511bd 100644 --- a/src/run_time_string.h +++ b/src/run_time_string.h @@ -46,14 +46,14 @@ class RunTimeString { public: RunTimeString() : m_containsMacro(false) { } - void appendText(std::string text); + void appendText(const std::string &text); void appendVar(std::unique_ptr var); std::string evaluate(Transaction *t); std::string evaluate(Transaction *t, Rule *r); std::string evaluate() { return evaluate(NULL); } - inline bool containsMacro() { return m_containsMacro; } + inline bool containsMacro() const { return m_containsMacro; } bool m_containsMacro; protected: diff --git a/src/transaction.cc b/src/transaction.cc index 179a60d4..3b07b0eb 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -100,37 +100,67 @@ namespace modsecurity { * */ Transaction::Transaction(ModSecurity *ms, Rules *rules, void *logCbData) - : m_clientPort(0), - m_serverPort(0), + : m_creationTimeStamp(utils::cpu_seconds()), + m_clientIpAddress(""), + m_httpVersion(""), + m_serverIpAddress(""), + m_uri(""), m_uri_no_query_string_decoded(""), - m_rules(rules), - m_timeStamp(std::time(NULL)), - m_httpCodeReturned(200), - m_highestSeverityAction(255), m_ARGScombinedSizeDouble(0), + m_clientPort(0), + m_highestSeverityAction(255), + m_httpCodeReturned(200), + m_serverPort(0), + m_ms(ms), m_requestBodyType(UnknownFormat), m_requestBodyProcessor(UnknownFormat), + m_rules(rules), + m_ruleRemoveById(), + m_ruleRemoveByIdRange(), + m_ruleRemoveByTag(), + m_ruleRemoveTargetByTag(), + m_ruleRemoveTargetById(), m_requestBodyAccess(Rules::PropertyNotSetConfigBoolean), + m_auditLogModifier(), + m_rulesMessages(), + m_requestBody(), + m_responseBody(), + m_id(), m_marker(""), - m_allowType(modsecurity::actions::disruptive::NoneAllowType), m_skip_next(0), - m_creationTimeStamp(utils::cpu_seconds()), - m_logCbData(logCbData), - m_ms(ms), - m_secRuleEngine(RulesProperties::PropertyNotSetRuleEngine), + m_allowType(modsecurity::actions::disruptive::NoneAllowType), + m_uri_decoded(""), + m_actions(), + m_it(), + m_timeStamp(std::time(NULL)), m_collections(ms->m_global_collection, ms->m_ip_collection, ms->m_session_collection, ms->m_user_collection, ms->m_resource_collection), -#ifdef WITH_YAJL - m_json(new RequestBodyProcessor::JSON(this)), -#else - m_json(NULL), -#endif + m_matched(), #ifdef WITH_LIBXML2 m_xml(new RequestBodyProcessor::XML(this)), #else m_xml(NULL), #endif +#ifdef WITH_YAJL + m_json(new RequestBodyProcessor::JSON(this)), +#else + m_json(NULL), +#endif + m_secRuleEngine(RulesProperties::PropertyNotSetRuleEngine), + m_variableDuration(""), + m_variableEnvs(), + m_variableHighestSeverityAction(""), + m_variableRemoteUser(""), + m_variableTime(""), + m_variableTimeDay(""), + m_variableTimeEpoch(""), + m_variableTimeHour(""), + m_variableTimeMin(""), + m_variableTimeSec(""), + m_variableTimeWDay(""), + m_variableTimeYear(""), + m_logCbData(logCbData), TransactionAnchoredVariables(this) { m_id = std::to_string(this->m_timeStamp) + \ std::to_string(modsecurity::utils::generate_transaction_unique_id()); @@ -144,39 +174,68 @@ Transaction::Transaction(ModSecurity *ms, Rules *rules, void *logCbData) } Transaction::Transaction(ModSecurity *ms, Rules *rules, char *id, void *logCbData) - : m_clientPort(0), - m_serverPort(0), + : m_creationTimeStamp(utils::cpu_seconds()), + m_clientIpAddress(""), + m_httpVersion(""), + m_serverIpAddress(""), + m_uri(""), m_uri_no_query_string_decoded(""), - m_rules(rules), - m_timeStamp(std::time(NULL)), - m_httpCodeReturned(200), - m_highestSeverityAction(255), m_ARGScombinedSizeDouble(0), + m_clientPort(0), + m_highestSeverityAction(255), + m_httpCodeReturned(200), + m_serverPort(0), + m_ms(ms), m_requestBodyType(UnknownFormat), m_requestBodyProcessor(UnknownFormat), + m_rules(rules), + m_ruleRemoveById(), + m_ruleRemoveByIdRange(), + m_ruleRemoveByTag(), + m_ruleRemoveTargetByTag(), + m_ruleRemoveTargetById(), m_requestBodyAccess(Rules::PropertyNotSetConfigBoolean), + m_auditLogModifier(), + m_rulesMessages(), + m_requestBody(), + m_responseBody(), + m_id(std::string(id)), m_marker(""), - m_allowType(modsecurity::actions::disruptive::NoneAllowType), m_skip_next(0), - m_creationTimeStamp(utils::cpu_seconds()), - m_logCbData(logCbData), - m_ms(ms), - m_secRuleEngine(RulesProperties::PropertyNotSetRuleEngine), + m_allowType(modsecurity::actions::disruptive::NoneAllowType), + m_uri_decoded(""), + m_actions(), + m_it(), + m_timeStamp(std::time(NULL)), m_collections(ms->m_global_collection, ms->m_ip_collection, ms->m_session_collection, ms->m_user_collection, ms->m_resource_collection), -#ifdef WITH_YAJL - m_json(new RequestBodyProcessor::JSON(this)), -#else - m_json(NULL), -#endif + m_matched(), #ifdef WITH_LIBXML2 m_xml(new RequestBodyProcessor::XML(this)), #else m_xml(NULL), #endif +#ifdef WITH_YAJL + m_json(new RequestBodyProcessor::JSON(this)), +#else + m_json(NULL), +#endif + m_secRuleEngine(RulesProperties::PropertyNotSetRuleEngine), + m_variableDuration(""), + m_variableEnvs(), + m_variableHighestSeverityAction(""), + m_variableRemoteUser(""), + m_variableTime(""), + m_variableTimeDay(""), + m_variableTimeEpoch(""), + m_variableTimeHour(""), + m_variableTimeMin(""), + m_variableTimeSec(""), + m_variableTimeWDay(""), + m_variableTimeYear(""), + m_logCbData(logCbData), TransactionAnchoredVariables(this) { - m_id = std::string(id); m_rules->incrementReferenceCount(); m_variableUrlEncodedError.set("0", 0); @@ -1143,7 +1202,7 @@ int Transaction::processResponseBody() { + ". It is not marked to be inspected."); std::string validContetTypes(""); for (std::set::iterator i = bi.begin(); - i != bi.end(); i++) { + i != bi.end(); ++i) { validContetTypes.append(*i + " "); } ms_dbg(8, "Content-Type(s) marked to be inspected: " \ @@ -1250,7 +1309,7 @@ int Transaction::appendResponseBody(const unsigned char *buf, size_t len) { * @retval NULL Nothing was updated. * */ -const char *Transaction::getResponseBody() { +const char *Transaction::getResponseBody() const { // int there_is_update = this->rules->loadResponseBodyFromJS(this); return this->m_responseBody.str().c_str(); } @@ -1324,7 +1383,7 @@ int Transaction::processLogging() { ms_dbg(8, "Checking if this request is suitable to be " \ "saved as an audit log."); - if (this->m_auditLogModifier.size() > 0) { + if (!this->m_auditLogModifier.empty()) { ms_dbg(4, "There was an audit log modifier for this transaction."); std::list>::iterator it; ms_dbg(7, "AuditLog parts before modification(s): " + @@ -1754,7 +1813,7 @@ void Transaction::serverLog(std::shared_ptr rm) { } -int Transaction::getRuleEngineState() { +int Transaction::getRuleEngineState() const { if (m_secRuleEngine == RulesProperties::PropertyNotSetRuleEngine) { return m_rules->m_secRuleEngine; } diff --git a/src/unique_id.cc b/src/unique_id.cc index f271164a..883f85b0 100644 --- a/src/unique_id.cc +++ b/src/unique_id.cc @@ -72,7 +72,7 @@ void UniqueId::fillUniqueId() { // Based on: // http://stackoverflow.com/questions/16858782/how-to-obtain-almost-unique-system-identifier-in-a-cross-platform-way -std::string const UniqueId::machineName() { +std::string UniqueId::machineName() { char machine_name[MAX_MACHINE_NAME_SIZE]; size_t len = MAX_MACHINE_NAME_SIZE; #ifdef WIN32 @@ -105,7 +105,7 @@ failed: #endif } -std::string const UniqueId::ethernetMacAddress() { +std::string UniqueId::ethernetMacAddress() { char mac[MAC_ADDRESS_SIZE]; memset(mac, '\0', sizeof(char)*(MAC_ADDRESS_SIZE)); #ifdef DARWIN diff --git a/src/unique_id.h b/src/unique_id.h index 437ef657..1b275f9b 100644 --- a/src/unique_id.h +++ b/src/unique_id.h @@ -46,8 +46,8 @@ class UniqueId { } void fillUniqueId(); - std::string const machineName(); - std::string const ethernetMacAddress(); + static std::string machineName(); + static std::string ethernetMacAddress(); std::string uniqueId_str; diff --git a/src/variables/variable.h b/src/variables/variable.h index 35f51003..3e6c2837 100644 --- a/src/variables/variable.h +++ b/src/variables/variable.h @@ -106,6 +106,7 @@ namespace variables { class KeyExclusion { public: + KeyExclusion() { } virtual bool match(const std::string &a) = 0; virtual ~KeyExclusion() { } }; @@ -150,6 +151,9 @@ class KeyExclusionString : public KeyExclusion { class KeyExclusions : public std::deque> { public: + KeyExclusions() { + } + bool toOmit(std::string a) { for (auto &z : *this) { if (z->match(a)) { @@ -163,6 +167,7 @@ class KeyExclusions : public std::deque> { class VariableMonkeyResolution { public: + VariableMonkeyResolution () { } static inline bool comp(const std::string &a, const std::string &b) { return a.size() == b.size() && std::equal(a.begin(), a.end(), b.begin(), diff --git a/test/common/modsecurity_test.cc b/test/common/modsecurity_test.cc index 888b67be..034a0f52 100644 --- a/test/common/modsecurity_test.cc +++ b/test/common/modsecurity_test.cc @@ -78,8 +78,8 @@ bool ModSecurityTest::load_test_json(std::string file) { if (this->count(u->filename + ":" + u->name) == 0) { std::vector *vector = new std::vector; vector->push_back(u); - std::pair *> a(u->filename + ":" + - u->name, vector); + std::string filename(u->filename + ":" + u->name); + std::pair*> a(filename, vector); this->insert(a); } else { std::vector *vec = this->at(u->filename + ":" + u->name); diff --git a/test/cppcheck_suppressions.txt b/test/cppcheck_suppressions.txt index b0536f1b..1c7ae0df 100644 --- a/test/cppcheck_suppressions.txt +++ b/test/cppcheck_suppressions.txt @@ -1,11 +1,6 @@ -constStatement:test/common/modsecurity_test.cc:81 -duplicateCondition:src/utils/geo_lookup.cc:82 -initializerList:src/actions/action.h:90 -initializerList:src/actions/action.h:91 -invalidScanfArgType_int:src/rules_properties.cc -knownConditionTrueFalse:test/regression/regression.cc:453 -leakReturnValNotUsed:src/debug_log_writer_agent.cc:31 -nullPointerRedundantCheck:src/utils/msc_tree.cc:654 +// +// Ignore libinjection related stuff. +// *:others/libinjection/src/libinjection_html5.c *:others/libinjection/src/libinjection_sqli.c *:others/libinjection/src/libinjection_xss.c @@ -14,64 +9,55 @@ nullPointerRedundantCheck:src/utils/msc_tree.cc:654 *:others/libinjection/src/testdriver.c *:others/libinjection/src/test_speed_sqli.c *:others/libinjection/src/test_speed_xss.c + + +// +// Lets ignore mbedtls. +// *:others/mbedtls/base64.c *:others/mbedtls/md5.c *:others/mbedtls/sha1.c -*:parser/seclang-parser.hh -*:parser/seclang-scanner.cc -passedByValue:src/variables/time.h:34 -postfixOperator:* -readdirCalled:test/common/modsecurity_test.cc:114 -*:seclang-parser.tab.hh -*:seclang-scanner.cc -*:seclang-scanner.ll + + +// +// Code imported from ModSecurity v2... +// shiftNegative:src/utils/msc_tree.cc -*:src/parser/seclang-parser.cc -*:src/parser/seclang-parser.hh -*:src/parser/seclang-scanner.cc -*:src/seclang-scanner.cc *:src/utils/acmp.cc *:src/utils/msc_tree.cc -*:test/benchmark/owasp-v3/util/av-scanning/runAV/* -unreadVariable:test/regression/regression.cc:380 -unusedFunction:src/macro_expansion.cc -unusedFunction:src/modsecurity.cc -unusedFunction:src/rules.cc -unusedFunction:src/transaction.cc -unusedFunction:src/utils.cc -unusedFunction:src/utils/mbedtls/base64.c -unusedFunction:src/utils/mbedtls/md5.c -unusedFunction:src/utils/mbedtls/sha1.c -unusedFunction:src/utils/msc_tree.cc -unusedFunction:src/utils/string.cc -unusedFunction:test/optimization/optimization.cc -unusedFunction:test/regression/regression_test.cc -unusedFunction:test/unit/unit_test.cc:33 -unusedLabel:src/unique_id.cc:222 -unusedLabel:src/unique_id.cc:224 -useStlAlgorithm:src/rule.cc -useStlAlgorithm:src/rules_exceptions.cc:193 -useStlAlgorithm:src/rules_exceptions.cc:199 -useStlAlgorithm:src/utils/shared_files.cc:41 -useStlAlgorithm:test/regression/regression.cc:493 -useStlAlgorithm:test/unit/unit.cc:174 -useStlAlgorithm:test/unit/unit.cc:209 -unusedFunction -funcArgNamesDifferent -preprocessorErrorDirective -useStlAlgorithm -functionStatic:test/regression/regression_test.h:36 -missingInclude -toomanyconfigs -functionStatic:src/unique_id.h:49 -functionStatic:src/unique_id.h:50 -functionConst:src/utils/geo_lookup.h:49 -functionStatic:headers/modsecurity/transaction.h:374 +invalidScanfArgType_int:src/rules_properties.cc:102 +invalidScanfArgType_int:src/rules_properties.cc:103 -functionStatic:src/operators/geo_lookup.h:35 -useInitializationList:src/operators/rbl.h:69 - +// +// 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:373 +duplicateBranch:src/audit_log/audit_log.cc:224 +unreadVariable:src/request_body_processor/multipart.cc:391 +stlcstrParam:src/audit_log/writer/parallel.cc:145 +functionStatic:src/engine/lua.h:71 +functionStatic:src/engine/lua.h:72 +functionConst:src/utils/geo_lookup.h:49 +useInitializationList:src/operators/rbl.h:69 +constStatement:test/common/modsecurity_test.cc:82 +danglingTemporaryLifetime:src/modsecurity.cc:204 +functionStatic:src/operators/geo_lookup.h:35 +duplicateBreak:src/operators/validate_utf8_encoding.cc +duplicateBranch:src/request_body_processor/multipart.cc:91 +syntaxError:src/transaction.cc:62 +noConstructor:src/variables/variable.h:152 + + + + +unusedFunction +missingIncludeSystem +useStlAlgorithm +preprocessorErrorDirective +funcArgNamesDifferent +unmatchedSuppression +missingInclude diff --git a/test/regression/regression_test.cc b/test/regression/regression_test.cc index 453b1ca9..881ba872 100644 --- a/test/regression/regression_test.cc +++ b/test/regression/regression_test.cc @@ -23,7 +23,7 @@ namespace modsecurity_test { -const std::string RegressionTest::print() { +std::string RegressionTest::print() { std::stringstream i; #if 0 diff --git a/test/regression/regression_test.h b/test/regression/regression_test.h index bde672a2..a9fe22ce 100644 --- a/test/regression/regression_test.h +++ b/test/regression/regression_test.h @@ -33,7 +33,7 @@ class RegressionTest { public: static RegressionTest *from_yajl_node(const yajl_val &); - const std::string print(); + static std::string print(); std::string filename; std::string name; std::string title; diff --git a/test/unit/unit_test.cc b/test/unit/unit_test.cc index bface739..89616db0 100644 --- a/test/unit/unit_test.cc +++ b/test/unit/unit_test.cc @@ -66,7 +66,7 @@ void json2bin(std::string *str) { unsigned int p; std::string toBeReplaced = match.str(); toBeReplaced.erase(0, 2); - sscanf(toBeReplaced.c_str(), "%x", &p); + sscanf(toBeReplaced.c_str(), "%3x", &p); replaceAll(str, match.str(), p); }