Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove cppcheck suppressions with line numbers in test/cppcheck_suppressions.txt #3134

Merged
Merged
1 change: 1 addition & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ cppcheck:
@cppcheck -U YYSTYPE -U MBEDTLS_MD5_ALT -U MBEDTLS_SHA1_ALT \
-D MS_CPPCHECK_DISABLED_FOR_PARSER -U YY_USER_INIT \
--suppressions-list=./test/cppcheck_suppressions.txt \
--inline-suppr \
--enable=warning,style,performance,portability,unusedFunction,missingInclude \
--inconclusive \
--template="warning: {file},{line},{severity},{id},{message}" \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,35 +124,31 @@ class ReadingLogsViaRuleMessage {
m_rules(rules)
{ }

int process() {
int process() const {
pthread_t threads[NUM_THREADS];
int i;
struct data_ms dms;
void *status;

modsecurity::ModSecurity *modsec;
modsecurity::RulesSet *rules;

modsec = new modsecurity::ModSecurity();
auto modsec = std::make_unique<modsecurity::ModSecurity>();
modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha" \
" (ModSecurity test)");
modsec->setServerLogCb(logCb, modsecurity::RuleMessageLogProperty
| modsecurity::IncludeFullHighlightLogProperty);

rules = new modsecurity::RulesSet();
auto rules = std::make_unique<modsecurity::RulesSet>();
if (rules->loadFromUri(m_rules.c_str()) < 0) {
std::cout << "Problems loading the rules..." << std::endl;
std::cout << rules->m_parserError.str() << std::endl;
return -1;
}

dms.modsec = modsec;
dms.rules = rules;
dms.modsec = modsec.get();
dms.rules = rules.get();

for (i = 0; i < NUM_THREADS; i++) {
pthread_create(&threads[i], NULL, process_request,
reinterpret_cast<void *>(&dms));
// process_request((void *)&dms);
}

usleep(10000);
Expand All @@ -162,8 +158,6 @@ class ReadingLogsViaRuleMessage {
std::cout << "Main: completed thread id :" << i << std::endl;
}

delete rules;
delete modsec;
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions headers/modsecurity/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ class TransactionSecMarkerManagement {
if (m_marker) {
return m_marker;
} else {
throw;
throw; // cppcheck-suppress rethrowNoCurrentException
}
}

Expand Down Expand Up @@ -405,7 +405,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
size_t getRequestBodyLength();

#ifndef NO_LOGS
void debug(int, const std::string&) const;
void debug(int, const std::string &) const; // cppcheck-suppress functionStatic
#endif
void serverLog(std::shared_ptr<RuleMessage> rm);

Expand Down
2 changes: 1 addition & 1 deletion src/audit_log/writer/parallel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ bool Parallel::write(Transaction *transaction, int parts, std::string *error) {
log = transaction->toOldAuditLogFormat(parts, "-" + boundary + "--");
}

std::string logPath = m_audit->m_storage_dir;
const auto &logPath = m_audit->m_storage_dir;
fileName = logPath + fileName + "-" + *transaction->m_id.get();

if (logPath.empty()) {
Expand Down
2 changes: 1 addition & 1 deletion src/collection/backend/lmdb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ MDB_dbi* MDBEnvProvider::GetDBI() {
return &m_dbi;
}

bool MDBEnvProvider::isValid() {
bool MDBEnvProvider::isValid() const {
return valid;
}

Expand Down
2 changes: 1 addition & 1 deletion src/collection/backend/lmdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class MDBEnvProvider {
}
MDB_env* GetEnv();
MDB_dbi* GetDBI();
bool isValid();
bool isValid() const;

~MDBEnvProvider();
private:
Expand Down
4 changes: 2 additions & 2 deletions src/engine/lua.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ class Lua {
public:
Lua() { }

bool load(const std::string &script, std::string *err);
int run(Transaction *t, const std::string &str="");
bool load(const std::string &script, std::string *err); // cppcheck-suppress functionStatic ; triggered when compiling without LUA
int run(Transaction *t, const std::string &str = ""); // cppcheck-suppress functionStatic ; triggered when compiling without LUA
static bool isCompatible(const std::string &script, Lua *l, std::string *error);

#ifdef WITH_LUA
Expand Down
2 changes: 1 addition & 1 deletion src/modsecurity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ void ModSecurity::serverLog(void *data, std::shared_ptr<RuleMessage> rm) {
}

if (m_logProperties & TextLogProperty) {
std::string &&d = rm->log();
auto d = rm->log();
const void *a = static_cast<const void *>(d.c_str());
m_logCb(data, a);
return;
Expand Down
1 change: 1 addition & 0 deletions src/operators/geo_lookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class GeoLookup : public Operator {
bool evaluate(Transaction *transaction, const std::string &exp) override;

protected:
// cppcheck-suppress functionStatic
bool debug(Transaction *transaction, int x, const std::string &a) {
ms_dbg_a(transaction, x, a);
return true;
Expand Down
9 changes: 5 additions & 4 deletions src/operators/rbl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ class Rbl : public Operator {
m_demandsPassword(false),
m_provider(RblProvider::UnknownProvider),
Operator("Rbl", std::move(param)) {
m_service = m_string->evaluate();
if (m_service.find("httpbl.org") != std::string::npos) {
m_demandsPassword = true;
m_provider = RblProvider::httpbl;
m_service = m_string->evaluate(); // cppcheck-suppress useInitializationList
if (m_service.find("httpbl.org") != std::string::npos)
{
m_demandsPassword = true;
m_provider = RblProvider::httpbl;
} else if (m_service.find("uribl.com") != std::string::npos) {
m_provider = RblProvider::uribl;
} else if (m_service.find("spamhaus.org") != std::string::npos) {
Expand Down
2 changes: 1 addition & 1 deletion src/operators/validate_url_encoding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ bool ValidateUrlEncoding::evaluate(Transaction *transaction, RuleWithActions *ru
bool res = false;

if (input.empty() == true) {
return res;
return res; // cppcheck-suppress knownConditionTrueFalse
}

int rc = validate_url_encoding(input.c_str(), input.size(), &offset);
Expand Down
5 changes: 0 additions & 5 deletions src/operators/validate_utf8_encoding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
std::to_string(i) + "\"]");
}
return true;
break;
case UNICODE_ERROR_INVALID_ENCODING :
if (transaction) {
ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: "
Expand All @@ -142,7 +141,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
logOffset(ruleMessage, i, str.size());
}
return true;
break;
case UNICODE_ERROR_OVERLONG_CHARACTER :
if (transaction) {
ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: "
Expand All @@ -152,7 +150,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
logOffset(ruleMessage, i, str.size());
}
return true;
break;
case UNICODE_ERROR_RESTRICTED_CHARACTER :
if (transaction) {
ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: "
Expand All @@ -162,7 +159,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
logOffset(ruleMessage, i, str.size());
}
return true;
break;
case UNICODE_ERROR_DECODING_ERROR :
if (transaction) {
ms_dbg_a(transaction, 8, "Error validating UTF-8 decoding "
Expand All @@ -171,7 +167,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
logOffset(ruleMessage, i, str.size());
}
return true;
break;
}

if (rc <= 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/operators/verify_cpf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ bool VerifyCPF::verify(const char *cpfnumber, int len) {
c = cpf_len;

for (i = 0; i < 9; i++) {
sum += (cpf[i] * --c);
sum += (cpf[i] * --c); // cppcheck-suppress uninitvar
}

factor = (sum % cpf_len);
Expand Down
4 changes: 2 additions & 2 deletions src/operators/verify_svnr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ bool VerifySVNR::verify(const char *svnrnumber, int len) {
}
//Laufnummer mit 3, 7, 9
//Geburtsdatum mit 5, 8, 4, 2, 1, 6
sum = svnr[0] * 3 + svnr[1] * 7 + svnr[2] * 9 + svnr[4] * 5 + svnr[5] * 8 + svnr[6] * 4 + svnr[7] * 2 + svnr[8] * 1 + svnr[9] * 6;
sum = svnr[0] * 3 + svnr[1] * 7 + svnr[2] * 9 + svnr[4] * 5 + svnr[5] * 8 + svnr[6] * 4 + svnr[7] * 2 + svnr[8] * 1 + svnr[9] * 6; // cppcheck-suppress uninitvar
sum %= 11;
if(sum == 10){
sum = 0;
Expand All @@ -84,7 +84,7 @@ bool VerifySVNR::evaluate(Transaction *t, RuleWithActions *rule,
int i;

if (m_param.empty()) {
return is_svnr;
return is_svnr; // cppcheck-suppress knownConditionTrueFalse
}

for (i = 0; i < input.size() - 1 && is_svnr == false; i++) {
Expand Down
4 changes: 2 additions & 2 deletions src/rule_with_actions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ RuleWithActions::RuleWithActions(
delete a;
std::cout << "General failure, action: " << a->m_name;
std::cout << " has an unknown type." << std::endl;
throw;
throw; // cppcheck-suppress rethrowNoCurrentException
}
}
delete actions;
Expand Down Expand Up @@ -241,7 +241,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans,
bool containsBlock, std::shared_ptr<RuleMessage> ruleMessage) {
bool disruptiveAlreadyExecuted = false;

for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) {
for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) { // cppcheck-suppress ctunullpointer
if (a.get()->action_kind != actions::Action::RunTimeOnlyIfMatchKind) {
continue;
}
Expand Down
7 changes: 4 additions & 3 deletions src/rule_with_operator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ void RuleWithOperator::updateMatchedVars(Transaction *trans, const std::string &

void RuleWithOperator::cleanMatchedVars(Transaction *trans) {
ms_dbg_a(trans, 9, "Matched vars cleaned.");
// cppcheck-suppress ctunullpointer
trans->m_variableMatchedVar.unset();
trans->m_variableMatchedVars.unset();
trans->m_variableMatchedVarName.unset();
Expand Down Expand Up @@ -132,7 +133,7 @@ bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string &

void RuleWithOperator::getVariablesExceptions(Transaction *t,
variables::Variables *exclusion, variables::Variables *addition) {
for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) {
for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer
if (containsTag(*a.first.get(), t) == false) {
continue;
}
Expand All @@ -146,7 +147,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t,
}
}

for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) {
for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) {
if (containsMsg(*a.first.get(), t) == false) {
continue;
}
Expand All @@ -160,7 +161,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t,
}
}

for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) {
for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) {
if (m_ruleId != a.first) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/rules_set_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ void ConfigUnicodeMap::loadConfig(std::string f, double configCodePage,

if (mapping != NULL) {
ucode = strtok_r(mapping, ":", &hmap);
sscanf(ucode, "%x", &code);
sscanf(hmap, "%x", &Map);
sscanf(ucode, "%x", &code); // cppcheck-suppress invalidScanfArgType_int
sscanf(hmap, "%x", &Map); // cppcheck-suppress invalidScanfArgType_int
if (code >= 0 && code <= 65535) {
driver->m_unicodeMapTable.m_unicodeMapTable->change(code, Map);
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/shared_files.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class SharedFiles {
bool toBeCreated(false);
bool err = false;

m_memKeyStructure = ftok(".", 1);
m_memKeyStructure = ftok(".", 1); // cppcheck-suppress useInitializationList
if (m_memKeyStructure < 0) {
err = true;
goto err_mem_key;
Expand Down
13 changes: 6 additions & 7 deletions test/common/modsecurity_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ std::string ModSecurityTest<T>::header() {
}

template <class T>
bool ModSecurityTest<T>::load_test_json(std::string file) {
bool ModSecurityTest<T>::load_test_json(const std::string &file) {
char errbuf[1024];
yajl_val node;

Expand Down Expand Up @@ -76,13 +76,12 @@ bool ModSecurityTest<T>::load_test_json(std::string file) {
u->filename = file;

if (this->count(u->filename + ":" + u->name) == 0) {
std::vector<T *> *vector = new std::vector<T *>;
vector->push_back(u);
auto vec = new std::vector<T *>;
vec->push_back(u);
std::string filename(u->filename + ":" + u->name);
std::pair<std::string, std::vector<T*>*> a(filename, vector);
this->insert(a);
this->insert({filename, vec});
} else {
std::vector<T *> *vec = this->at(u->filename + ":" + u->name);
auto vec = this->at(u->filename + ":" + u->name);
vec->push_back(u);
}
}
Expand All @@ -95,7 +94,7 @@ bool ModSecurityTest<T>::load_test_json(std::string file) {

template <class T>
std::pair<std::string, std::vector<T *>>*
ModSecurityTest<T>::load_tests(std::string path) {
ModSecurityTest<T>::load_tests(const std::string &path) {
DIR *dir;
struct dirent *ent;
struct stat buffer;
Expand Down
4 changes: 2 additions & 2 deletions test/common/modsecurity_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ template <class T> class ModSecurityTest :
std::string header();
void cmd_options(int, char **);
std::pair<std::string, std::vector<T *>>* load_tests();
std::pair<std::string, std::vector<T *>>* load_tests(std::string path);
bool load_test_json(std::string);
std::pair<std::string, std::vector<T *>>* load_tests(const std::string &path);
bool load_test_json(const std::string &file);

std::string target;
bool verbose = false;
Expand Down
38 changes: 0 additions & 38 deletions test/cppcheck_suppressions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,48 +25,11 @@
shiftNegative:src/utils/msc_tree.cc
*:src/utils/acmp.cc
*:src/utils/msc_tree.cc
invalidScanfArgType_int:src/rules_set_properties.cc:101
invalidScanfArgType_int:src/rules_set_properties.cc:102


//
// ModSecurity v3 code...
//
unmatchedSuppression:src/utils/geo_lookup.cc:82
useInitializationList:src/utils/shared_files.h:87
unmatchedSuppression:src/utils/msc_tree.cc
functionStatic:headers/modsecurity/transaction.h:408
duplicateBranch:src/audit_log/audit_log.cc:226
unreadVariable:src/request_body_processor/multipart.cc:435
stlcstrParam:src/audit_log/writer/parallel.cc:145
eduar-hte marked this conversation as resolved.
Show resolved Hide resolved
functionStatic:src/engine/lua.h:70
functionStatic:src/engine/lua.h:71
functionConst:src/utils/geo_lookup.h:49
useInitializationList:src/operators/rbl.h:69
constStatement:test/common/modsecurity_test.cc:82
danglingTemporaryLifetime:src/modsecurity.cc:206
functionStatic:src/operators/geo_lookup.h:35
duplicateBreak:src/operators/validate_utf8_encoding.cc
syntaxError:src/transaction.cc:62
noConstructor:src/variables/variable.h:152
danglingTempReference:src/modsecurity.cc:206
knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77
knownConditionTrueFalse:src/operators/verify_svnr.cc:87
rethrowNoCurrentException:headers/modsecurity/transaction.h:313
rethrowNoCurrentException:src/rule_with_actions.cc:127
ctunullpointer:src/rule_with_actions.cc:244
ctunullpointer:src/rule_with_operator.cc:135
ctunullpointer:src/rule_with_operator.cc:95
passedByValue:test/common/modsecurity_test.cc:49
passedByValue:test/common/modsecurity_test.cc:98
unreadVariable:src/rule_with_operator.cc:219

uninitvar:src/operators/verify_cpf.cc:77
uninitvar:src/operators/verify_svnr.cc:67

functionConst:src/collection/backend/lmdb.h:86
unusedLabel:src/collection/backend/lmdb.cc:297

variableScope:src/operators/rx.cc
variableScope:src/operators/rx_global.cc

Expand Down Expand Up @@ -101,5 +64,4 @@ stlcstrStream
uselessCallsSubstr

// Examples
memleak:examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h:147
memleak:examples/using_bodies_in_chunks/simple_request.cc