Cosmetics: fix some cppcheck complains to please QA

Makefile.am

@@ -57,7 +57,7 @@ parser:
 
 cppcheck:
 	@cppcheck -U YYSTYPE -U MBEDTLS_MD5_ALT -U MBEDTLS_SHA1_ALT \
-		-D MS_CPPCHECK_DISABLED_FOR_PARSER \
+		-D MS_CPPCHECK_DISABLED_FOR_PARSER -U YY_USER_INIT \
 		--suppressions-list=./test/cppcheck_suppressions.txt \
 		--enable=warning,style,performance,portability,unusedFunction,missingInclude \
 		--inconclusive \

examples/multiprocess_c/multi.c

@@ -42,11 +42,14 @@ void process_special_request (int j) {
     msc_process_uri(transaction,
         "http://www.modsecurity.org/test?foo=herewego",
         "GET", "1.1");
-    msc_add_request_header(transaction, "User-Agent",
-        "Basic ModSecurity example");
+    msc_add_request_header(transaction,
+        (const unsigned char *) "User-Agent",
+        (const unsigned char *) "Basic ModSecurity example");
     msc_process_request_headers(transaction);
     msc_process_request_body(transaction);
-    msc_add_response_header(transaction, "Content-type", "text/html");
+    msc_add_response_header(transaction,
+        (const unsigned char *) "Content-type",
+        (const unsigned char *) "text/html");
     msc_process_response_headers(transaction, 200, "HTTP 1.0");
     msc_process_response_body(transaction);
     msc_process_logging(transaction);
@@ -70,11 +73,14 @@ void process_request (int j) {
         msc_process_uri(transaction,
             "http://www.modsecurity.org/test?key1=value1&key2=value2&key3=value3",
             "GET", "1.1");
-        msc_add_request_header(transaction, "User-Agent",
-            "Basic ModSecurity example");
+        msc_add_request_header(transaction,
+            (const unsigned char *) "User-Agent",
+            (const unsigned char *) "Basic ModSecurity example");
         msc_process_request_headers(transaction);
         msc_process_request_body(transaction);
-        msc_add_response_header(transaction, "Content-type", "text/html");
+        msc_add_response_header(transaction,
+            (const unsigned char *) "Content-type",
+            (const unsigned char *) "text/html");
         msc_process_response_headers(transaction, 200, "HTTP 1.0");
         msc_process_response_body(transaction);
         msc_process_logging(transaction);

examples/reading_logs_via_rule_message/simple_request.cc

@@ -29,8 +29,8 @@ int main(int argc, char **argv) {
         return -1;
     }
 
-    *(argv++);
-    std::string rules(*argv);
+    char *rule = *(argv++);
+    std::string rules(rule);
     ReadingLogsViaRuleMessage rlvrm(request_header, request_uri, request_body,
         response_headers, response_body, ip, rules);
     rlvrm.process();

examples/using_bodies_in_chunks/simple_request.cc

@@ -133,9 +133,8 @@ int main(int argc, char **argv) {
         std::cout << std::endl << std::endl;
         return -1;
     }
-    *(argv++);
-
-    std::string rules_arg(*argv);
+    char *rule = *(argv++);
+    std::string rules_arg(rule);
 
     /**
      * ModSecurity initial setup

headers/modsecurity/actions/action.h

@@ -56,6 +56,22 @@ class Action {
             set_name_and_payload(_action);
         }
 
+    Action(const Action &a)
+        : m_isNone(a.m_isNone),
+        temporaryAction(a.temporaryAction),
+        action_kind(a.action_kind),
+        m_name(a.m_name),
+        m_parser_payload(a.m_parser_payload) { }
+
+    Action &operator=(const Action& a) {
+        m_isNone = a.m_isNone;
+        temporaryAction = a.temporaryAction;
+        action_kind = a.action_kind;
+        m_name = a.m_name;
+        m_parser_payload = a.m_parser_payload;
+        return *this;
+    }
+
     virtual ~Action() { }
 
     virtual std::string evaluate(const std::string &exp,

headers/modsecurity/rule.h

@@ -73,6 +73,19 @@ class Rule {
         m_phase(modsecurity::Phases::RequestHeadersPhase) {
         }
 
+    Rule(const Rule &other) :
+        m_fileName(other.m_fileName),
+        m_lineNumber(other.m_lineNumber),
+        m_phase(other.m_phase)
+    { }
+
+    Rule &operator=(const Rule& other) {
+        m_fileName = other.m_fileName;
+        m_lineNumber = other.m_lineNumber;
+        m_phase = other.m_phase;
+        return *this;
+    }
+
     virtual bool evaluate(Transaction *transaction) = 0;
 
     virtual bool evaluate(Transaction *transaction,

headers/modsecurity/rule_message.h

@@ -69,7 +69,8 @@ class RuleMessage {
         m_serverIpAddress(trans->m_serverIpAddress),
         m_severity(0),
         m_uriNoQueryStringDecoded(trans->m_uri_no_query_string_decoded),
-        m_ver(rule->m_ver)
+        m_ver(rule->m_ver),
+        m_tags()
     { }
 
     explicit RuleMessage(RuleMessage *rule) :
@@ -93,9 +94,61 @@ class RuleMessage {
         m_serverIpAddress(rule->m_serverIpAddress),
         m_severity(rule->m_severity),
         m_uriNoQueryStringDecoded(rule->m_uriNoQueryStringDecoded),
-        m_ver(rule->m_ver)
+        m_ver(rule->m_ver),
+        m_tags(rule->m_tags)
     { }
 
+    RuleMessage(const RuleMessage& ruleMessage)
+        : m_accuracy(ruleMessage.m_accuracy),
+        m_clientIpAddress(ruleMessage.m_clientIpAddress),
+        m_data(ruleMessage.m_data),
+        m_id(ruleMessage.m_id),
+        m_isDisruptive(ruleMessage.m_isDisruptive),
+        m_match(ruleMessage.m_match),
+        m_maturity(ruleMessage.m_maturity),
+        m_message(ruleMessage.m_message),
+        m_noAuditLog(ruleMessage.m_noAuditLog),
+        m_phase(ruleMessage.m_phase),
+        m_reference(ruleMessage.m_reference),
+        m_rev(ruleMessage.m_rev),
+        m_rule(ruleMessage.m_rule),
+        m_ruleFile(ruleMessage.m_ruleFile),
+        m_ruleId(ruleMessage.m_ruleId),
+        m_ruleLine(ruleMessage.m_ruleLine),
+        m_saveMessage(ruleMessage.m_saveMessage),
+        m_serverIpAddress(ruleMessage.m_serverIpAddress),
+        m_severity(ruleMessage.m_severity),
+        m_uriNoQueryStringDecoded(ruleMessage.m_uriNoQueryStringDecoded),
+        m_ver(ruleMessage.m_ver),
+        m_tags(ruleMessage.m_tags)
+    { }
+
+    RuleMessage &operator=(const RuleMessage& ruleMessage) {
+        m_accuracy = ruleMessage.m_accuracy;
+        m_clientIpAddress = ruleMessage.m_clientIpAddress;
+        m_data = ruleMessage.m_data;
+        m_id = ruleMessage.m_id;
+        m_isDisruptive = ruleMessage.m_isDisruptive;
+        m_match = ruleMessage.m_match;
+        m_maturity = ruleMessage.m_maturity;
+        m_message = ruleMessage.m_message;
+        m_noAuditLog = ruleMessage.m_noAuditLog;
+        m_phase = ruleMessage.m_phase;
+        m_reference = ruleMessage.m_reference;
+        m_rev = ruleMessage.m_rev;
+        m_rule = ruleMessage.m_rule;
+        m_ruleFile = ruleMessage.m_ruleFile;
+        m_ruleId = ruleMessage.m_ruleId;
+        m_ruleLine = ruleMessage.m_ruleLine;
+        m_saveMessage = ruleMessage.m_saveMessage;
+        m_serverIpAddress = ruleMessage.m_serverIpAddress;
+        m_severity = ruleMessage.m_severity;
+        m_uriNoQueryStringDecoded = ruleMessage.m_uriNoQueryStringDecoded;
+        m_ver = ruleMessage.m_ver;
+        m_tags = ruleMessage.m_tags;
+        return *this;
+    }
+
     void clean() {
         m_data = "";
         m_match = "";

headers/modsecurity/rule_with_actions.h

@@ -45,6 +45,57 @@ class RuleWithActions : public Rule {
 
     ~RuleWithActions();
 
+    RuleWithActions(const RuleWithActions& r)
+        : Rule(r),
+        m_rev(r.m_rev),
+        m_ver(r.m_ver),
+        m_accuracy(r.m_accuracy),
+        m_maturity(r.m_maturity),
+        m_ruleId(r.m_ruleId),
+        m_chainedRuleChild(r.m_chainedRuleChild),
+        m_chainedRuleParent(r.m_chainedRuleParent),
+        m_disruptiveAction(r.m_disruptiveAction),
+        m_logData(r.m_logData),
+        m_msg(r.m_msg),
+        m_severity(r.m_severity),
+        m_actionsRuntimePos(r.m_actionsRuntimePos),
+        m_actionsSetVar(r.m_actionsSetVar),
+        m_actionsTag(r.m_actionsTag),
+        m_transformations(r.m_transformations),
+        m_containsCaptureAction(r.m_containsCaptureAction),
+        m_containsMultiMatchAction(r.m_containsMultiMatchAction),
+        m_containsStaticBlockAction(r.m_containsStaticBlockAction),
+        m_isChained(r.m_isChained)
+    { }
+
+    RuleWithActions &operator=(const RuleWithActions& r) {
+        Rule::operator = (r);
+        m_rev = r.m_rev;
+        m_ver = r.m_ver;
+        m_accuracy = r.m_accuracy;
+        m_maturity = r.m_maturity;
+        m_ruleId = r.m_ruleId;
+        m_chainedRuleChild = r.m_chainedRuleChild;
+        m_chainedRuleParent = r.m_chainedRuleParent;
+
+        m_disruptiveAction = r.m_disruptiveAction;
+        m_logData = r.m_logData;
+        m_msg = r.m_msg;
+        m_severity = r.m_severity;
+        m_actionsRuntimePos = r.m_actionsRuntimePos;
+        m_actionsSetVar = r.m_actionsSetVar;
+        m_actionsTag = r.m_actionsTag;
+
+        m_transformations = r.m_transformations;
+
+        m_containsCaptureAction = r.m_containsCaptureAction;
+        m_containsMultiMatchAction = r.m_containsMultiMatchAction;
+        m_containsStaticBlockAction = r.m_containsStaticBlockAction;
+        m_isChained = r.m_isChained;
+
+        return *this;
+    }
+
     virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
 
     virtual bool evaluate(Transaction *transaction) override;
@@ -111,7 +162,7 @@ class RuleWithActions : public Rule {
 
     int64_t m_ruleId;
 
-    std::unique_ptr<RuleWithActions> m_chainedRuleChild;
+    std::shared_ptr<RuleWithActions> m_chainedRuleChild;
     RuleWithActions *m_chainedRuleParent;
 
  private:

headers/modsecurity/rules.h

@@ -63,7 +63,7 @@ class Rules {
          return j;
     }
 
-    bool insert(std::shared_ptr<Rule> rule) {
+    bool insert(const std::shared_ptr<Rule> &rule) {
         return insert(rule, nullptr, nullptr);
     }
 

headers/modsecurity/transaction.h

@@ -309,7 +309,7 @@ class TransactionSecMarkerManagement {
         m_marker.reset();
     }
 
-    void addMarker(std::shared_ptr<std::string> name) {
+    void addMarker(const std::shared_ptr<std::string> &name) {
         m_marker = name;
     }
 

src/actions/tag.cc

@@ -24,6 +24,7 @@
 #include "modsecurity/rule.h"
 #include "modsecurity/rule_message.h"
 
+#ifdef MSC_DOCUMENTATION
 /**
  * Description: Assigns a tag (category) to a rule or a chain.
  *
@@ -44,7 +45,7 @@
  *
  *
  */
-
+#endif
 
 namespace modsecurity {
 namespace actions {

src/actions/transformations/remove_whitespace.cc

@@ -41,11 +41,12 @@ std::string RemoveWhitespace::evaluate(const std::string &val,
     std::string value(val);
 
     int64_t i = 0;
+    char nonBreakingSpaces = 0xa0;
 
     // loop through all the chars
     while (i < value.size()) {
         // remove whitespaces and non breaking spaces (NBSP)
-        if (isspace(value[i]) || (value[i] == NBSP)) {
+        if (isspace(value[i]) || (value[i] == nonBreakingSpaces)) {
             value.erase(i, 1);
         } else {
           /* if the space is not a whitespace char, increment counter

src/actions/transformations/trim.cc

@@ -32,15 +32,25 @@ namespace transformations {
 
 
 std::string *Trim::ltrim(std::string *s) {
-    s->erase(s->begin(), std::find_if(s->begin(), s->end(),
-        std::not1(std::ptr_fun<int, int>(std::isspace))));
+    s->erase(
+        s->begin(),
+        std::find_if(s->begin(), s->end(), [](unsigned char c) {
+            return !std::isspace(c);
+        })
+    );
+
     return s;
 }
 
 
 std::string *Trim::rtrim(std::string *s) {
-    s->erase(std::find_if(s->rbegin(), s->rend(),
-        std::not1(std::ptr_fun<int, int>(std::isspace))).base(), s->end());
+    s->erase(
+        std::find_if(s->rbegin(), s->rend(), [](unsigned char c) {
+            return !std::isspace(c);
+        }).base(),
+        s->end()
+    );
+
     return s;
 }
 

src/actions/transformations/url_decode.cc

@@ -39,7 +39,7 @@ UrlDecode::UrlDecode(const std::string &action)
 
 std::string UrlDecode::evaluate(const std::string &value,
     Transaction *transaction) {
-    unsigned char *val = NULL;
+    unsigned char *val(NULL);
     int invalid_count = 0;
     int changed;
 

src/engine/lua.cc

@@ -65,8 +65,7 @@ bool Lua::isCompatible(const std::string &script, Lua *l, std::string *error) {
 
 bool Lua::load(const std::string &script, std::string *err) {
 #ifdef WITH_LUA
-    lua_State *L = NULL;
-    L = luaL_newstate();
+    lua_State *L = luaL_newstate();
     luaL_openlibs(L);
 
     m_scriptName = script;
@@ -234,7 +233,7 @@ err:
 
 #ifdef WITH_LUA
 int Lua::log(lua_State *L) {
-    const Transaction *t = NULL;
+    const Transaction *t(NULL);
     const char *text;
     int level;
 
@@ -256,9 +255,9 @@ int Lua::log(lua_State *L) {
 
 
 int Lua::getvar(lua_State *L) {
-    const char *varname = NULL;
-    Transaction *t = NULL;
-    void *z = NULL;
+    const char *varname(NULL);
+    Transaction *t(NULL);
+    void *z(NULL);
 
     /* Retrieve parameters. */
     varname = reinterpret_cast<const char *>(luaL_checkstring(L, 1));
@@ -282,9 +281,9 @@ int Lua::getvar(lua_State *L) {
 
 
 int Lua::getvars(lua_State *L) {
-    const char *varname = NULL;
-    Transaction *t = NULL;
-    void *z = NULL;
+    const char *varname(NULL);
+    Transaction *t(NULL);
+    void *z(NULL);
     std::vector<const VariableValue *> l;
     int idx = 1;
 
@@ -323,16 +322,16 @@ int Lua::getvars(lua_State *L) {
 
 
 int Lua::setvar(lua_State *L) {
-    Transaction *t = NULL;
-    const char *var_value = NULL;
-    const char *var_name = NULL;
+    Transaction *t(NULL);
+    const char *var_value(NULL);
+    const char *var_name(NULL);
     std::string vname;
     std::string collection;
     std::string variableName;
     int nargs = lua_gettop(L);
     char *chr = NULL;
     size_t pos;
-    void *z = NULL;
+    void *z(NULL);
 
     lua_getglobal(L, "__transaction");
     z = const_cast<void *>(lua_topointer(L, -1));
@@ -453,7 +452,7 @@ std::string Lua::applyTransformations(lua_State *L, Transaction *t,
     }
 
     if (lua_isstring(L, idx)) {
-        const char *name = NULL;
+        const char *name(NULL);
         name = reinterpret_cast<const char *>(luaL_checkstring(L, idx));
 
         actions::transformations::Transformation *tfn = \

src/engine/lua.h

@@ -45,8 +45,7 @@ class LuaScriptBlob {
 
 
     void write(const void *data, size_t len) {
-        unsigned char *d = NULL;
-        d = (unsigned char *)realloc((unsigned char *)m_data, len + m_len);
+        unsigned char *d = (unsigned char *)realloc((unsigned char *)m_data, len + m_len);
         std::memcpy(d + m_len, data, len);
         m_len = m_len + len;
         m_data = d;

src/operators/detect_sqli.cc

@@ -32,8 +32,11 @@ bool DetectSQLi::evaluate(Transaction *t, RuleWithActions *rule,
 
     issqli = libinjection_sqli(input.c_str(), input.length(), fingerprint);
 
+    if (!t) {
+        goto tisempty;
+    }
+
     if (issqli) {
-        if (t) {
         t->m_matched.push_back(fingerprint);
         ms_dbg_a(t, 4, "detected SQLi using libinjection with " \
             "fingerprint '" + std::string(fingerprint) + "' at: '" +
@@ -44,14 +47,12 @@ bool DetectSQLi::evaluate(Transaction *t, RuleWithActions *rule,
             ms_dbg_a(t, 7, "Added DetectSQLi match TX.0: " + \
                 std::string(fingerprint));
         }
-        }
     } else {
-        if (t) {
         ms_dbg_a(t, 9, "detected SQLi: not able to find an " \
             "inject on '" + input + "'");
     }
-    }
 
+tisempty:
     return issqli != 0;
 }
 

src/operators/rx.cc

@@ -69,12 +69,12 @@ bool Rx::evaluate(Transaction *transaction, RuleWithActions *rule,
         logOffset(ruleMessage, capture.m_offset, capture.m_length);
     }
 
-    if (m_string->m_containsMacro) {
-        delete re;
+    if (!captures.empty()) {
+        return true;
     }
 
-    if (captures.size() > 0) {
-        return true;
+    if (m_string->m_containsMacro) {
+        delete re;
     }
 
     return false;

src/rule_script.h

@@ -50,7 +50,8 @@ class RuleScript : public RuleWithActions {
         std::unique_ptr<std::string> fileName,
         int lineNumber)
             : RuleWithActions(actions, t, std::move(fileName), lineNumber),
-        m_name(name) { }
+        m_name(name),
+        m_lua() { }
 
     bool init(std::string *err);
     bool evaluate(Transaction *trans,

src/transaction.cc

@@ -340,7 +340,6 @@ bool Transaction::extractArguments(const std::string &orig,
 
     for (std::string t : key_value_sets) {
         char sep2 = '=';
-        int i = 0;
         size_t key_s = 0;
         size_t value_s = 0;
         int invalid = 0;
@@ -1307,14 +1306,13 @@ int Transaction::appendResponseBody(const unsigned char *buf, size_t len) {
  * contents of the response body, otherwise there is no need to call this
  * method.
  *
+ * WARN: This is a skeleton that it is not in use yet.
+ *
  * @return It returns a buffer (const char *)
- * @retval >0   body was update and available.
- * @retval NULL Nothing was updated.
  *
  */
 const char *Transaction::getResponseBody() const {
-    // int there_is_update = this->rules->loadResponseBodyFromJS(this);
-    return this->m_responseBody.str().c_str();
+    return strdup(this->m_responseBody.str().c_str());
 }
 
 

src/utils/geo_lookup.cc

@@ -56,8 +56,12 @@ void GeoLookup::cleanUp() {
 
 bool GeoLookup::setDataBase(const std::string& filePath,
     std::string *err) {
+#ifdef WITH_MAXMIND
     std::string intMax;
+#endif
+#ifdef WITH_GEOIP
     std::string intGeo;
+#endif
 
 #ifdef WITH_MAXMIND
     int status = MMDB_open(filePath.c_str(), MMDB_MODE_MMAP, &mmdb);
@@ -85,19 +89,22 @@ bool GeoLookup::setDataBase(const std::string& filePath,
 #ifdef WITH_MAXMIND
         err->append(" libMaxMind");
 #endif
-
 #ifdef WITH_GEOIP
         err->append(" GeoIP");
 #endif
         err->append(".");
 
+#ifdef WITH_MAXMIND
         if (!intMax.empty()) {
             err->append(" " + intMax);
-
         }
+#endif
+#ifdef WITH_GEOIP
         if (!intGeo.empty()) {
             err->append(" " + intGeo);
         }
+#endif
+
         return false;
     }
 

src/utils/msc_tree.cc

@@ -290,7 +290,7 @@ TreeNode *CPTAddElement(unsigned char *ipdata, unsigned int ip_bitmask, CPTTree
     unsigned char *buffer = NULL;
     unsigned char bitlen = 0;
     int bit_validation = 0, test_bit = 0;
-    int i = 0;
+    size_t i = 0;
     unsigned int x, y;
     TreeNode *node = NULL, *new_node = NULL;
     TreeNode *parent = NULL, *i_node = NULL;

src/utils/shared_files.h

@@ -81,7 +81,7 @@ class SharedFiles {
     {
 #ifdef MODSEC_USE_GENERAL_LOCK
         int shm_id;
-        bool toBeCreated;
+        bool toBeCreated(false);
         bool err = false;
 
         m_memKeyStructure = ftok(".", 1);

test/cppcheck_suppressions.txt

@@ -39,8 +39,8 @@ functionStatic:headers/modsecurity/transaction.h:405
 duplicateBranch:src/audit_log/audit_log.cc:223
 unreadVariable:src/request_body_processor/multipart.cc:435
 stlcstrParam:src/audit_log/writer/parallel.cc:145
+functionStatic:src/engine/lua.h:70
 functionStatic:src/engine/lua.h:71
-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
@@ -51,11 +51,12 @@ duplicateBranch:src/request_body_processor/multipart.cc:91
 syntaxError:src/transaction.cc:62
 noConstructor:src/variables/variable.h:152
 duplicateBranch:src/request_body_processor/multipart.cc:93
-knownConditionTrueFalse:src/utils/geo_lookup.cc:94
-knownConditionTrueFalse:src/utils/geo_lookup.cc:98
 danglingTempReference:src/modsecurity.cc:204
+knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77
+knownConditionTrueFalse:src/operators/verify_svnr.cc:87
 
 noExplicitConstructor:seclang-parser.hh
+constParameter:seclang-parser.hh
 
 unusedFunction
 missingIncludeSystem
@@ -65,3 +66,9 @@ funcArgNamesDifferent
 unmatchedSuppression
 missingInclude
 
+purgedConfiguration
+
+
+// Examples
+memleak:examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h:147
+memleak:examples/using_bodies_in_chunks/simple_request.cc

test/fuzzer/afl_fuzzer.cc

@@ -128,7 +128,9 @@ inline void op_test(const std::string &opName, const std::string &s) {
 int main(int argc, char** argv) {
     uint8_t buf[128];
 
+#if 0
     std::string lastString;
+#endif
 
     while (__AFL_LOOP(1000)) {
         ssize_t read_bytes;
@@ -138,7 +140,9 @@ int main(int argc, char** argv) {
 
         std::string currentString = std::string(read_bytes, 128);
         std::string s = currentString;
+#if 0
         std::string z = lastString;
+#endif
 
         ModSecurity *ms = new ModSecurity();
         RulesSet *rules = new RulesSet();
@@ -266,8 +270,9 @@ op_test("Within", s);
         delete t;
         delete rules;
         delete ms;
-
+#if 0
         lastString = currentString;
+#endif
     }
     return 0;
 }

test/test-cases/secrules-language-tests

@@ -1 +1 @@
-Subproject commit d03f4c1e930440df46c1faa37d820a919704d9da
+Subproject commit a3d4405e5a2c90488c387e589c5534974575e35b