Using performLogging function

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> ruleMessage,
+        bool lastLog = true,
+        bool chainedParentNull = false);
+
     std::vector<actions::Action *> getActionsByName(const std::string& name,
         Transaction *t);
     bool containsTag(const std::string& name, Transaction *t);

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. */
+    performLogging(trans, ruleMessage);
-    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);
-        }
-    }
 
     return true;
 }

src/rule_with_actions.cc

@@ -474,6 +474,72 @@ std::vector<actions::Action *> RuleWithActions::getActionsByName(const std::stri
     return ret;
 }
 
+void RuleWithActions::performLogging(Transaction *trans,
+    std::shared_ptr<RuleMessage> 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); }

src/rule_with_operator.cc

@@ -325,20 +325,7 @@ bool RuleWithOperator::evaluate(Transaction *trans,
                     executeActionsIndependentOfChainedRuleResult(trans,
                         &containsBlock, ruleMessage);
 
-                    bool isItToBeLogged = ruleMessage->m_saveMessage;
+                    performLogging(trans, ruleMessage, false);
-                    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);
-                    }
 
                     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));
+    performLogging(trans, ruleMessage, true, true);
-    if (isItToBeLogged && !hasMultimatch()) {
-        /* warn */
-        trans->m_rulesMessages.push_back(*ruleMessage);
-
-        /* error */
-        if (!ruleMessage->m_isDisruptive) {
-            trans->serverLog(ruleMessage);
-        }
-	}
     return true;
 }