Skip to content

Commit

Permalink
Minor changes to RuleWithActions
Browse files Browse the repository at this point in the history 
- Made executeAction private
- Leveraged structured binding when processing ruleId/action containers.
  • Loading branch information
@eduar-hte
eduar-hte committed Oct 22, 2024
1 parent f4ef3aa commit 09df813
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 57 deletions.
23 changes: 11 additions & 12 deletions headers/modsecurity/rule_with_actions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,6 @@ class RuleWithActions : public Rule {
bool containsDisruptive,
RuleMessage &ruleMessage);

void executeAction(Transaction *trans,
bool containsBlock,
RuleMessage &ruleMessage,
actions::Action *a,
bool context);


void executeTransformations(
const Transaction *trasn, const std::string &value, TransformationResults &ret) const;

Expand All @@ -86,15 +79,15 @@ class RuleWithActions : public Rule {
inline bool isChained() const { return m_isChained == true; }
inline bool hasCaptureAction() const { return m_containsCaptureAction == true; }
inline void setChained(bool b) { m_isChained = b; }
inline bool hasDisruptiveAction() const { return m_disruptiveAction != NULL; }
inline bool hasDisruptiveAction() const { return m_disruptiveAction != nullptr; }
inline bool hasBlockAction() const { return m_containsStaticBlockAction == true; }
inline bool hasMultimatch() const { return m_containsMultiMatchAction == true; }

inline bool hasLogData() const { return m_logData != NULL; }
inline bool hasLogData() const { return m_logData != nullptr; }
std::string logData(Transaction *t);
inline bool hasMsg() const { return m_msg != NULL; }
inline bool hasMsg() const { return m_msg != nullptr; }
std::string msg(Transaction *t);
inline bool hasSeverity() const { return m_severity != NULL; }
inline bool hasSeverity() const { return m_severity != nullptr; }
int severity() const;

std::string m_rev;
Expand All @@ -109,14 +102,20 @@ class RuleWithActions : public Rule {
RuleWithActions *m_chainedRuleParent;

private:
inline void executeTransformation(
void executeTransformation(
const actions::transformations::Transformation &a,
std::string &value,
const Transaction *trans,
TransformationResults &ret,
std::string &path,
int &nth) const;

void executeAction(Transaction *trans,
bool containsBlock,
RuleMessage &ruleMessage,
actions::Action &a,
bool context);

/* actions */
actions::Action *m_disruptiveAction;
actions::LogData *m_logData;
Expand Down
2 changes: 1 addition & 1 deletion src/parser/seclang-parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2432,7 +2432,7 @@ namespace yy {
}
checkedActions.push_back(a);
} else {
driver.error(yystack_[2].location, "The action '" + a->m_name + "' is not suitable to be part of the SecDefaultActions");
driver.error(yystack_[2].location, "The action '" + *a->m_name.get() + "' is not suitable to be part of the SecDefaultActions");
YYERROR;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/parser/seclang-parser.yy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,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;
}
}
Expand Down
81 changes: 38 additions & 43 deletions src/rule_with_actions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,19 +210,18 @@ void RuleWithActions::executeActionsIndependentOfChainedRuleResult(Transaction *
a->evaluate(*this, trans);
}

for (auto &b :
for (auto &[ruleId, action] :
trans->m_rules->m_exceptions.m_action_pre_update_target_by_id) {
if (m_ruleId != b.first) {
if (m_ruleId != ruleId) {
continue;
}
actions::Action *a = b.second.get();
if (a->isDisruptive() == true && *a->m_name.get() == "block") {
if (action->isDisruptive() == true && *action->m_name.get() == "block") {
ms_dbg_a(trans, 9, "Rule contains a `block' action");
*containsBlock = true;
} else if (*a->m_name.get() == "setvar") {
} else if (*action->m_name.get() == "setvar") {
ms_dbg_a(trans, 4, fmt::format("Running [independent] (non-disruptive) " \
"action: {}", *a->m_name.get()));
a->evaluate(*this, trans, ruleMessage);
"action: {}", *action->m_name.get()));
action->evaluate(*this, trans, ruleMessage);
}
}

Expand All @@ -236,11 +235,10 @@ void RuleWithActions::executeActionsIndependentOfChainedRuleResult(Transaction *
if (m_msg) {
m_msg->evaluate(*this, trans, ruleMessage);
}
for (actions::Tag *a : m_actionsTag) {
a->evaluate(*this, trans, ruleMessage);
for (auto tag : m_actionsTag) {
tag->evaluate(*this, trans, ruleMessage);
}
}

}


Expand All @@ -249,27 +247,26 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans,
bool disruptiveAlreadyExecuted = false;

for (const auto &a : trans->m_rules->m_defaultActions[getPhase()]) { // cppcheck-suppress ctunullpointer
if (a.get()->action_kind != actions::Action::Kind::RunTimeOnlyIfMatchKind) {
if (a->action_kind != actions::Action::Kind::RunTimeOnlyIfMatchKind) {
continue;
}
if (!a.get()->isDisruptive()) {
executeAction(trans, containsBlock, ruleMessage, a.get(), true);
if (!a->isDisruptive()) {
executeAction(trans, containsBlock, ruleMessage, *a, true);
}
}

for (actions::Tag *a : this->m_actionsTag) {
for (auto tag : this->m_actionsTag) {
ms_dbg_a(trans, 4, fmt::format("Running (non-disruptive) action: {}",
*a->m_name.get()));
a->evaluate(*this, trans, ruleMessage);
*tag->m_name.get()));
tag->evaluate(*this, trans, ruleMessage);
}

for (auto &b :
for (auto &[ruleId, action] :
trans->m_rules->m_exceptions.m_action_pos_update_target_by_id) {
if (m_ruleId != b.first) {
if (m_ruleId != ruleId) {
continue;
}
actions::Action *a = b.second.get();
executeAction(trans, containsBlock, ruleMessage, a, false);
executeAction(trans, containsBlock, ruleMessage, *action, false);
disruptiveAlreadyExecuted = true;
}
if (m_severity) {
Expand All @@ -283,47 +280,47 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans,
if (m_msg) {
m_msg->evaluate(*this, trans, ruleMessage);
}
for (Action *a : this->m_actionsRuntimePos) {
for (auto a : this->m_actionsRuntimePos) {
if (!a->isDisruptive()
&& !(disruptiveAlreadyExecuted
&& dynamic_cast<actions::Block *>(a))) {
executeAction(trans, containsBlock, ruleMessage, a, false);
executeAction(trans, containsBlock, ruleMessage, *a, false);
}
}
if (!disruptiveAlreadyExecuted && m_disruptiveAction != nullptr) {
executeAction(trans, containsBlock, ruleMessage,
m_disruptiveAction, false);
*m_disruptiveAction, false);
}
}


void RuleWithActions::executeAction(Transaction *trans,
bool containsBlock, RuleMessage &ruleMessage,
Action *a, bool defaultContext) {
if (a->isDisruptive() == false && *a->m_name.get() != "block") {
Action &a, bool defaultContext) {
if (a.isDisruptive() == false && *a.m_name.get() != "block") {
ms_dbg_a(trans, 9, fmt::format("Running action: {}",
*a->m_name.get()));
a->evaluate(*this, trans, ruleMessage);
*a.m_name.get()));
a.evaluate(*this, trans, ruleMessage);
return;
}

if (defaultContext && !containsBlock) {
ms_dbg_a(trans, 4, fmt::format("Ignoring action: {}" \
" (rule does not cotains block)",
*a->m_name.get()));
*a.m_name.get()));
return;
}

if (trans->getRuleEngineState() == RulesSet::EnabledRuleEngine) {
ms_dbg_a(trans, 4, fmt::format("Running (disruptive) action: {}.",
*a->m_name.get()));
a->evaluate(*this, trans, ruleMessage);
*a.m_name.get()));
a.evaluate(*this, trans, ruleMessage);
return;
}

ms_dbg_a(trans, 4, fmt::format("Not running any disruptive action " \
"(or block): {}. SecRuleEngine is not On.",
*a->m_name.get()));
*a.m_name.get()));
}


