Skip to content

Commit c805648

Browse files
airweenairween
authored
Merge pull request owasp-modsecurity#3134 from eduar-hte/inline-cppcheck-suppressions
Remove cppcheck suppressions with line numbers in test/cppcheck_suppressions.txt
2 parents 07e5a70 + 1f419bb commit c805648

21 files changed

+41
-87
lines changed

‎Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ cppcheck:
5959
@cppcheck -U YYSTYPE -U MBEDTLS_MD5_ALT -U MBEDTLS_SHA1_ALT \
6060
-D MS_CPPCHECK_DISABLED_FOR_PARSER -U YY_USER_INIT \
6161
--suppressions-list=./test/cppcheck_suppressions.txt \
62+
--inline-suppr \
6263
--enable=warning,style,performance,portability,unusedFunction,missingInclude \
6364
--inconclusive \
6465
--template="warning: {file},{line},{severity},{id},{message}" \

‎examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,35 +124,31 @@ class ReadingLogsViaRuleMessage {
124124
m_rules(rules)
125125
{ }
126126

127-
int process() {
127+
int process() const {
128128
pthread_t threads[NUM_THREADS];
129129
int i;
130130
struct data_ms dms;
131131
void *status;
132132

133-
modsecurity::ModSecurity *modsec;
134-
modsecurity::RulesSet *rules;
135-
136-
modsec = new modsecurity::ModSecurity();
133+
auto modsec = std::make_unique<modsecurity::ModSecurity>();
137134
modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha" \
138135
" (ModSecurity test)");
139136
modsec->setServerLogCb(logCb, modsecurity::RuleMessageLogProperty
140137
| modsecurity::IncludeFullHighlightLogProperty);
141138

142-
rules = new modsecurity::RulesSet();
139+
auto rules = std::make_unique<modsecurity::RulesSet>();
143140
if (rules->loadFromUri(m_rules.c_str()) < 0) {
144141
std::cout << "Problems loading the rules..." << std::endl;
145142
std::cout << rules->m_parserError.str() << std::endl;
146143
return -1;
147144
}
148145

149-
dms.modsec = modsec;
150-
dms.rules = rules;
146+
dms.modsec = modsec.get();
147+
dms.rules = rules.get();
151148

152149
for (i = 0; i < NUM_THREADS; i++) {
153150
pthread_create(&threads[i], NULL, process_request,
154151
reinterpret_cast<void *>(&dms));
155-
// process_request((void *)&dms);
156152
}
157153

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

165-
delete rules;
166-
delete modsec;
167161
return 0;
168162
}
169163

‎headers/modsecurity/transaction.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ class TransactionSecMarkerManagement {
310310
if (m_marker) {
311311
return m_marker;
312312
} else {
313-
throw;
313+
throw; // cppcheck-suppress rethrowNoCurrentException
314314
}
315315
}
316316

@@ -405,7 +405,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
405405
size_t getRequestBodyLength();
406406

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

‎src/audit_log/writer/parallel.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ bool Parallel::write(Transaction *transaction, int parts, std::string *error) {
118118
log = transaction->toOldAuditLogFormat(parts, "-" + boundary + "--");
119119
}
120120

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

124124
if (logPath.empty()) {

‎src/collection/backend/lmdb.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ MDB_dbi* MDBEnvProvider::GetDBI() {
659659
return &m_dbi;
660660
}
661661

662-
bool MDBEnvProvider::isValid() {
662+
bool MDBEnvProvider::isValid() const {
663663
return valid;
664664
}
665665

‎src/collection/backend/lmdb.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class MDBEnvProvider {
8383
}
8484
MDB_env* GetEnv();
8585
MDB_dbi* GetDBI();
86-
bool isValid();
86+
bool isValid() const;
8787

8888
~MDBEnvProvider();
8989
private:

‎src/engine/lua.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ class Lua {
6767
public:
6868
Lua() { }
6969

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

7474
#ifdef WITH_LUA

‎src/modsecurity.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ void ModSecurity::serverLog(void *data, std::shared_ptr<RuleMessage> rm) {
202202
}
203203

204204
if (m_logProperties & TextLogProperty) {
205-
std::string &&d = rm->log();
205+
auto d = rm->log();
206206
const void *a = static_cast<const void *>(d.c_str());
207207
m_logCb(data, a);
208208
return;

‎src/operators/geo_lookup.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class GeoLookup : public Operator {
3232
bool evaluate(Transaction *transaction, const std::string &exp) override;
3333

3434
protected:
35+
// cppcheck-suppress functionStatic
3536
bool debug(Transaction *transaction, int x, const std::string &a) {
3637
ms_dbg_a(transaction, x, a);
3738
return true;

‎src/operators/rbl.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ class Rbl : public Operator {
6666
m_demandsPassword(false),
6767
m_provider(RblProvider::UnknownProvider),
6868
Operator("Rbl", std::move(param)) {
69-
m_service = m_string->evaluate();
70-
if (m_service.find("httpbl.org") != std::string::npos) {
71-
m_demandsPassword = true;
72-
m_provider = RblProvider::httpbl;
69+
m_service = m_string->evaluate(); // cppcheck-suppress useInitializationList
70+
if (m_service.find("httpbl.org") != std::string::npos)
71+
{
72+
m_demandsPassword = true;
73+
m_provider = RblProvider::httpbl;
7374
} else if (m_service.find("uribl.com") != std::string::npos) {
7475
m_provider = RblProvider::uribl;
7576
} else if (m_service.find("spamhaus.org") != std::string::npos) {

‎src/operators/validate_url_encoding.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ bool ValidateUrlEncoding::evaluate(Transaction *transaction, RuleWithActions *ru
7474
bool res = false;
7575

7676
if (input.empty() == true) {
77-
return res;
77+
return res; // cppcheck-suppress knownConditionTrueFalse
7878
}
7979

8080
int rc = validate_url_encoding(input.c_str(), input.size(), &offset);

‎src/operators/validate_utf8_encoding.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
132132
std::to_string(i) + "\"]");
133133
}
134134
return true;
135-
break;
136135
case UNICODE_ERROR_INVALID_ENCODING :
137136
if (transaction) {
138137
ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: "
@@ -142,7 +141,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
142141
logOffset(ruleMessage, i, str.size());
143142
}
144143
return true;
145-
break;
146144
case UNICODE_ERROR_OVERLONG_CHARACTER :
147145
if (transaction) {
148146
ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: "
@@ -152,7 +150,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
152150
logOffset(ruleMessage, i, str.size());
153151
}
154152
return true;
155-
break;
156153
case UNICODE_ERROR_RESTRICTED_CHARACTER :
157154
if (transaction) {
158155
ms_dbg_a(transaction, 8, "Invalid UTF-8 encoding: "
@@ -162,7 +159,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
162159
logOffset(ruleMessage, i, str.size());
163160
}
164161
return true;
165-
break;
166162
case UNICODE_ERROR_DECODING_ERROR :
167163
if (transaction) {
168164
ms_dbg_a(transaction, 8, "Error validating UTF-8 decoding "
@@ -171,7 +167,6 @@ bool ValidateUtf8Encoding::evaluate(Transaction *transaction, RuleWithActions *r
171167
logOffset(ruleMessage, i, str.size());
172168
}
173169
return true;
174-
break;
175170
}
176171

177172
if (rc <= 0) {

‎src/operators/verify_cpf.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ bool VerifyCPF::verify(const char *cpfnumber, int len) {
7474
c = cpf_len;
7575

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

8080
factor = (sum % cpf_len);

‎src/operators/verify_svnr.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ bool VerifySVNR::verify(const char *svnrnumber, int len) {
6464
}
6565
//Laufnummer mit 3, 7, 9
6666
//Geburtsdatum mit 5, 8, 4, 2, 1, 6
67-
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;
67+
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
6868
sum %= 11;
6969
if(sum == 10){
7070
sum = 0;
@@ -84,7 +84,7 @@ bool VerifySVNR::evaluate(Transaction *t, RuleWithActions *rule,
8484
int i;
8585

8686
if (m_param.empty()) {
87-
return is_svnr;
87+
return is_svnr; // cppcheck-suppress knownConditionTrueFalse
8888
}
8989

9090
for (i = 0; i < input.size() - 1 && is_svnr == false; i++) {

‎src/rule_with_actions.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ RuleWithActions::RuleWithActions(
124124
delete a;
125125
std::cout << "General failure, action: " << a->m_name;
126126
std::cout << " has an unknown type." << std::endl;
127-
throw;
127+
throw; // cppcheck-suppress rethrowNoCurrentException
128128
}
129129
}
130130
delete actions;
@@ -241,7 +241,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans,
241241
bool containsBlock, std::shared_ptr<RuleMessage> ruleMessage) {
242242
bool disruptiveAlreadyExecuted = false;
243243

244-
for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) {
244+
for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) { // cppcheck-suppress ctunullpointer
245245
if (a.get()->action_kind != actions::Action::RunTimeOnlyIfMatchKind) {
246246
continue;
247247
}

‎src/rule_with_operator.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ void RuleWithOperator::updateMatchedVars(Transaction *trans, const std::string &
9292

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

133134
void RuleWithOperator::getVariablesExceptions(Transaction *t,
134135
variables::Variables *exclusion, variables::Variables *addition) {
135-
for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) {
136+
for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer
136137
if (containsTag(*a.first.get(), t) == false) {
137138
continue;
138139
}
@@ -146,7 +147,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t,
146147
}
147148
}
148149

149-
for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) {
150+
for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) {
150151
if (containsMsg(*a.first.get(), t) == false) {
151152
continue;
152153
}
@@ -160,7 +161,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t,
160161
}
161162
}
162163

163-
for (auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) {
164+
for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) {
164165
if (m_ruleId != a.first) {
165166
continue;
166167
}

‎src/rules_set_properties.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ void ConfigUnicodeMap::loadConfig(std::string f, double configCodePage,
9898

9999
if (mapping != NULL) {
100100
ucode = strtok_r(mapping, ":", &hmap);
101-
sscanf(ucode, "%x", &code);
102-
sscanf(hmap, "%x", &Map);
101+
sscanf(ucode, "%x", &code); // cppcheck-suppress invalidScanfArgType_int
102+
sscanf(hmap, "%x", &Map); // cppcheck-suppress invalidScanfArgType_int
103103
if (code >= 0 && code <= 65535) {
104104
driver->m_unicodeMapTable.m_unicodeMapTable->change(code, Map);
105105
}

‎src/utils/shared_files.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class SharedFiles {
8484
bool toBeCreated(false);
8585
bool err = false;
8686

87-
m_memKeyStructure = ftok(".", 1);
87+
m_memKeyStructure = ftok(".", 1); // cppcheck-suppress useInitializationList
8888
if (m_memKeyStructure < 0) {
8989
err = true;
9090
goto err_mem_key;

‎test/common/modsecurity_test.cc

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ std::string ModSecurityTest<T>::header() {
4646
}
4747

4848
template <class T>
49-
bool ModSecurityTest<T>::load_test_json(std::string file) {
49+
bool ModSecurityTest<T>::load_test_json(const std::string &file) {
5050
char errbuf[1024];
5151
yajl_val node;
5252

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

7878
if (this->count(u->filename + ":" + u->name) == 0) {
79-
std::vector<T *> *vector = new std::vector<T *>;
80-
vector->push_back(u);
79+
auto vec = new std::vector<T *>;
80+
vec->push_back(u);
8181
std::string filename(u->filename + ":" + u->name);
82-
std::pair<std::string, std::vector<T*>*> a(filename, vector);
83-
this->insert(a);
82+
this->insert({filename, vec});
8483
} else {
85-
std::vector<T *> *vec = this->at(u->filename + ":" + u->name);
84+
auto vec = this->at(u->filename + ":" + u->name);
8685
vec->push_back(u);
8786
}
8887
}
@@ -95,7 +94,7 @@ bool ModSecurityTest<T>::load_test_json(std::string file) {
9594

9695
template <class T>
9796
std::pair<std::string, std::vector<T *>>*
98-
ModSecurityTest<T>::load_tests(std::string path) {
97+
ModSecurityTest<T>::load_tests(const std::string &path) {
9998
DIR *dir;
10099
struct dirent *ent;
101100
struct stat buffer;

‎test/common/modsecurity_test.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ template <class T> class ModSecurityTest :
3939
std::string header();
4040
void cmd_options(int, char **);
4141
std::pair<std::string, std::vector<T *>>* load_tests();
42-
std::pair<std::string, std::vector<T *>>* load_tests(std::string path);
43-
bool load_test_json(std::string);
42+
std::pair<std::string, std::vector<T *>>* load_tests(const std::string &path);
43+
bool load_test_json(const std::string &file);
4444

4545
std::string target;
4646
bool verbose = false;

‎test/cppcheck_suppressions.txt

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -25,48 +25,11 @@
2525
shiftNegative:src/utils/msc_tree.cc
2626
*:src/utils/acmp.cc
2727
*:src/utils/msc_tree.cc
28-
invalidScanfArgType_int:src/rules_set_properties.cc:101
29-
invalidScanfArgType_int:src/rules_set_properties.cc:102
3028

3129

3230
//
3331
// ModSecurity v3 code...
3432
//
35-
unmatchedSuppression:src/utils/geo_lookup.cc:82
36-
useInitializationList:src/utils/shared_files.h:87
37-
unmatchedSuppression:src/utils/msc_tree.cc
38-
functionStatic:headers/modsecurity/transaction.h:408
39-
duplicateBranch:src/audit_log/audit_log.cc:226
40-
unreadVariable:src/request_body_processor/multipart.cc:435
41-
stlcstrParam:src/audit_log/writer/parallel.cc:145
42-
functionStatic:src/engine/lua.h:70
43-
functionStatic:src/engine/lua.h:71
44-
functionConst:src/utils/geo_lookup.h:49
45-
useInitializationList:src/operators/rbl.h:69
46-
constStatement:test/common/modsecurity_test.cc:82
47-
danglingTemporaryLifetime:src/modsecurity.cc:206
48-
functionStatic:src/operators/geo_lookup.h:35
49-
duplicateBreak:src/operators/validate_utf8_encoding.cc
50-
syntaxError:src/transaction.cc:62
51-
noConstructor:src/variables/variable.h:152
52-
danglingTempReference:src/modsecurity.cc:206
53-
knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77
54-
knownConditionTrueFalse:src/operators/verify_svnr.cc:87
55-
rethrowNoCurrentException:headers/modsecurity/transaction.h:313
56-
rethrowNoCurrentException:src/rule_with_actions.cc:127
57-
ctunullpointer:src/rule_with_actions.cc:244
58-
ctunullpointer:src/rule_with_operator.cc:135
59-
ctunullpointer:src/rule_with_operator.cc:95
60-
passedByValue:test/common/modsecurity_test.cc:49
61-
passedByValue:test/common/modsecurity_test.cc:98
62-
unreadVariable:src/rule_with_operator.cc:219
63-
64-
uninitvar:src/operators/verify_cpf.cc:77
65-
uninitvar:src/operators/verify_svnr.cc:67
66-
67-
functionConst:src/collection/backend/lmdb.h:86
68-
unusedLabel:src/collection/backend/lmdb.cc:297
69-
7033
variableScope:src/operators/rx.cc
7134
variableScope:src/operators/rx_global.cc
7235

@@ -101,5 +64,4 @@ stlcstrStream
10164
uselessCallsSubstr
10265

10366
// Examples
104-
memleak:examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h:147
10567
memleak:examples/using_bodies_in_chunks/simple_request.cc

0 commit comments

Comments
 (0)