Cosmetics: fix some cppcheck complains

This commit is contained in:
Felipe Zimmerle 2020-04-29 10:19:49 -03:00
parent 42ec74fcac
commit f7e9039f20
No known key found for this signature in database
GPG Key ID: E6DFB08CE8B11277
21 changed files with 164 additions and 52 deletions

View File

@ -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=all \
--inconclusive \

View File

@ -63,6 +63,15 @@ class Action {
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 execute(const std::string &exp,

View File

@ -56,6 +56,13 @@ class Rule {
}
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;
std::shared_ptr<std::string> getFileName() const {

View File

@ -55,6 +55,19 @@ class RuleMessage {
{ }
RuleMessage &operator=(const RuleMessage& ruleMessage) {
m_severity = ruleMessage.m_severity;
m_tags = ruleMessage.m_tags;
m_data = ruleMessage.m_data;
m_match = ruleMessage.m_match;
m_message = ruleMessage.m_message;
m_reference = ruleMessage.m_reference;
m_transaction = ruleMessage.m_transaction;
m_rule = ruleMessage.m_rule;
return *this;
}
explicit RuleMessage(Transaction *transaction) :
m_severity(0),
m_tags(),

View File

@ -124,6 +124,50 @@ class RuleWithActions : public Rule {
m_defaultContainsStaticBlockAction(r.m_defaultContainsStaticBlockAction),
m_isChained(r.m_isChained) { };
RuleWithActions &operator=(const RuleWithActions& r) {
Rule::operator = (r);
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_actionsRuntimePos = r.m_actionsRuntimePos;
m_actionsSetVar = r.m_actionsSetVar;
m_actionsTag = r.m_actionsTag;
m_XmlNSs = r.m_XmlNSs;
m_defaultActionDisruptiveAction = r.m_defaultActionDisruptiveAction;
m_defaultActionLogData = r.m_defaultActionLogData;
m_defaultActionMsg = r.m_defaultActionMsg;
m_defaultActionActionsRuntimePos = r.m_defaultActionActionsRuntimePos;
m_defaultActionActionsSetVar = r.m_defaultActionActionsSetVar;
m_defaultActionActionsTag = r.m_defaultActionActionsTag;
m_transformations = r.m_transformations;
m_defaultTransformations = r.m_defaultTransformations;
m_severity = r.m_severity;
m_revision = r.m_revision;
m_version = r.m_version;
m_accuracy = r.m_accuracy;
m_maturity = r.m_maturity;
m_containsCaptureAction = r.m_containsCaptureAction;
m_containsLogAction = r.m_containsLogAction;
m_containsNoLogAction = r.m_containsNoLogAction;
m_containsMultiMatchAction = r.m_containsMultiMatchAction;
m_containsStaticBlockAction = r.m_containsStaticBlockAction;
m_defaultSeverity = r.m_defaultSeverity;
m_defaultRevision = r.m_defaultRevision;
m_defaultVersion = r.m_defaultVersion;
m_defaultAccuracy = r.m_defaultAccuracy;
m_defaultMaturity = r.m_defaultMaturity;
m_defaultContainsCaptureAction = r.m_defaultContainsCaptureAction;
m_defaultContainsLogAction = r.m_defaultContainsLogAction;
m_defaultContainsNoLogAction = r.m_defaultContainsNoLogAction;
m_defaultContainsMultiMatchAction = r.m_defaultContainsMultiMatchAction;
m_defaultContainsStaticBlockAction = r.m_defaultContainsStaticBlockAction;
m_isChained = r.m_isChained;
return *this;
}
virtual bool evaluate(Transaction *transaction) override;
@ -263,7 +307,7 @@ class RuleWithActions : public Rule {
inline bool hasCaptureAction() const { return m_containsCaptureAction || m_defaultContainsCaptureAction; }
inline bool hasDisruptiveAction() const { return m_disruptiveAction != nullptr || m_defaultActionDisruptiveAction != nullptr; }
inline void setDisruptiveAction(std::shared_ptr<actions::Action> a) { m_disruptiveAction = a; }
inline void setDisruptiveAction(const std::shared_ptr<actions::Action> &a) { m_disruptiveAction = a; }
inline std::shared_ptr<actions::Action> getDisruptiveAction() const { return m_disruptiveAction; }
inline bool hasBlockAction() const { return m_containsStaticBlockAction || m_defaultContainsStaticBlockAction; }
@ -279,11 +323,11 @@ class RuleWithActions : public Rule {
inline bool hasLogDataAction() const { return m_logData != nullptr || m_defaultActionLogData != nullptr; }
inline std::shared_ptr<actions::LogData> getLogDataAction() const { return m_logData; }
std::string getLogData(/*const */Transaction *t);
inline void setLogDataAction(std::shared_ptr<actions::LogData> data) { m_logData = data; }
inline void setLogDataAction(const std::shared_ptr<actions::LogData> &data) { m_logData = data; }
inline bool hasMessageAction() const { return m_msg != nullptr || m_defaultActionMsg != nullptr; }
inline std::shared_ptr<actions::Msg> getMessageAction() const { return m_msg; }
inline void setMessageAction(std::shared_ptr<actions::Msg> msg) { m_msg = msg; }
inline void setMessageAction(const std::shared_ptr<actions::Msg> &msg) { m_msg = msg; }
std::string getMessage(/*const */Transaction *t);

View File

@ -50,6 +50,13 @@ class RuleWithOperator : public RuleWithActions {
m_variables(op.m_variables),
m_operator(op.m_operator) { };
RuleWithOperator &operator=(const RuleWithOperator& r) {
RuleWithActions::operator = (r);
m_variables = r.m_variables;
m_operator = r.m_operator;
return *this;
}
virtual ~RuleWithOperator();
bool evaluate(Transaction *transaction) override;

View File

@ -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);
}

View File

@ -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;
}

View File

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

View File

@ -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 = \

View File

@ -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;

View File

@ -256,7 +256,7 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
strlen("highlight"));
yajl_gen_array_open(g);
while (vars.size() > 0) {
while (!vars.empty()) {
std::string value;
yajl_gen_map_open(g);
vars.pop_back();
@ -303,7 +303,7 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
varValue.size());
yajl_gen_map_close(g);
while (trans.size() > 0) {
while (!trans.empty()) {
modsecurity::actions::transformations::Transformation *t;
std::string varValueRes;
yajl_gen_map_open(g);
@ -338,7 +338,7 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
yajl_gen_map_open(g);
while (ops.size() > 0) {
while (!ops.empty()) {
std::string value;
yajl_gen_string(g, reinterpret_cast<const unsigned char*>("highlight"),
strlen("highlight"));

View File

@ -32,26 +32,27 @@ 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: '" +
input + "'");
if (rule && t && rule->hasCaptureAction()) {
if (rule && rule->hasCaptureAction()) {
t->m_collections.m_tx_collection->storeOrUpdateFirst(
"0", std::string(fingerprint));
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;
}

View File

@ -74,7 +74,7 @@ bool Rx::evaluate(Transaction *transaction, RuleWithActions *rule,
delete re;
}
if (matches.size() > 0) {
if (!matches.empty()) {
return true;
}

View File

@ -58,6 +58,12 @@ class RuleScript : public RuleWithActions {
m_name(rs.m_name),
m_lua(rs.m_lua) { }
RuleScript &operator=(const RuleScript& r) {
RuleWithActions::operator = (r);
m_name = r.m_name;
m_lua = r.m_lua;
return *this;
}
bool init(std::string *err);
bool evaluate(Transaction *trans) override;

View File

@ -368,7 +368,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;

View File

@ -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.size() > 0) {
err->append(" " + intMax);
}
#endif
#ifdef WITH_GEOIP
if (intGeo.size() > 0) {
err->append(" " + intGeo);
}
#endif
return false;
}

View File

@ -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);

View File

@ -38,7 +38,6 @@ class WebAppId : public Variable {
void evaluate(Transaction *transaction,
RuleWithActions *rule,
std::vector<const VariableValue *> *l) override {
const std::string name("WEBAPPID");
const std::string rname = transaction->m_rules->m_secWebAppId.m_value;
l->push_back(new VariableValue(&m_name, &rname));
}

View File

@ -41,8 +41,8 @@ functionStatic:headers/modsecurity/transaction.h:453
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
@ -53,6 +53,9 @@ 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/operators/validate_url_encoding.cc:77
knownConditionTrueFalse:src/operators/verify_svnr.cc:87
noExplicitConstructor:seclang-parser.hh
@ -63,3 +66,17 @@ preprocessorErrorDirective
funcArgNamesDifferent
unmatchedSuppression
missingInclude
unusedFunction
missingIncludeSystem
useStlAlgorithm
preprocessorErrorDirective
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

View File

@ -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;
}