setvar: needs review

src/actions/capture.cc

@@ -33,17 +33,6 @@ namespace actions {
 
 
 bool Capture::evaluate(Rule *rule, Transaction *transaction) {
-    if (transaction->m_matched.empty()) {
-        return false;
-    }
-
-    int i = 0;
-    while (transaction->m_matched.empty() == false) {
-        transaction->m_collections.storeOrUpdateFirst("TX",
-            std::to_string(i), transaction->m_matched.back());
-        transaction->m_matched.pop_back();
-        i++;
-    }
     return true;
 }
 

src/operators/operator.cc

@@ -75,6 +75,17 @@ bool Operator::debug(Transaction *transaction, int x, std::string a) {
 }
 
 
+bool Operator::evaluateInternal(Transaction *transaction,
+    Rule *rule, const std::string& a) {
+    bool res = evaluate(transaction, rule, a);
+
+    if (m_negation) {
+        return !res;
+    }
+
+    return res;
+}
+
 bool Operator::evaluateInternal(Transaction *transaction,
     const std::string& a) {
     bool res = evaluate(transaction, a);

src/operators/operator.h

@@ -21,6 +21,7 @@
 #define SRC_OPERATORS_OPERATOR_H__
 
 #include "modsecurity/transaction.h"
+#include "modsecurity/rule.h"
 
 #ifdef __cplusplus
 namespace modsecurity {
@@ -48,7 +49,14 @@ class Operator {
     }
 
     bool evaluateInternal(Transaction *t, const std::string& a);
+    bool evaluateInternal(Transaction *t, Rule *rule,
+        const std::string& a);
+
     virtual bool evaluate(Transaction *transaction, const std::string &str);
+    virtual bool evaluate(Transaction *transaction, Rule *rule,
+        const std::string &str) {
+        return evaluate(transaction, str);
+    }
 
     bool m_negation;
     std::string m_match_message;

src/operators/rx.cc

@@ -20,13 +20,15 @@
 
 #include "src/operators/operator.h"
 #include "src/macro_expansion.h"
+#include "modsecurity/rule.h"
 
 namespace modsecurity {
 namespace operators {
 
 
 
-bool Rx::evaluate(Transaction *transaction, const std::string& input) {
+bool Rx::evaluate(Transaction *transaction, Rule *rule,
+    const std::string& input) {
     SMatch match;
     std::list<SMatch> matches;
 
@@ -35,10 +37,16 @@ bool Rx::evaluate(Transaction *transaction, const std::string& input) {
     }
 
     matches = m_re->searchAll(input);
-    for (const SMatch& a : matches) {
-        if (transaction) {
+    if (rule && rule->getActionsByName("capture").size() > 0 && transaction) {
+        int i = 0;
+        matches.reverse();
+        for (const SMatch& a : matches) {
+            transaction->m_collections.storeOrUpdateFirst("TX",
+                std::to_string(i), a.match);
+            transaction->debug(7, "Added regex subexpression TX." +
+                std::to_string(i) + ": " + a.match);
             transaction->m_matched.push_back(a.match);
-            transaction->debug(7, "Added regex subexpression: " + a.match);
+            i++;
         }
     }
 

src/operators/rx.h

@@ -43,7 +43,12 @@ class Rx : public Operator {
     ~Rx() {
         delete m_re;
     }
-    bool evaluate(Transaction *transaction, const std::string &input);
+    bool evaluate(Transaction *transaction, Rule *rule,
+        const std::string &input) override;
+    bool evaluate(Transaction *transaction,
+        const std::string &input) override {
+        return evaluate(transaction, NULL, input);
+    }
 
  private:
     std::string m_param;

src/rule.cc

@@ -208,7 +208,7 @@ bool Rule::evaluateActions(Transaction *trasn) {
     for (Action *a : this->actions_runtime_pos) {
         if (a->isDisruptive() == false) {
 #ifndef NO_LOGS
-            trasn->debug(4, "Running (_non_ disruptive) action: " +
+            trasn->debug(4, "Running [III] (_non_ disruptive) action: " +
                 a->m_name + ".");
 #endif
             a->evaluate(this, trasn);
@@ -438,7 +438,7 @@ bool Rule::evaluate(Transaction *trasn) {
                 + "\" (Variable: " + v->m_key + ")");
 #endif
 
-            ret = this->op->evaluateInternal(trasn, value);
+            ret = this->op->evaluateInternal(trasn, this, value);
 
 #ifndef NO_LOGS
             clock_t end = clock();
@@ -487,6 +487,12 @@ bool Rule::evaluate(Transaction *trasn) {
                         } else {
                             containsDisruptive = true;
                         }
+                    } else {
+                        if (a->m_name == "setvar") {
+                            trasn->debug(4, "Running [I] (_non_ disruptive) " \
+                                "action: " + a->m_name);
+                            a->evaluate(this, trasn, ruleMessage);
+                        }
                     }
                 }
 
@@ -574,11 +580,14 @@ bool Rule::evaluate(Transaction *trasn) {
 #endif
                             }
                         } else if (!a->isDisruptive()) {
+                            // here
+                            if (a->m_name != "capture" && a->m_name != "setvar") {
 #ifndef NO_LOGS
-                            trasn->debug(4, "Running (_non_ disruptive) " \
-                                "action: " + a->m_name);
+                                trasn->debug(4, "Running [II] (_non_ disruptive) " \
+                                    "action: " + a->m_name);
 #endif
-                            a->evaluate(this, trasn, ruleMessage);
+                                a->evaluate(this, trasn, ruleMessage);
+                            }
                         }
                     }
                 }

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

@@ -111,8 +111,8 @@
     },
     "rules":[
       "SecRuleEngine On",
-      "SecRule REQUEST_HEADERS \"@contains PHPSESSID\" \"id:1,capture,t:lowercase,t:none,msg:'This is a test: %{TX.0}% ops'\"",
-      "SecRule TX \"@contains to_test\" \"id:2,t:lowercase,capture,t:none\""
+      "SecRule REQUEST_HEADERS \"@rx PHPSESSID\" \"id:1,capture,t:lowercase,t:none,msg:'This is a test: %{TX.0}% ops'\"",
+      "SecRule TX \"@rx to_test\" \"id:2,t:lowercase,capture,t:none\""
     ]
   }
 ]