From 96849c07dec7989669e70675f17c40e504b440cf Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Thu, 14 Feb 2019 23:20:32 -0300 Subject: [PATCH] Makes action name a shared pointer --- headers/modsecurity/actions/action.h | 10 ++++---- src/parser/seclang-parser.yy | 2 +- src/rule.cc | 37 ++++++++++++++-------------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/headers/modsecurity/actions/action.h b/headers/modsecurity/actions/action.h index 9fb4398b..db0d2540 100644 --- a/headers/modsecurity/actions/action.h +++ b/headers/modsecurity/actions/action.h @@ -42,7 +42,7 @@ class Action { : m_isNone(false), temporaryAction(false), action_kind(2), - m_name(""), + m_name(nullptr), m_parser_payload("") { set_name_and_payload(_action); } @@ -50,7 +50,7 @@ class Action { : m_isNone(false), temporaryAction(false), action_kind(kind), - m_name(""), + m_name(nullptr), m_parser_payload("") { set_name_and_payload(_action); } @@ -77,11 +77,11 @@ class Action { } if (pos == std::string::npos) { - m_name = data; + m_name = std::shared_ptr(new std::string(data)); return; } - m_name = std::string(data, 0, pos); + m_name = std::shared_ptr(new std::string(data, 0, pos)); m_parser_payload = std::string(data, pos + 1, data.length()); if (m_parser_payload.at(0) == '\'' && m_parser_payload.size() > 2) { @@ -93,7 +93,7 @@ class Action { bool m_isNone; bool temporaryAction; int action_kind; - std::string m_name; + std::shared_ptr m_name; std::string m_parser_payload; /** diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index f5273c0d..cf050523 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -1171,7 +1171,7 @@ expression: } checkedActions.push_back(a); } else { - driver.error(@0, "The action '" + a->m_name + "' is not suitable to be part of the SecDefaultActions"); + driver.error(@0, "The action '" + *a->m_name.get() + "' is not suitable to be part of the SecDefaultActions"); YYERROR; } } diff --git a/src/rule.cc b/src/rule.cc index b9efefa7..53719f02 100644 --- a/src/rule.cc +++ b/src/rule.cc @@ -262,7 +262,7 @@ void Rule::executeActionsIndependentOfChainedRuleResult(Transaction *trans, for (actions::SetVar *a : m_actionsSetVar) { ms_dbg_a(trans, 4, "Running [independent] (non-disruptive) " \ - "action: " + a->m_name); + "action: " + *a->m_name.get()); a->evaluate(this, trans); } @@ -273,12 +273,12 @@ void Rule::executeActionsIndependentOfChainedRuleResult(Transaction *trans, continue; } actions::Action *a = dynamic_cast(b.second.get()); - if (a->isDisruptive() == true && a->m_name == "block") { + if (a->isDisruptive() == true && *a->m_name.get() == "block") { ms_dbg_a(trans, 9, "Rule contains a `block' action"); *containsBlock = true; - } else if (a->m_name == "setvar") { + } else if (*a->m_name.get() == "setvar") { ms_dbg_a(trans, 4, "Running [independent] (non-disruptive) " \ - "action: " + a->m_name); + "action: " + *a->m_name.get()); a->evaluate(this, trans, ruleMessage); } } @@ -340,22 +340,21 @@ inline void Rule::executeTransformation(actions::Action *a, if (newValue != *oldValue) { std::shared_ptr u(new std::string(newValue)); if (m_containsMultiMatchAction) { - std::shared_ptr t(new std::string(a->m_name)); - ret->push_back(std::make_pair(u, t)); + ret->push_back(std::make_pair(u, a->m_name)); (*nth)++; } *value = u; } if (path->empty()) { - path->append(a->m_name); + path->append(*a->m_name.get()); } else { - path->append("," + a->m_name); + path->append("," + *a->m_name.get()); } ms_dbg_a(trans, 9, " T (" + \ std::to_string(*nth) + ") " + \ - a->m_name + ": \"" + \ + *a->m_name.get() + ": \"" + \ utils::string::limitTo(80, newValue) +"\""); } @@ -540,28 +539,28 @@ inline void Rule::getFinalVars(variables::Variables *vars, void Rule::executeAction(Transaction *trans, bool containsBlock, std::shared_ptr ruleMessage, Action *a, bool defaultContext) { - if (a->isDisruptive() == false && a->m_name != "block") { + if (a->isDisruptive() == false && *a->m_name.get() != "block") { ms_dbg_a(trans, 9, "Running " \ - "action: " + a->m_name); + "action: " + *a->m_name.get()); a->evaluate(this, trans, ruleMessage); return; } if (defaultContext && !containsBlock) { - ms_dbg_a(trans, 4, "Ignoring action: " + a->m_name + \ + ms_dbg_a(trans, 4, "Ignoring action: " + *a->m_name.get() + \ " (rule does not cotains block)"); return; } if (trans->getRuleEngineState() == RulesSet::EnabledRuleEngine) { - ms_dbg_a(trans, 4, "Running (disruptive) action: " + a->m_name + \ + ms_dbg_a(trans, 4, "Running (disruptive) action: " + *a->m_name.get() + \ "."); a->evaluate(this, trans, ruleMessage); return; } ms_dbg_a(trans, 4, "Not running any disruptive action (or block): " \ - + a->m_name + ". SecRuleEngine is not On."); + + *a->m_name.get() + ". SecRuleEngine is not On."); } @@ -581,7 +580,7 @@ void Rule::executeActionsAfterFullMatch(Transaction *trans, for (actions::Tag *a : this->m_actionsTag) { ms_dbg_a(trans, 4, "Running (non-disruptive) action: " \ - + a->m_name); + + *a->m_name.get()); a->evaluate(this, trans, ruleMessage); } @@ -811,12 +810,12 @@ std::vector Rule::getActionsByName(const std::string& name, Transaction *trans) { std::vector ret; for (auto &z : m_actionsRuntimePos) { - if (z->m_name == name) { + if (*z->m_name.get() == name) { ret.push_back(z); } } for (auto &z : m_actionsRuntimePre) { - if (z->m_name == name) { + if (*z->m_name.get() == name) { ret.push_back(z); } } @@ -826,7 +825,7 @@ std::vector Rule::getActionsByName(const std::string& name, continue; } actions::Action *z = dynamic_cast(b.second.get()); - if (z->m_name == name) { + if (*z->m_name.get() == name) { ret.push_back(z); } } @@ -836,7 +835,7 @@ std::vector Rule::getActionsByName(const std::string& name, continue; } actions::Action *z = dynamic_cast(b.second.get()); - if (z->m_name == name) { + if (*z->m_name.get() == name) { ret.push_back(z); } }