Refactoring on the Rule class

headers/modsecurity/rule.h

@@ -38,6 +38,12 @@ class Variables;
 }
 namespace actions {
 class Action;
+class Severity;
+class LogData;
+class Msg;
+class Rev;
+class SetVar;
+class Tag;
 }
 namespace operators {
 class Operator;
@@ -55,15 +61,31 @@ class Rule {
 
     virtual bool evaluate(Transaction *transaction,
         std::shared_ptr<RuleMessage> rm);
-    bool evaluateActions(Transaction *transaction);
+
-    std::vector<std::unique_ptr<VariableValue>>
+    void organizeActions(std::vector<actions::Action *> *actions);
-	getFinalVars(Transaction *trasn);
+    void cleanUpActions();
+    void executeAction(Transaction *trans,
+    bool containsBlock, std::shared_ptr<RuleMessage> ruleMessage,
+        actions::Action *a, bool context);
+
+    inline void executeTransformation(actions::Action *a,
+        std::shared_ptr<std::string> *value,
+        Transaction *trans,
+        std::list<std::pair<std::shared_ptr<std::string>,
+        std::shared_ptr<std::string>>> *ret,
+        std::string *path,
+        int *nth);
+
+    void getVariablesExceptions(Transaction *t,
+        Variables::Variables *exclusion, Variables::Variables *addition);
+    inline void getFinalVars(Variables::Variables *vars,
+        Variables::Variables *eclusion, Transaction *trans);
     void executeActionsAfterFullMatch(Transaction *trasn,
         bool containsDisruptive, std::shared_ptr<RuleMessage> ruleMessage);
 
     std::list<std::pair<std::shared_ptr<std::string>,
         std::shared_ptr<std::string>>> executeDefaultTransformations(
-        Transaction *trasn, const std::string &value, bool multiMatch);
+        Transaction *trasn, const std::string &value);
 
     bool executeOperatorAt(Transaction *trasn, std::string key,
         std::string value, std::shared_ptr<RuleMessage> rm);
@@ -72,14 +94,12 @@ class Rule {
     void updateMatchedVars(Transaction *trasn, std::string key,
         std::string value);
     void cleanMatchedVars(Transaction *trasn);
-    void updateRulesVariable(Transaction *trasn);
+    void updateRulesVariable(Transaction *trasn, std::shared_ptr<RuleMessage> rm);
 
-    //std::vector<std::string> getActionNames();
     std::vector<actions::Action *> getActionsByName(const std::string& name,
         Transaction *t);
     bool containsTag(const std::string& name, Transaction *t);
     bool containsMsg(const std::string& name, Transaction *t);
-    bool containsStaticDisruptiveAction();
 
     int refCountDecreaseAndCheck() {
         m_referenceCount--;
@@ -95,26 +115,48 @@ class Rule {
         m_referenceCount++;
     }
 
+    void executeTransformations(
+        actions::Action *a,
+        std::shared_ptr<std::string> newValue,
+        std::shared_ptr<std::string> value,
+        Transaction *trans,
+        std::list<std::pair<std::shared_ptr<std::string>,
+        std::shared_ptr<std::string>>> *ret,
+        std::shared_ptr<std::string> transStr,
+        int nth);
 
-    int m_accuracy;
-    std::vector<actions::Action *> m_actionsConf;
     std::vector<actions::Action *> m_actionsRuntimePos;
     std::vector<actions::Action *> m_actionsRuntimePre;
     bool m_chained;
     Rule *m_chainedRule;
     std::string m_fileName;
     int m_lineNumber;
-    std::string m_logData;
     std::string m_marker;
-    int m_maturity;
     operators::Operator *m_op;
-    int m_phase;
-    std::string m_rev;
-    int64_t m_ruleId;
     bool m_secMarker;
     modsecurity::Variables::Variables *m_variables;
-    std::string m_ver;
 
+
+    int64_t m_ruleId;
+    std::string m_rev;
+    // msg ?
+    std::string m_ver;
+    //std::string m_logData;
+
+    //if (child->severity != NOT_SET) merged->severity = child->severity;
+    int m_accuracy;
+    int m_maturity;
+    int m_phase;
+
+    bool m_containsStaticDisruptiveAction;
+    bool m_containsCaptureAction;
+    bool m_containsMultiMatchAction;
+    bool m_containsStaticBlockAction;
+    actions::Severity *m_severity;
+    actions::LogData *m_logData;
+    actions::Msg *m_msg;
+    std::vector<actions::SetVar *> m_actionsSetVar;
+    std::vector<actions::Tag *> m_actionsTag;
  private:
     bool m_unconditional;
     int m_referenceCount;

src/operators/detect_sqli.cc

@@ -40,8 +40,7 @@ bool DetectSQLi::evaluate(Transaction *t, Rule *rule,
                 "fingerprint '" + std::string(fingerprint) + "' at: '" +
                 input + "'");
 #endif
-            if (rule && t
+            if (rule && t && rule->m_containsCaptureAction) {
-                && rule->getActionsByName("capture", t).size() > 0) {
                 t->m_collections.m_tx_collection->storeOrUpdateFirst(
                     "0", std::string(fingerprint));
 #ifndef NO_LOGS

src/operators/detect_xss.cc

@@ -36,8 +36,7 @@ bool DetectXSS::evaluate(Transaction *t, Rule *rule,
 #ifndef NO_LOGS
             t->debug(5, "detected XSS using libinjection.");
 #endif
-            if (rule && t
+            if (rule && t && rule->m_containsCaptureAction) {
-                && rule->getActionsByName("capture", t).size() > 0) {
                 t->m_collections.m_tx_collection->storeOrUpdateFirst(
                     "0", std::string(input));
 #ifndef NO_LOGS

src/operators/pm.cc

@@ -96,8 +96,6 @@ bool Pm::evaluate(Transaction *transaction, Rule *rule,
 #ifdef MODSEC_MUTEX_ON_PM
     pthread_mutex_unlock(&m_lock);
 #endif
-    bool capture = rule && rule->getActionsByName("capture",
-        transaction).size() > 0;
 
     if (rc > 0 && transaction) {
         std::string match_(match);
@@ -105,7 +103,7 @@ bool Pm::evaluate(Transaction *transaction, Rule *rule,
         transaction->m_matched.push_back(match_);
     }
 
-    if (capture && transaction && rc) {
+    if (rule && rule->m_containsCaptureAction && transaction && rc) {
         transaction->m_collections.m_tx_collection->storeOrUpdateFirst("0",
             std::string(match));
 #ifndef NO_LOGS

src/operators/rbl.cc

@@ -222,8 +222,7 @@ bool Rbl::evaluate(Transaction *t, Rule *rule,
     furtherInfo(sin, ipStr, t);
 
     freeaddrinfo(info);
-    if (rule && t
+    if (rule && t && rule->m_containsCaptureAction) {
-        && rule->getActionsByName("capture", t).size() > 0) {
         t->m_collections.m_tx_collection->storeOrUpdateFirst(
         "0", std::string(ipStr));
 #ifndef NO_LOGS

src/operators/rx.cc

@@ -54,8 +54,7 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule,
     }
 
     matches = re->searchAll(input);
-    if (rule && rule->getActionsByName("capture",
+    if (rule && rule->m_containsCaptureAction && transaction) {
-            transaction).size() > 0 && transaction) {
         int i = 0;
         matches.reverse();
         for (const SMatch& a : matches) {

src/operators/verify_cc.cc

@@ -142,8 +142,7 @@ bool VerifyCC::evaluate(Transaction *t, Rule *rule,
             is_cc = luhnVerify(match.c_str(), match.size());
             if (is_cc) {
                 if (t) {
-                    if (rule && t
+                    if (rule && t && rule->m_containsCaptureAction) {
-                        && rule->getActionsByName("capture", t).size() > 0) {
                         t->m_collections.m_tx_collection->storeOrUpdateFirst(
                             "0", std::string(match));
 #ifndef NO_LOGS

src/operators/verify_cpf.cc

@@ -133,8 +133,7 @@ bool VerifyCPF::evaluate(Transaction *t, Rule *rule,
             is_cpf = verify(i.match.c_str(), i.match.size());
             if (is_cpf) {
                 logOffset(ruleMessage, i.m_offset, i.m_length);
-                if (rule && t
+                if (rule && t && rule->m_containsCaptureAction) {
-                    && rule->getActionsByName("capture", t).size() > 0) {
                     t->m_collections.m_tx_collection->storeOrUpdateFirst(
                         "0", std::string(i.match));
 #ifndef NO_LOGS

src/operators/verify_ssn.cc

@@ -124,8 +124,7 @@ bool VerifySSN::evaluate(Transaction *t, Rule *rule,
             is_ssn = verify(i.match.c_str(), i.match.size());
             if (is_ssn) {
                 logOffset(ruleMessage, i.m_offset, i.m_length);
-                if (rule && t
+                if (rule && t && rule->m_containsCaptureAction) {
-                    && rule->getActionsByName("capture", t).size() > 0) {
                     t->m_collections.m_tx_collection->storeOrUpdateFirst(
                         "0", std::string(i.match));
 #ifndef NO_LOGS

src/parser/driver.cc

@@ -81,7 +81,7 @@ int Driver::addSecRule(Rule *rule) {
         if (lastRule->m_chainedRule == NULL) {
             rule->m_phase = lastRule->m_phase;
             lastRule->m_chainedRule = rule;
-            if (rule->containsStaticDisruptiveAction()) {
+            if (rule->m_containsStaticDisruptiveAction) {
                 m_parserError << "Disruptive actions can only be specified by";
                 m_parserError << " chain starter rules.";
                 return false;
@@ -94,7 +94,7 @@ int Driver::addSecRule(Rule *rule) {
             }
             if (a->m_chained && a->m_chainedRule == NULL) {
                 a->m_chainedRule = rule;
-                if (a->containsStaticDisruptiveAction()) {
+                if (a->m_containsStaticDisruptiveAction) {
                     m_parserError << "Disruptive actions can only be ";
                     m_parserError << "specified by chain starter rules.";
                     return false;

src/variables/variable.h

@@ -605,6 +605,16 @@ class VariableRegex : public Variable {
 
 class Variables : public std::vector<Variable *> {
  public:
+    bool contains(Variable *v) {
+        return std::find_if(begin(), end(),
+            [v](Variable *m) -> bool { return *v == *m; }) != end();
+    };
+    bool contains(const std::string &v) {
+        return std::find_if(begin(), end(),
+            [v](Variable *m) -> bool {
+                return v == *m->m_fullName.get();
+            }) != end();
+    };
 };
 
 

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

@@ -4,7 +4,7 @@
     "version_min":300000,
     "title":"Testing CtlRuleRemoveTargetById (1)",
     "expected":{
-      "debug_log": "Variable: ARGS:pwd was excluded by ruleRemoveTargetById..."
+      "http_code": 200
     },
     "client":{
       "ip":"200.249.12.31",
@@ -27,8 +27,9 @@
       "port":80
     },
     "rules":[
+        "SecRuleEngine On",
         "SecRule REQUEST_FILENAME \"@endsWith /wp-login.php\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetById=1;ARGS:pwd\"",
-        "SecRule ARGS \"@contais whe\" \"id:1,phase:3,t:none,nolog,pass,tag:'CRS'\""
+        "SecRule ARGS \"@contains lhebs\" \"id:1,phase:3,t:none,status:202,block,deny,tag:'CRS'\""
     ]
   },
   {
@@ -68,7 +69,7 @@
     "version_min":300000,
     "title":"Testing CtlRuleRemoveTargetById (3)",
     "expected":{
-      "debug_log": "Variable: ARGS was excluded by ruleRemoveTargetById..."
+      "http_code": 200
     },
     "client":{
       "ip":"200.249.12.31",
@@ -92,7 +93,7 @@
     },
     "rules":[
         "SecRule REQUEST_FILENAME \"@endsWith /wp-login.php\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetById=1;ARGS\"",
-        "SecRule ARGS \"@contais whe\" \"id:1,phase:3,t:none,nolog,pass,tag:'CRS'\""
+        "SecRule ARGS \"@contains lhebs\" \"id:1,phase:3,t:none,status:202,block,deny,tag:'CRS'\""
     ]
   }
 ]

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

@@ -4,7 +4,7 @@
     "version_min":300000,
     "title":"Testing CtlRuleRemoteTargetByTag (1)",
     "expected":{
-      "debug_log": "Variable: ARGS:pwd was excluded by ruleRemoteTargetByTag..."
+      "http_code": 200
     },
     "client":{
       "ip":"200.249.12.31",
@@ -27,8 +27,9 @@
       "port":80
     },
     "rules":[
+        "SecRuleEngine On",
         "SecRule REQUEST_FILENAME \"@endsWith /wp-login.php\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetByTag=CRS;ARGS:pwd\"",
-        "SecRule ARGS \"@contais whe\" \"id:1,phase:3,t:none,nolog,pass,tag:'CRS'\""
+        "SecRule ARGS \"@contains lhebs\" \"id:1,phase:3,t:none,nolog,pass,tag:'CRS',deny\""
     ]
   },
   {

test/test-cases/regression/config-update-target-by-id.json

@@ -31,11 +31,11 @@
       ]
     },
     "expected":{
-      "debug_log":"ARGS:key is part of the exclusion list \\(from update by ID\\), skipping"
+      "http_code": 200
     },
     "rules":[
       "SecRuleUpdateTargetById 1 !ARGS",
-      "SecRule ARGS \"@contains test\" \"id:1,pass,t:trim,tag:'test'\""
+      "SecRule ARGS \"@contains value\" \"id:1,pass,t:trim,tag:'test',deny\""
     ]
   }
 ]

test/test-cases/regression/config-update-target-by-msg.json

@@ -31,11 +31,12 @@
       ]
     },
     "expected":{
-      "debug_log":"ARGS:key is part of the exclusion list \\(from update by msg\\), skipping"
+      "http_code": 200
     },
     "rules":[
+      "SecRuleEngine On",
       "SecRuleUpdateTargetByMsg test !ARGS",
-      "SecRule ARGS \"@contains test\" \"id:1,pass,t:trim,msg:'test'\""
+      "SecRule ARGS \"@contains value\" \"id:1,pass,t:trim,msg:'test',deny\""
     ]
   }
 ]

test/test-cases/regression/config-update-target-by-tag.json

@@ -31,11 +31,12 @@
       ]
     },
     "expected":{
-      "debug_log":"ARGS:key is part of the exclusion list \\(from update by tag\\), skipping"
+      "http_code": 200
     },
     "rules":[
+      "SecRuleEngine On",
       "SecRuleUpdateTargetByTag test !ARGS",
-      "SecRule ARGS \"@contains test\" \"id:1,pass,t:trim,tag:'test'\""
+      "SecRule ARGS \"@contains value\" \"id:1,pass,t:trim,tag:'test',deny\""
     ]
   }
 ]