Refactoring: how to report to error logs

headers/modsecurity/actions/action.h

@@ -67,7 +67,6 @@ class Action {
     }
     virtual bool init(std::string *error) { return true; }
     virtual bool isDisruptive() { return false; }
-    virtual void fillIntervention(ModSecurityIntervention *intervention);
     static Action *instantiate(const std::string& name);
 
 

headers/modsecurity/rule_message.h

@@ -18,6 +18,7 @@
 #include <vector>
 #include <string>
 #include <list>
+#include <cstring>
 #endif
 
 #ifndef HEADERS_MODSECURITY_RULE_MESSAGE_H_
@@ -50,6 +51,7 @@ class RuleMessage {
     { }
 
     std::string errorLog(Transaction *trans);
+    std::string disruptiveErrorLog(Transaction *trans, std::string log2);
 
 
     std::string m_match;
@@ -65,7 +67,6 @@ class RuleMessage {
     int m_accuracy;
 
     std::list<std::string> m_tags;
-    std::vector<actions::Action *> m_tmp_actions;
     std::list<std::string> m_server_logs;
 
     Rule *m_rule;

headers/modsecurity/transaction.h

@@ -73,8 +73,10 @@ class Rules;
 class RuleMessage;
 namespace actions {
 class Action;
+namespace disruptive {
 enum AllowType : int;
 }
+}
 namespace RequestBodyProcessor {
 class XML;
 class JSON;
@@ -339,7 +341,7 @@ class Transaction {
     /**
      * If allow action was utilized, this variable holds the allow type.
      */
-    modsecurity::actions::AllowType m_allowType;
+    modsecurity::actions::disruptive::AllowType m_allowType;
 
     /**
      * Holds the decode URI. Notice that m_uri holds the raw version
@@ -351,7 +353,8 @@ class Transaction {
      * Actions (disruptive?) that should be taken by the connector related to
      * that transaction.
      */
-    std::vector<actions::Action *> m_actions;
+    std::vector<ModSecurityIntervention> m_actions;
+    ModSecurityIntervention m_it;
 
     /**
      * Holds the creation time stamp, using std::time.

src/Makefile.am

@@ -86,9 +86,7 @@ VARIABLES = \
 ACTIONS = \
 	actions/accuracy.cc \
 	actions/action.cc \
-	actions/allow.cc \
 	actions/audit_log.cc \
-	actions/block.cc \
 	actions/capture.cc \
 	actions/chain.cc \
 	actions/ctl/audit_log_parts.cc \
@@ -98,8 +96,12 @@ ACTIONS = \
 	actions/ctl/rule_remove_target_by_id.cc \
 	actions/ctl/rule_remove_by_id.cc \
 	actions/ctl/request_body_access.cc\
+	actions/disruptive/allow.cc \
+	actions/disruptive/block.cc \
+	actions/disruptive/deny.cc \
+	actions/disruptive/redirect.cc \
+	actions/disruptive/pass.cc \
 	actions/init_col.cc \
-	actions/deny.cc \
 	actions/log.cc \
 	actions/log_data.cc \
 	actions/maturity.cc \
@@ -107,16 +109,14 @@ ACTIONS = \
 	actions/multi_match.cc \
 	actions/no_audit_log.cc \
 	actions/no_log.cc \
-	actions/pass.cc \
 	actions/phase.cc \
-	actions/redirect.cc \
 	actions/rev.cc \
 	actions/rule_id.cc \
 	actions/severity.cc \
 	actions/set_sid.cc \
 	actions/set_uid.cc \
 	actions/set_var.cc \
-	actions/status.cc \
+	actions/data/status.cc \
 	actions/skip.cc \
 	actions/skip_after.cc \
 	actions/tag.cc \

src/actions/action.cc

@@ -22,16 +22,16 @@
 #include "modsecurity/rule.h"
 #include "src/utils/string.h"
 
-#include "src/actions/block.h"
+#include "src/actions/disruptive/block.h"
 #include "src/actions/chain.h"
-#include "src/actions/deny.h"
+#include "src/actions/disruptive/deny.h"
-#include "src/actions/redirect.h"
+#include "src/actions/disruptive/redirect.h"
-#include "src/actions/status.h"
+#include "src/actions/data/status.h"
 #include "src/actions/rule_id.h"
 #include "src/actions/phase.h"
 #include "src/actions/severity.h"
 #include "src/actions/capture.h"
-#include "src/actions/pass.h"
+#include "src/actions/disruptive/pass.h"
 #include "src/actions/log.h"
 #include "src/actions/no_log.h"
 #include "src/actions/multi_match.h"
@@ -55,9 +55,6 @@ bool Action::evaluate(Rule *rule, Transaction *transaction) {
 }
 
 
-void Action::fillIntervention(ModSecurityIntervention *i) {
-}
-
 Action *Action::instantiate(const std::string& name) {
     std::string status("status:");
     std::string redirect("redirect:");
@@ -66,13 +63,13 @@ Action *Action::instantiate(const std::string& name) {
     std::string rule_id("id:");
 
     if (name.compare(0, status.length(), status) == 0) {
-        return new Status(name);
+        return new data::Status(name);
     }
     if (name.compare(0, redirect.length(), redirect) == 0) {
-        return new Redirect(name);
+        return new disruptive::Redirect(name);
     }
     if (name.compare(0, block.length(), block) == 0) {
-        return new Block(name);
+        return new disruptive::Block(name);
     }
     if (name.compare(0, phase.length(), phase) == 0) {
         return new Phase(name);
@@ -87,10 +84,10 @@ Action *Action::instantiate(const std::string& name) {
         return new Capture(name);
     }
     if (name == "pass") {
-        return new Pass(name);
+        return new disruptive::Pass(name);
     }
     if (name == "deny") {
-        return new Deny(name);
+        return new disruptive::Deny(name);
     }
     if (name == "log") {
         return new Log(name);

src/actions/data/status.cc

@@ -13,7 +13,7 @@
  *
  */
 
-#include "src/actions/status.h"
+#include "src/actions/data/status.h"
 
 #include <iostream>
 #include <string>
@@ -23,7 +23,7 @@
 
 namespace modsecurity {
 namespace actions {
-
+namespace data {
 
 bool Status::init(std::string *error) {
     try {
@@ -38,16 +38,11 @@ bool Status::init(std::string *error) {
 
 
 bool Status::evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) {
-    rm->m_tmp_actions.push_back(this);
+    transaction->m_it.status = m_status;
     return true;
 }
 
 
-void Status::fillIntervention(ModSecurityIntervention *i) {
+}  // namespace data
-    i->status = m_status;
-    i->log = "Status";
-}
-
-
 }  // namespace actions
 }  // namespace modsecurity

src/actions/data/status.h

@@ -27,6 +27,8 @@ class Transaction;
 namespace modsecurity {
 class Transaction;
 namespace actions {
+namespace data {
+
 
 class Status : public Action {
  public:
@@ -36,12 +38,12 @@ class Status : public Action {
     bool init(std::string *error) override;
     bool evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm)
         override;
-    void fillIntervention(ModSecurityIntervention *i) override;
 
- protected:
     int m_status;
 };
 
+
+}  // namespace data
 }  // namespace actions
 }  // namespace modsecurity
 #endif

src/actions/disruptive/allow.cc

@@ -13,7 +13,7 @@
  *
  */
 
-#include "src/actions/allow.h"
+#include "src/actions/disruptive/allow.h"
 
 #include <iostream>
 #include <string>
@@ -26,6 +26,8 @@
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
+
 
 bool Allow::init(std::string *error) {
     std::string a = utils::string::tolower(m_parser_payload);
@@ -56,5 +58,7 @@ bool Allow::evaluate(Rule *rule, Transaction *transaction) {
     return true;
 }
 
+
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity

src/actions/disruptive/allow.h

@@ -28,6 +28,8 @@ class Transaction;
 class Rule;
 
 namespace actions {
+namespace disruptive {
+
 
 enum AllowType : int {
   /**
@@ -76,6 +78,8 @@ class Allow : public Action {
     }
 };
 
+
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity
 #endif

src/actions/disruptive/block.cc

@@ -13,36 +13,38 @@
  *
  */
 
-#include "src/actions/block.h"
+#include "src/actions/disruptive/block.h"
 
 #include <iostream>
 #include <string>
 
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule.h"
+#include "modsecurity/rules.h"
 #include "modsecurity/intervention.h"
+#include "src/actions/data/status.h"
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
 
 
 bool Block::evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) {
-#ifndef NO_LOGS
+    std::string log;
-    transaction->debug(8, "Running action block");
+
-#endif
+    transaction->debug(8, "Marking request as disruptive.");
-    for (Action *a : rule->m_actionsRuntimePos) {
+
-        if (a->isDisruptive() == true) {
+    for (Action *a : transaction->m_rules->defaultActions[rule->phase]) {
-            rm->m_tmp_actions.push_back(a);
+        if (a->isDisruptive() == false) {
+            continue;
         }
+        a->evaluate(rule, transaction, rm);
     }
+
     return true;
 }
 
 
-void Block::fillIntervention(ModSecurityIntervention *i) {
+}  // namespace disruptive
-    i->disruptive = true;
-}
-
-
 }  // namespace actions
 }  // namespace modsecurity

src/actions/disruptive/block.h

@@ -28,6 +28,7 @@ namespace modsecurity {
 class Transaction;
 
 namespace actions {
+namespace disruptive {
 
 
 class Block : public Action {
@@ -36,11 +37,11 @@ class Block : public Action {
 
     bool evaluate(Rule *rule, Transaction *transaction,
         RuleMessage *rm) override;
-    void fillIntervention(ModSecurityIntervention *i) override;
     bool isDisruptive() override { return true; }
 };
 
 
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity
 #endif

src/actions/disruptive/deny.cc

@@ -13,34 +13,41 @@
  *
  */
 
-#include "src/actions/deny.h"
+#include "src/actions/disruptive/deny.h"
 
 #include <iostream>
 #include <string>
+#include <cstring>
+#include <string.h>
 
 #include "modsecurity/transaction.h"
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
 
 
 bool Deny::evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) {
 #ifndef NO_LOGS
     transaction->debug(8, "Running action deny");
 #endif
-    rm->m_tmp_actions.push_back(this);
+    std::string log;
+
+    if (transaction->m_it.status == 200) {
+        transaction->m_it.status = 403;
+    }
+
+    log.append("Access denied with code %d");
+    log.append(" (phase ");
+    log.append(std::to_string(rm->m_rule->phase - 1) + "). ");
+
+    transaction->m_it.disruptive = true;
+    transaction->m_it.log = strdup(rm->disruptiveErrorLog(transaction, log).c_str());
+
     return true;
 }
 
 
-void Deny::fillIntervention(ModSecurityIntervention *i) {
+}  // namespace disruptive
-    if (i->status == 200) {
-        i->status = 403;
-    }
-    i->log = "Deny action";
-    i->disruptive = true;
-}
-
-
 }  // namespace actions
 }  // namespace modsecurity

src/actions/disruptive/deny.h

@@ -24,6 +24,7 @@
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
 
 
 class Deny : public Action {
@@ -32,11 +33,11 @@ class Deny : public Action {
 
     bool evaluate(Rule *rule, Transaction *transaction,
         RuleMessage *rm) override;
-    void fillIntervention(ModSecurityIntervention *i) override;
     bool isDisruptive() override { return true; }
 };
 
 
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity
 

src/actions/disruptive/pass.cc

@@ -13,7 +13,7 @@
  *
  */
 
-#include "src/actions/pass.h"
+#include "src/actions/disruptive/pass.h"
 
 #include <iostream>
 #include <string>
@@ -24,13 +24,22 @@
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
 
 
 bool Pass::evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) {
-    rm->m_tmp_actions.clear();
+    transaction->m_it.status = 200;
+    transaction->m_it.disruptive = false;
+    transaction->m_it.url = NULL;
+    transaction->m_it.log = NULL;
+    transaction->m_it.pause = 0;
+
+    transaction->debug(8, "Running action pass");
+
     return true;
 }
 
 
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity

src/actions/disruptive/pass.h

@@ -18,11 +18,12 @@
 #include "modsecurity/actions/action.h"
 #include "modsecurity/transaction.h"
 
-#ifndef SRC_ACTIONS_PASS_H_
+#ifndef SRC_ACTIONS_DISRUPTIVE_PASS_H_
-#define SRC_ACTIONS_PASS_H_
+#define SRC_ACTIONS_DISRUPTIVE_PASS_H_
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
 
 
 class Pass : public Action {
@@ -34,8 +35,10 @@ class Pass : public Action {
     bool isDisruptive() override { return true; }
 };
 
+
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity
 
 
-#endif  // SRC_ACTIONS_PASS_H_
+#endif  // SRC_ACTIONS_DISRUPTIVE_PASS_H_

src/actions/disruptive/redirect.cc

@@ -13,16 +13,19 @@
  *
  */
 
-#include "src/actions/redirect.h"
+#include "src/actions/disruptive/redirect.h"
 
 #include <iostream>
 #include <string>
+#include <string.h>
+
 
 #include "modsecurity/transaction.h"
 #include "src/macro_expansion.h"
 
 namespace modsecurity {
 namespace actions {
+namespace disruptive {
 
 
 bool Redirect::init(std::string *error) {
@@ -32,23 +35,26 @@ bool Redirect::init(std::string *error) {
 }
 
 
-bool Redirect::evaluate(Rule *rule, Transaction *transaction) {
+bool Redirect::evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) {
     m_urlExpanded = MacroExpansion::expand(m_url, transaction);
-    transaction->m_actions.push_back(this);
+    std::string log;
+
+    /* if it was changed before, lets keep it. */
+    if (transaction->m_it.status == 200) {
+        transaction->m_it.status = m_status;
+    }
+    log.append("Access denied with code %d");
+    log.append(" (phase ");
+    log.append(std::to_string(rm->m_rule->phase - 1) + "). ");
+
+    transaction->m_it.url = strdup(m_urlExpanded.c_str());
+    transaction->m_it.disruptive = true;
+    transaction->m_it.log = strdup(rm->disruptiveErrorLog(transaction, log).c_str());
+
     return true;
 }
 
 
-void Redirect::fillIntervention(ModSecurityIntervention *i) {
+}  // namespace disruptive
-    /* if it was changed before, lets keep it. */
-    if (i->status == 200) {
-        i->status = m_status;
-    }
-    i->url = m_urlExpanded.c_str();
-    i->log = "Redirecting";
-    i->disruptive = true;
-}
-
-
 }  // namespace actions
 }  // namespace modsecurity

src/actions/disruptive/redirect.h

@@ -16,6 +16,7 @@
 #include <string>
 
 #include "modsecurity/actions/action.h"
+#include "modsecurity/rule_message.h"
 
 #ifndef SRC_ACTIONS_REDIRECT_H_
 #define SRC_ACTIONS_REDIRECT_H_
@@ -27,6 +28,8 @@ namespace modsecurity {
 class Transaction;
 
 namespace actions {
+namespace disruptive {
+
 
 class Redirect : public Action {
  public:
@@ -36,9 +39,8 @@ class Redirect : public Action {
         m_urlExpanded(""),
         m_url("") { }
 
-    bool evaluate(Rule *rule, Transaction *transaction) override;
+    bool evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) override;
     bool init(std::string *error) override;
-    void fillIntervention(ModSecurityIntervention *i) override;
     bool isDisruptive() override { return true; }
 
  private:
@@ -47,6 +49,8 @@ class Redirect : public Action {
     std::string m_url;
 };
 
+
+}  // namespace disruptive
 }  // namespace actions
 }  // namespace modsecurity
 #endif

src/actions/msg.cc

@@ -48,10 +48,12 @@ namespace actions {
 
 bool Msg::evaluate(Rule *rule, Transaction *transaction, RuleMessage *rm) {
     std::string msg = data(transaction);
-    transaction->debug(9, "Saving msg: " + msg);
     rm->m_message = msg;
+    transaction->debug(9, "Saving msg: " + msg);
 
     transaction->m_collections.storeOrUpdateFirst("RULE:msg", msg);
+
+    rm->m_server_logs.push_back(rm->errorLog(transaction));
     return true;
 }
 

src/operators/pm.cc

@@ -26,6 +26,7 @@
 
 #include "src/operators/operator.h"
 #include "src/utils/acmp.h"
+#include "src/utils/string.h"
 
 namespace modsecurity {
 namespace operators {
@@ -78,15 +79,6 @@ void Pm::postOrderTraversal(acmp_btree_node_t *node) {
     node = NULL;
 }
 
-void Pm::replaceAll(std::string str, const std::string& from,
-    const std::string& to) {
-    size_t start_pos = 0;
-    while ((start_pos = str.find(from, start_pos)) != std::string::npos) {
-        size_t end_pos = start_pos + from.length();
-        str.replace(start_pos, end_pos, to);
-        start_pos += to.length();
-    }
-}
 
 bool Pm::evaluate(Transaction *transaction, Rule *rule,
     const std::string &input) {
@@ -119,8 +111,6 @@ bool Pm::init(const std::string &file, std::string *error) {
     std::istringstream *iss;
     const char *err = NULL;
 
-    replaceAll(m_param, "\\", "\\\\");
-
     char *content = parse_pm_content(m_param.c_str(), m_param.length(), &err);
     if (content == NULL) {
         iss = new std::istringstream(m_param);

src/operators/pm.h

@@ -35,8 +35,6 @@ class Pm : public Operator {
         m_p = acmp_create(0);
     }
     ~Pm();
-    void replaceAll(std::string str, const std::string& from,
-        const std::string& to);
     bool evaluate(Transaction *transaction, Rule *rule,
         const std::string &input) override;
     bool evaluate(Transaction *transaction,

src/parser/seclang-parser.yy

@@ -22,7 +22,7 @@ class Driver;
 
 #include "src/actions/accuracy.h"
 #include "modsecurity/actions/action.h"
-#include "src/actions/allow.h"
+#include "src/actions/disruptive/allow.h"
 #include "src/actions/audit_log.h"
 #include "src/actions/ctl/audit_log_parts.h"
 #include "src/actions/ctl/request_body_access.h"
@@ -36,7 +36,7 @@ class Driver;
 #include "src/actions/maturity.h"
 #include "src/actions/msg.h"
 #include "src/actions/phase.h"
-#include "src/actions/redirect.h"
+#include "src/actions/disruptive/redirect.h"
 #include "src/actions/rev.h"
 #include "src/actions/set_sid.h"
 #include "src/actions/set_uid.h"
@@ -102,7 +102,7 @@ using modsecurity::Variables::Variations::Exclusion;
 using modsecurity::Variables::XML;
 using modsecurity::actions::Accuracy;
 using modsecurity::actions::Action;
-using modsecurity::actions::Allow;
+using modsecurity::actions::disruptive::Allow;
 using modsecurity::actions::ctl::AuditLogParts;
 using modsecurity::actions::ctl::RequestBodyProcessorJSON;
 using modsecurity::actions::ctl::RequestBodyProcessorXML;
@@ -111,7 +111,7 @@ using modsecurity::actions::LogData;
 using modsecurity::actions::Maturity;
 using modsecurity::actions::Msg;
 using modsecurity::actions::Phase;
-using modsecurity::actions::Redirect;
+using modsecurity::actions::disruptive::Redirect;
 using modsecurity::actions::Rev;
 using modsecurity::actions::SetSID;
 using modsecurity::actions::SetUID;

src/rule.cc

@@ -251,9 +251,6 @@ void Rule::executeActionsIndependentOfChainedRuleResult(Transaction *trasn,
             }
         }
     }
-    for (auto &z : ruleMessage->m_tmp_actions) {
-        trasn->m_actions.push_back(z);
-    }
 }
 
 
@@ -505,16 +502,14 @@ void Rule::executeActionsAfterFullMatch(Transaction *trasn,
             a->evaluate(this, trasn, ruleMessage);
             continue;
         } else {
+            trasn->debug(4, "_Not_ running (disruptive) action: "
+                + a->m_name + ". SecRuleEngine is not On.");
             continue;
         }
 
         trasn->debug(4, "Not running disruptive action: " + \
                 a->m_name + ". SecRuleEngine is not On");
     }
-
-    for (auto &z : ruleMessage->m_tmp_actions) {
-        trasn->m_actions.push_back(z);
-    }
 }
 
 
@@ -524,6 +519,8 @@ bool Rule::evaluate(Transaction *trasn) {
     bool recursiveGlobalRet;
     bool containsDisruptive = false;
     RuleMessage ruleMessage(this);
+    std::vector<const collection::Variable *> finalVars;
+    std::string eparam;
 
     trasn->m_matched.clear();
 
@@ -535,8 +532,7 @@ bool Rule::evaluate(Transaction *trasn) {
             + ") Executing unconditional rule...");
         executeActionsIndependentOfChainedRuleResult(trasn,
             &containsDisruptive, &ruleMessage);
-        executeActionsAfterFullMatch(trasn, false, &ruleMessage);
+        goto end_exec;
-        return true;
     }
 
     for (auto &i : trasn->m_ruleRemoveById) {
@@ -548,7 +544,7 @@ bool Rule::evaluate(Transaction *trasn) {
         return true;
     }
 
-    std::string eparam = MacroExpansion::expand(this->op->m_param, trasn);
+    eparam = MacroExpansion::expand(this->op->m_param, trasn);
 
     if (this->op->m_param != eparam) {
         eparam = "\"" + eparam + "\" Was: \"" + this->op->m_param + "\"";
@@ -565,7 +561,7 @@ bool Rule::evaluate(Transaction *trasn) {
 
     updateRulesVariable(trasn);
 
-    std::vector<const collection::Variable *> finalVars = getFinalVars(trasn);
+    finalVars = getFinalVars(trasn);
 
     for (const collection::Variable *v : finalVars) {
         std::string value = v->m_value;
@@ -583,13 +579,6 @@ bool Rule::evaluate(Transaction *trasn) {
                 updateMatchedVars(trasn, v->m_key, value);
                 executeActionsIndependentOfChainedRuleResult(trasn,
                     &containsDisruptive, &ruleMessage);
-                std::string msg2save = ruleMessage.errorLog(trasn);
-                if (ruleMessage.m_message.empty() == false) {
-                    trasn->debug(4,
-                        "Scheduled to be saved on the server log: " \
-                        + msg2save + "");
-                    ruleMessage.m_server_logs.push_back(msg2save);
-                }
                 globalRet = true;
             }
         }
@@ -604,14 +593,7 @@ bool Rule::evaluate(Transaction *trasn) {
     trasn->debug(4, "Rule returned 1.");
 
     if (this->chained == false) {
-        executeActionsAfterFullMatch(trasn, containsDisruptive, &ruleMessage);
+        goto end_exec;
-        trasn->debug(4, "Merging temporary actions to the main transaction.");
-
-        for (const auto &u : ruleMessage.m_server_logs) {
-            trasn->debug(8, "To save: " + u);
-            trasn->serverLog(u);
-        }
-        return true;
     }
 
     if (this->chainedRule == NULL) {
@@ -624,15 +606,18 @@ bool Rule::evaluate(Transaction *trasn) {
     recursiveGlobalRet = this->chainedRule->evaluate(trasn);
 
     if (recursiveGlobalRet == true) {
-        executeActionsAfterFullMatch(trasn, containsDisruptive, &ruleMessage);
+        goto end_exec;
-        trasn->debug(4, "Merging temporary actions to the main transaction.");
-        for (const auto &u : ruleMessage.m_server_logs) {
-            trasn->serverLog(u);
-        }
-        return true;
     }
 
     return false;
+
+end_exec:
+    executeActionsAfterFullMatch(trasn, containsDisruptive, &ruleMessage);
+    for (const auto &u : ruleMessage.m_server_logs) {
+        trasn->serverLog(u);
+    }
+
+    return true;
 }
 
 

src/rule_message.cc

@@ -23,6 +23,36 @@
 
 namespace modsecurity {
 
+std::string RuleMessage::disruptiveErrorLog(Transaction *trans, std::string msg2) {
+    std::string msg;
+
+    msg.append("[client " + std::string(trans->m_clientIpAddress) + "]");
+    msg.append(" ModSecurity: ");
+    msg.append(msg2);
+    msg.append(m_match);
+    msg.append(" [file \"" + std::string(m_ruleFile) + "\"]");
+    msg.append(" [line \"" + std::to_string(m_ruleLine) + "\"]");
+    msg.append(" [id \"" + std::to_string(m_ruleId) + "\"]");
+    msg.append(" [rev \"" + m_rev + "\"]");
+    msg.append(" [msg \"" + m_message + "\"]");
+    msg.append(" [data \"" + m_data + "\"]");
+    msg.append(" [severity \"" +
+        std::to_string(m_severity) + "\"]");
+    msg.append(" [ver \"" + m_ver + "\"]");
+    msg.append(" [maturity \"" + std::to_string(m_maturity) + "\"]");
+    msg.append(" [accuracy \"" + std::to_string(m_accuracy) + "\"]");
+    for (auto &a : m_tags) {
+        msg.append(" [tag \"" + a + "\"]");
+    }
+    msg.append(" [hostname \"" + std::string(trans->m_serverIpAddress) \
+        + "\"]");
+    msg.append(" [uri \"" + trans->m_uri_no_query_string_decoded + "\"]");
+    msg.append(" [unique_id \"" + trans->m_id + "\"]");
+
+    return modsecurity::utils::string::toHexIfNeeded(msg);
+
+}
+
 std::string RuleMessage::errorLog(Transaction *trans) {
     std::string msg;
 

src/rules.cc

@@ -189,20 +189,20 @@ int Rules::evaluate(int phase, Transaction *transaction) {
     debug(9, "This phase consists of " + std::to_string(rules.size()) + \
         " rule(s).");
 
-    if (transaction->m_allowType == actions::FromNowOneAllowType
+    if (transaction->m_allowType == actions::disruptive::FromNowOneAllowType
         && phase != modsecurity::Phases::LoggingPhase) {
         debug(9, "Skipping all rules evaluation on this phase as request " \
             "through the utilization of an `allow' action.");
         return true;
     }
-    if (transaction->m_allowType == actions::RequestAllowType
+    if (transaction->m_allowType == actions::disruptive::RequestAllowType
         && phase <= modsecurity::Phases::RequestBodyPhase) {
         debug(9, "Skipping all rules evaluation on this phase as request " \
             "through the utilization of an `allow' action.");
         return true;
     }
-    if (transaction->m_allowType != actions::NoneAllowType) {
+    if (transaction->m_allowType != actions::disruptive::NoneAllowType) {
-        transaction->m_allowType = actions::NoneAllowType;
+        transaction->m_allowType = actions::disruptive::NoneAllowType;
     }
 
     for (int i = 0; i < rules.size(); i++) {
@@ -223,7 +223,8 @@ int Rules::evaluate(int phase, Transaction *transaction) {
             debug(9, "Skipped rule id '" + std::to_string(rule->rule_id) \
                 + "' due to a `skip' action. Still " + \
                 std::to_string(transaction->m_skip_next) + " to be skipped.");
-        } else if (transaction->m_allowType != actions::NoneAllowType) {
+        } else if (transaction->m_allowType
+            != actions::disruptive::NoneAllowType) {
             debug(9, "Skipped rule id '" + std::to_string(rule->rule_id) \
                 + "' as request trough the utilization of an `allow' action.");
         } else if (m_exceptions.contains(rule->rule_id)) {
@@ -231,6 +232,11 @@ int Rules::evaluate(int phase, Transaction *transaction) {
                 + "'. Removed by an SecRuleRemove directive.");
         } else {
             rule->evaluate(transaction);
+            if (transaction->m_it.disruptive == true) {
+                debug(8, "Skipping this phase as this " \
+                    "request was already intercepted.");
+                break;
+            }
         }
     }
     return 1;

src/transaction.cc

@@ -33,7 +33,7 @@
 #include <vector>
 
 #include "modsecurity/actions/action.h"
-#include "src/actions/deny.h"
+#include "src/actions/disruptive/deny.h"
 #include "modsecurity/intervention.h"
 #include "modsecurity/modsecurity.h"
 #include "src/request_body_processor/multipart.h"
@@ -48,7 +48,7 @@
 #include "modsecurity/rule.h"
 #include "modsecurity/rule_message.h"
 #include "modsecurity/rules_properties.h"
-#include "src/actions/allow.h"
+#include "src/actions/disruptive/allow.h"
 
 
 
@@ -123,7 +123,7 @@ Transaction::Transaction(ModSecurity *ms, Rules *rules, void *logCbData)
     m_responseContentType(NULL),
     m_requestBodyAccess(Rules::PropertyNotSetConfigBoolean),
     m_marker(""),
-    m_allowType(modsecurity::actions::NoneAllowType),
+    m_allowType(modsecurity::actions::disruptive::NoneAllowType),
     m_skip_next(0),
     m_creationTimeStamp(utils::cpu_seconds()),
     m_logCbData(logCbData),
@@ -158,8 +158,14 @@ Transaction::Transaction(ModSecurity *ms, Rules *rules, void *logCbData)
     m_collections.storeOrUpdateFirst("URLENCODED_ERROR", "0");
 
 #ifndef NO_LOGS
-    this->debug(4, "Initialising transaction");
+    this->debug(4, "Initializing transaction");
 #endif
+
+    m_it.status = 200;
+    m_it.disruptive = false;
+    m_it.url = NULL;
+    m_it.log = NULL;
+    m_it.pause = 0;
 }
 
 
@@ -879,9 +885,9 @@ int Transaction::appendRequestBody(const unsigned char *buf, size_t len) {
                 debug(5, "Request body limit is marked to reject the " \
                     "request");
 #endif
-                Action *a = new actions::Deny("deny");
+                m_it.log = "Request body limit is marked to reject the request";
-                a->temporaryAction = true;
+                m_it.status = 403;
-                m_actions.push_back(a);
+                m_it.disruptive = true;
             }
             return true;
         }
@@ -1136,9 +1142,10 @@ int Transaction::appendResponseBody(const unsigned char *buf, size_t len) {
                 debug(5, "Response body limit is marked to reject the " \
                     "request");
 #endif
-                Action *a = new actions::Deny("deny");
+
-                a->temporaryAction = true;
+                m_it.log = "Response body limit is marked to reject the request";
-                m_actions.push_back(a);
+                m_it.status = 403;
+                m_it.disruptive = true;
             }
             return true;
         }
@@ -1285,20 +1292,26 @@ int Transaction::processLogging() {
  *
  */
 bool Transaction::intervention(ModSecurityIntervention *it) {
-    it->status = 200;
+    if (m_it.disruptive) {
-    it->url = NULL;
+        it->url = m_it.url;
-    it->disruptive = false;
+        it->disruptive = m_it.disruptive;
-    if (m_actions.size() > 0) {
+        it->status = m_it.status;
-        for (Action *a : m_actions) {
+
-            if (a->action_kind == Action::Kind::RunTimeOnlyIfMatchKind) {
+        if (m_it.log != NULL) {
-                a->fillIntervention(it);
+            std::string log("");
+            const char *log_str;
+            log.append(m_it.log);
+            utils::string::replaceAll(&log, std::string("%d"), std::to_string(it->status));
+            log_str = strdup(log.c_str());
+            it->log = log_str;
         }
-            if (a->temporaryAction) {
+        m_it.status = 200;
-                delete a;
+        m_it.disruptive = false;
-            }
+        m_it.url = NULL;
-        }
+        m_it.log = NULL;
-        m_actions.clear();
+        m_it.pause = 0;
     }
+
     return it->disruptive;
 }
 

src/utils/string.cc

@@ -206,6 +206,17 @@ unsigned char *c2x(unsigned what, unsigned char *where) {
 }
 
 
+void replaceAll(std::string *str, const std::string& from,
+    const std::string& to) {
+    size_t start_pos = 0;
+    while ((start_pos = str->find(from, start_pos)) != std::string::npos) {
+        size_t end_pos = start_pos + from.length();
+        str->replace(start_pos, from.length(), to);
+        start_pos += to.length();
+    }
+}
+
+
 }  // namespace string
 }  // namespace utils
 }  // namespace modsecurity

src/utils/string.h

@@ -66,6 +66,8 @@ std::string tolower(std::string str);
 std::string toupper(std::string str);
 std::vector<std::string> split(std::string str, char delimiter);
 void chomp(std::string *str);
+void replaceAll(std::string *str, const std::string& from,
+    const std::string& to);
 
 unsigned char x2c(unsigned char *what);
 unsigned char xsingle2c(unsigned char *what);

test/test-cases/regression/action-disruptive.json

@@ -32,7 +32,7 @@
     "version_min":300000,
     "title":"Testing Disruptive actions (3/n)",
     "expected":{
-      "debug_log": "Running action block",
+      "debug_log": "Running .disruptive. action: block",
       "http_code":404
     },
     "rules":[

test/test-cases/regression/secruleengine.json

@@ -34,14 +34,14 @@
     "version_min":300000,
     "title":"Testing Disruptive actions (3/n)",
     "expected":{
-      "debug_log": "_Not_ running action: deny. Rule _does not_contains a disruptive action, but SecRuleEngine is not On.",
+      "debug_log": "_Not_ running .disruptive. action: block. SecRuleEngine is not On",
       "http_code":200
     },
     "rules":[
       "SecRuleEngine On",
       "SecRuleEngine DetectionOnly",
       "SecDefaultAction \"phase:2,deny,status:404\"",
-      "SecAction \"id:'1',phase:request,nolog,block,t:none\""
+      "SecAction \"id:'1',phase:request,nolog,nolog,block,t:none\""
     ]
   },
   {