From 7e0bc2691727b8c75f74638cdc4d1c45a689a7b6 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 12 Mar 2019 13:42:06 -0300 Subject: [PATCH] Using performLogging function --- headers/modsecurity/rule_with_actions.h | 7 ++- src/rule_unconditional.cc | 23 +-------- src/rule_with_actions.cc | 66 +++++++++++++++++++++++++ src/rule_with_operator.cc | 26 +--------- 4 files changed, 76 insertions(+), 46 deletions(-) diff --git a/headers/modsecurity/rule_with_actions.h b/headers/modsecurity/rule_with_actions.h index 5cd12e56..34004838 100644 --- a/headers/modsecurity/rule_with_actions.h +++ b/headers/modsecurity/rule_with_actions.h @@ -79,6 +79,11 @@ class RuleWithActions : public Rule { int *nth) const; + void performLogging(Transaction *trans, + std::shared_ptr ruleMessage, + bool lastLog = true, + bool chainedParentNull = false); + std::vector getActionsByName(const std::string& name, Transaction *t); bool containsTag(const std::string& name, Transaction *t); @@ -132,4 +137,4 @@ class RuleWithActions : public Rule { #endif -#endif // HEADERS_MODSECURITY_RULE_WITH_ACTIONS_H_ +#endif // HEADERS_MODSECURITY_RULE_WITH_ACTIONS_H_ \ No newline at end of file diff --git a/src/rule_unconditional.cc b/src/rule_unconditional.cc index 2e61428f..17532c67 100644 --- a/src/rule_unconditional.cc +++ b/src/rule_unconditional.cc @@ -14,7 +14,7 @@ */ #include "modsecurity/rule_unconditional.h" -#include "modsecurity/rule_message.h" + namespace modsecurity { @@ -34,26 +34,7 @@ bool RuleUnconditional::evaluate(Transaction *trans, executeActionsAfterFullMatch(trans, containsBlock, ruleMessage); - /* last rule in the chain. */ - bool isItToBeLogged = ruleMessage->m_saveMessage; - if (isItToBeLogged && !hasMultimatch() - && !ruleMessage->m_message.empty()) { - /* warn */ - trans->m_rulesMessages.push_back(*ruleMessage); - - /* error */ - if (!ruleMessage->m_isDisruptive) { - trans->serverLog(ruleMessage); - } - } - else if (hasBlockAction() && !hasMultimatch()) { - /* warn */ - trans->m_rulesMessages.push_back(*ruleMessage); - /* error */ - if (!ruleMessage->m_isDisruptive) { - trans->serverLog(ruleMessage); - } - } + performLogging(trans, ruleMessage); return true; } diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc index 1c6b7fb2..a2d7306e 100644 --- a/src/rule_with_actions.cc +++ b/src/rule_with_actions.cc @@ -474,6 +474,72 @@ std::vector RuleWithActions::getActionsByName(const std::stri return ret; } +void RuleWithActions::performLogging(Transaction *trans, + std::shared_ptr ruleMessage, + bool lastLog, + bool chainedParentNull) { + + /* last rule in the chain. */ + bool isItToBeLogged = ruleMessage->m_saveMessage; + + /** + * + * RuleMessage is stacked allocated for the rule execution, + * anything beyond this may lead to invalid pointer access. + * + * In case of a warning, o set of messages is saved to be read + * at audit log generation. Therefore demands a copy here. + * + * FIXME: Study an way to avoid the copy. + * + **/ + if (lastLog) { + if (chainedParentNull) { + isItToBeLogged = (ruleMessage->m_saveMessage && (m_chainedRuleParent == nullptr)); + if (isItToBeLogged && !hasMultimatch()) { + /* warn */ + trans->m_rulesMessages.push_back(*ruleMessage); + + /* error */ + if (!ruleMessage->m_isDisruptive) { + trans->serverLog(ruleMessage); + } + } + } else if (hasBlockAction() && !hasMultimatch()) { + /* warn */ + trans->m_rulesMessages.push_back(*ruleMessage); + /* error */ + if (!ruleMessage->m_isDisruptive) { + trans->serverLog(ruleMessage); + } + } else { + if (isItToBeLogged && !hasMultimatch() + && !ruleMessage->m_message.empty()) { + /* warn */ + trans->m_rulesMessages.push_back(*ruleMessage); + + /* error */ + if (!ruleMessage->m_isDisruptive) { + trans->serverLog(ruleMessage); + } + } + } + } else { + if (hasMultimatch() && isItToBeLogged) { + /* warn */ + trans->m_rulesMessages.push_back(*ruleMessage.get()); + + /* error */ + if (!ruleMessage->m_isDisruptive) { + trans->serverLog(ruleMessage); + } + + RuleMessage *rm = new RuleMessage(this, trans); + rm->m_saveMessage = ruleMessage->m_saveMessage; + ruleMessage.reset(rm); + } + } +} std::string RuleWithActions::logData(Transaction *t) { return m_logData->data(t); } std::string RuleWithActions::msg(Transaction *t) { return m_msg->data(t); } diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index 2dd590c8..e4ab6a57 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -325,20 +325,7 @@ bool RuleWithOperator::evaluate(Transaction *trans, executeActionsIndependentOfChainedRuleResult(trans, &containsBlock, ruleMessage); - bool isItToBeLogged = ruleMessage->m_saveMessage; - if (hasMultimatch() && isItToBeLogged) { - /* warn */ - trans->m_rulesMessages.push_back(*ruleMessage); - - /* error */ - if (!ruleMessage->m_isDisruptive) { - trans->serverLog(ruleMessage); - } - - RuleMessage *rm = new RuleMessage(this, trans); - rm->m_saveMessage = ruleMessage->m_saveMessage; - ruleMessage.reset(rm); - } + performLogging(trans, ruleMessage, false); globalRet = true; } @@ -382,16 +369,7 @@ end_exec: executeActionsAfterFullMatch(trans, containsBlock, ruleMessage); /* last rule in the chain. */ - bool isItToBeLogged = (ruleMessage->m_saveMessage && (m_chainedRuleParent == nullptr)); - if (isItToBeLogged && !hasMultimatch()) { - /* warn */ - trans->m_rulesMessages.push_back(*ruleMessage); - - /* error */ - if (!ruleMessage->m_isDisruptive) { - trans->serverLog(ruleMessage); - } - } + performLogging(trans, ruleMessage, true, true); return true; }