Expand Down Expand Up @@ -460,34 +457,32 @@ bool RuleWithActions::containsMsg(const std::string& name, Transaction *t) {
std::vector<actions::Action *> RuleWithActions::getActionsByName(const std::string& name,
const Transaction *trans) {
std::vector<actions::Action *> ret;
for (auto &z : m_actionsRuntimePos) {
for (auto z : m_actionsRuntimePos) {
if (*z->m_name.get() == name) {
ret.push_back(z);
}
}
for (auto &z : m_transformations) {
for (auto z : m_transformations) {
if (*z->m_name.get() == name) {
ret.push_back(z);
}
}
for (auto &b :
for (auto &[ruleId, action] :
trans->m_rules->m_exceptions.m_action_pre_update_target_by_id) {
if (m_ruleId != b.first) {
if (m_ruleId != ruleId) {
continue;
}
actions::Action *z = b.second.get();
if (*z->m_name.get() == name) {
ret.push_back(z);
if (*action->m_name.get() == name) {
ret.push_back(action.get());
}
}
for (auto &b :
for (auto &[ruleId, action] :
trans->m_rules->m_exceptions.m_action_pos_update_target_by_id) {
if (m_ruleId != b.first) {
if (m_ruleId != ruleId) {
continue;
}
actions::Action *z = b.second.get();
if (*z->m_name.get() == name) {
ret.push_back(z);
if (*action->m_name.get() == name) {
ret.push_back(action.get());
}
}
return ret;
Expand Down

0 comments on commit 09df813

Please sign in to comment.