github_mirrors/ModSecurity
3
0
Fork 1
mirror of https://github.com/owasp-modsecurity/ModSecurity.git synced 2025-09-29 19:24:29 +03:00
Code Issues Packages Projects Releases Wiki Activity

Initialize variable in if statement to avoid doing dynamic_cast twice

Browse Source 
- Refactored duplicate code in RuleWithOperator::getVariablesExceptions
- Leveraged auto to simplify declaration of dynamic_cast pointers.
This commit is contained in:
Eduardo Arias
2024-08-08 12:49:48 -07:00
parent 18378c10f8
commit c917d6a2dc
7 changed files with 839 additions and 843 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
headers/modsecurity/rule_with_operator.h
View File

@@ -50,7 +50,7 @@ class RuleWithOperator : public RuleWithActions {
bool evaluate(Transaction *transaction, bool evaluate(Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override; std::shared_ptr<RuleMessage> rm) override;
void getVariablesExceptions(Transaction *t, void getVariablesExceptions(Transaction &t,
variables::Variables *exclusion, variables::Variables *addition); variables::Variables *exclusion, variables::Variables *addition);
inline void getFinalVars(variables::Variables *vars, inline void getFinalVars(variables::Variables *vars,
variables::Variables *eclusion, Transaction *trans); variables::Variables *eclusion, Transaction *trans);

18
src/actions/set_var.cc
View File

@@ -51,18 +51,12 @@ bool SetVar::evaluate(RuleWithActions *rule, Transaction *t) {
std::string m_variableNameExpanded; std::string m_variableNameExpanded;
auto *v = m_variable.get(); auto *v = m_variable.get();
variables::Tx_DynamicElement *tx = dynamic_cast< auto tx = dynamic_cast<variables::Tx_DynamicElement *> (v);
variables::Tx_DynamicElement *> (v); auto session = dynamic_cast<variables::Session_DynamicElement *> (v);
variables::Session_DynamicElement *session = dynamic_cast< auto ip = dynamic_cast<variables::Ip_DynamicElement *> (v);
variables::Session_DynamicElement *> (v); auto resource = dynamic_cast<variables::Resource_DynamicElement *> (v);
variables::Ip_DynamicElement *ip = dynamic_cast< auto global = dynamic_cast<variables::Global_DynamicElement *> (v);
variables::Ip_DynamicElement *> (v); auto user = dynamic_cast<variables::User_DynamicElement *> (v);
variables::Resource_DynamicElement *resource = dynamic_cast<
variables::Resource_DynamicElement *> (v);
variables::Global_DynamicElement *global = dynamic_cast<
variables::Global_DynamicElement *> (v);
variables::User_DynamicElement *user = dynamic_cast<
variables::User_DynamicElement *> (v);
if (tx) { if (tx) {
m_variableNameExpanded = tx->m_string->evaluate(t, rule); m_variableNameExpanded = tx->m_string->evaluate(t, rule);
} else if (session) { } else if (session) {

1567
src/parser/seclang-parser.cc
View File

File diff suppressed because it is too large Load Diff

15
src/parser/seclang-parser.yy
View File

@@ -1086,8 +1086,9 @@ expression:
std::vector<actions::Action *> *a = new std::vector<actions::Action *>(); std::vector<actions::Action *> *a = new std::vector<actions::Action *>();
std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>(); std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>();
for (auto &i : *$4.get()) { for (auto &i : *$4.get()) {
if (dynamic_cast<actions::transformations::Transformation *>(i.get())) { if (auto pt = dynamic_cast<actions::transformations::Transformation *>(i.get())) {
t->push_back(dynamic_cast<actions::transformations::Transformation *>(i.release())); t->push_back(pt);
i.release();
} else { } else {
a->push_back(i.release()); a->push_back(i.release());
} }
@@ -1135,8 +1136,9 @@ expression:
std::vector<actions::Action *> *a = new std::vector<actions::Action *>(); std::vector<actions::Action *> *a = new std::vector<actions::Action *>();
std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>(); std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>();
for (auto &i : *$2.get()) { for (auto &i : *$2.get()) {
if (dynamic_cast<actions::transformations::Transformation *>(i.get())) { if (auto pt = dynamic_cast<actions::transformations::Transformation *>(i.get())) {
t->push_back(dynamic_cast<actions::transformations::Transformation *>(i.release())); t->push_back(pt);
i.release();
} else { } else {
a->push_back(i.release()); a->push_back(i.release());
} }
@@ -1155,8 +1157,9 @@ expression:
std::vector<actions::Action *> *a = new std::vector<actions::Action *>(); std::vector<actions::Action *> *a = new std::vector<actions::Action *>();
std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>(); std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>();
for (auto &i : *$2.get()) { for (auto &i : *$2.get()) {
if (dynamic_cast<actions::transformations::Transformation *>(i.get())) { if (auto pt = dynamic_cast<actions::transformations::Transformation *>(i.get())) {
t->push_back(dynamic_cast<actions::transformations::Transformation *>(i.release())); t->push_back(pt);
i.release();
} else { } else {
a->push_back(i.release()); a->push_back(i.release());
} }

21
src/rule_with_actions.cc
View File

@@ -99,17 +99,16 @@ RuleWithActions::RuleWithActions(
} else if (dynamic_cast<actions::MultiMatch *>(a)) { } else if (dynamic_cast<actions::MultiMatch *>(a)) {
m_containsMultiMatchAction = true; m_containsMultiMatchAction = true;
delete a; delete a;
} else if (dynamic_cast<actions::Severity *>(a)) { } else if (auto sa = dynamic_cast<actions::Severity *>(a)) {
m_severity = dynamic_cast<actions::Severity *>(a); m_severity = sa;
} else if (dynamic_cast<actions::LogData *>(a)) { } else if (auto lda = dynamic_cast<actions::LogData *>(a)) {
m_logData = dynamic_cast<actions::LogData*>(a); m_logData = lda;
} else if (dynamic_cast<actions::Msg *>(a)) { } else if (auto ma = dynamic_cast<actions::Msg *>(a)) {
m_msg = dynamic_cast<actions::Msg*>(a); m_msg = ma;
} else if (dynamic_cast<actions::SetVar *>(a)) { } else if (auto sva = dynamic_cast<actions::SetVar *>(a)) {
m_actionsSetVar.push_back( m_actionsSetVar.push_back(sva);
dynamic_cast<actions::SetVar *>(a)); } else if (auto ta = dynamic_cast<actions::Tag *>(a)) {
} else if (dynamic_cast<actions::Tag *>(a)) { m_actionsTag.push_back(ta);
m_actionsTag.push_back(dynamic_cast<actions::Tag *>(a));
} else if (dynamic_cast<actions::Block *>(a)) { } else if (dynamic_cast<actions::Block *>(a)) {
m_actionsRuntimePos.push_back(a); m_actionsRuntimePos.push_back(a);
m_containsStaticBlockAction = true; m_containsStaticBlockAction = true;

53
src/rule_with_operator.cc
View File

@@ -131,32 +131,15 @@ bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string &
} }
void RuleWithOperator::getVariablesExceptions(Transaction *t, template<typename MapType, typename Operation>
variables::Variables *exclusion, variables::Variables *addition) { void getVariablesExceptionsHelper(
for (const auto &[tag, v] : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer variables::Variables *exclusion, variables::Variables *addition,
if (containsTag(*tag.get(), t)) { const MapType &map, Operation op) {
if (Variable *b{v.get()};dynamic_cast<variables::VariableModificatorExclusion*>(b)) { for (const auto &[x, v] : map) {
exclusion->push_back(dynamic_cast<variables::VariableModificatorExclusion*>(b)->m_base.get()); if (op(x)) {
} else { auto b = v.get();
addition->push_back(b); if (auto vme = dynamic_cast<variables::VariableModificatorExclusion*>(b)) {
} exclusion->push_back(vme->m_base.get());
}
}
for (const auto &[msg, v] : t->m_rules->m_exceptions.m_variable_update_target_by_msg) {
if (containsMsg(*msg.get(), t)) {
if (Variable *b{v.get()}; dynamic_cast<variables::VariableModificatorExclusion *>(b)) {
exclusion->push_back(dynamic_cast<variables::VariableModificatorExclusion *>(b)->m_base.get());
} else {
addition->push_back(b);
}
}
}
for (const auto &[id, v] : t->m_rules->m_exceptions.m_variable_update_target_by_id) { // cppcheck-suppress unassignedVariable
if (m_ruleId == id) {
if (Variable *b{v.get()};dynamic_cast<variables::VariableModificatorExclusion *>(b)) {
exclusion->push_back(dynamic_cast<variables::VariableModificatorExclusion *>(b)->m_base.get());
} else { } else {
addition->push_back(b); addition->push_back(b);
} }
@@ -165,10 +148,26 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t,
} }
void RuleWithOperator::getVariablesExceptions(Transaction &t,
variables::Variables *exclusion, variables::Variables *addition) {
getVariablesExceptionsHelper(exclusion, addition,
t.m_rules->m_exceptions.m_variable_update_target_by_tag,
[this, &t](const auto &tag) { return containsTag(*tag.get(), &t); });
getVariablesExceptionsHelper(exclusion, addition,
t.m_rules->m_exceptions.m_variable_update_target_by_msg,
[this, &t](const auto &msg) { return containsMsg(*msg.get(), &t); });
getVariablesExceptionsHelper(exclusion, addition,
t.m_rules->m_exceptions.m_variable_update_target_by_id,
[this](const auto &id) { return m_ruleId == id; });
}
inline void RuleWithOperator::getFinalVars(variables::Variables *vars, inline void RuleWithOperator::getFinalVars(variables::Variables *vars,
variables::Variables *exclusion, Transaction *trans) { variables::Variables *exclusion, Transaction *trans) {
variables::Variables addition; variables::Variables addition;
getVariablesExceptions(trans, exclusion, &addition); getVariablesExceptions(*trans, exclusion, &addition); // cppcheck-suppress ctunullpointer
for (int i = 0; i < m_variables->size(); i++) { for (int i = 0; i < m_variables->size(); i++) {
Variable *variable = m_variables->at(i); Variable *variable = m_variables->at(i);

6
test/optimization/optimization.cc
View File

@@ -87,7 +87,7 @@ int main(int argc, char **argv) {
continue; continue;
} }
if (dynamic_cast<modsecurity::RuleUnconditional *>(z.get()) != nullptr) { if (dynamic_cast<modsecurity::RuleUnconditional *>(z.get())) {
std::string op = "Unconditional"; std::string op = "Unconditional";
if (operators.count(op) > 0) { if (operators.count(op) > 0) {
operators[op] = 1 + operators[op]; operators[op] = 1 + operators[op];
@@ -96,9 +96,7 @@ int main(int argc, char **argv) {
} }
} }
if (dynamic_cast<modsecurity::RuleWithOperator *>(z.get()) != nullptr) { if (auto rwo = dynamic_cast<modsecurity::RuleWithOperator *>(z.get())) {
auto *rwo = dynamic_cast<modsecurity::RuleWithOperator *>(z.get());
std::string op = rwo->getOperatorName(); std::string op = rwo->getOperatorName();
if (operators.count(op) > 0) { if (operators.count(op) > 0) {
operators[op] = 1 + operators[op]; operators[op] = 1 + operators[op];