Adds support to multiple ranges in ctl:ruleRemoveTargetById (#2110)

CHANGES

@@ -1,6 +1,8 @@
 v3.0.4 - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+ - Adds support to multiple ranges in ctl:ruleRemoveTargetById
+   [Issue #2110 - @j0k2r]
  - Making block action execution dependent of the SecEngine status
    [Issue #2113, #2111 - @theMiddleBlue, @airween]
  - Making block action execution dependent of the SecEngine status

Makefile.am

@@ -134,6 +134,7 @@ TESTS+=test/test-cases/regression/config-update-target-by-tag.json
 TESTS+=test/test-cases/regression/config-xml_external_entity.json
 TESTS+=test/test-cases/regression/debug_log.json
 TESTS+=test/test-cases/regression/directive-sec_rule_script.json
+TESTS+=test/test-cases/regression/issue-2110.json
 TESTS+=test/test-cases/regression/issue-1152.json
 TESTS+=test/test-cases/regression/issue-1528.json
 TESTS+=test/test-cases/regression/issue-1565.json

headers/modsecurity/transaction.h

@@ -477,6 +477,7 @@ class Transaction : public TransactionAnchoredVariables {
      *
      */
     std::list< std::pair<int, std::string> > m_ruleRemoveTargetById;
+    std::list< std::pair<std::pair<int, int>, std::string> > m_ruleRemoveTargetByIdRange;
 
     /**
      *

src/actions/ctl/rule_remove_target_by_id.cc

@@ -33,28 +33,73 @@ bool RuleRemoveTargetById::init(std::string *error) {
     std::string what(m_parser_payload, 21, m_parser_payload.size() - 21);
     std::vector<std::string> param = utils::string::split(what, ';');
 
+    bool added = false;
+
     if (param.size() < 2) {
         error->assign(what + " is not a valid `ID;VARIABLE'");
         return false;
     }
 
-    try {
-        m_id = std::stoi(param[0]);
-    } catch(...) {
-        error->assign("Not able to convert '" + param[0] +
-            "' into a number");
-        return false;
+    size_t dash = param[0].find('-');
+    if (dash != std::string::npos) {
+        std::string n1s = std::string(param[0], 0, dash);
+        std::string n2s = std::string(param[0], dash + 1, param[0].size() - (dash + 1));
+        int n1n = 0;
+        int n2n = 0;
+        try {
+            n1n = std::stoi(n1s);
+            added = true;
+        } catch (...) {
+            error->assign("Not a number: " + n1s);
+            return false;
+        }
+        try {
+            n2n = std::stoi(n2s);
+            added = true;
+        } catch (...) {
+            error->assign("Not a number: " + n2s);
+            return false;
+        }
+
+        if (n1n > n2n) {
+            error->assign("Invalid range: " + param[0]);
+            return false;
+        }
+        m_ranges.push_back(std::make_pair(n1n, n2n));
+        added = true;
+    } else {
+        try {
+            int num = std::stoi(param[0]);
+            m_ids.push_back(num);
+            added = true;
+        } catch (...) {
+            error->assign("Not able to convert '" + param[0] +
+              "' into a number");
+            return false;
+        }
     }
 
-    m_target = param[1];
+    if (added) {
+      m_target = param[1];
+      return true;
+    }
 
-    return true;
+    error->assign("Not a number or range: " + what);
+    return false;
 }
 
 bool RuleRemoveTargetById::evaluate(Rule *rule, Transaction *transaction) {
+  for (auto &i : m_ids) {
     transaction->m_ruleRemoveTargetById.push_back(
-        std::make_pair(m_id, m_target));
-    return true;
+      std::make_pair(i, m_target));
+  }
+
+  for (auto &i : m_ranges) {
+      transaction->m_ruleRemoveTargetByIdRange.push_back(
+        std::make_pair(i, m_target));
+  }
+
+  return true;
 }
 
 

src/actions/ctl/rule_remove_target_by_id.h

@@ -30,14 +30,13 @@ namespace ctl {
 class RuleRemoveTargetById : public Action {
  public:
     explicit RuleRemoveTargetById(std::string action)
-        : Action(action, RunTimeOnlyIfMatchKind),
-        m_id(0),
-        m_target("") { }
+        : Action(action, RunTimeOnlyIfMatchKind) { }
 
     bool init(std::string *error) override;
     bool evaluate(Rule *rule, Transaction *transaction) override;
 
-    int m_id;
+    std::list<std::pair<int, int> > m_ranges;
+    std::list<int> m_ids;
     std::string m_target;
 };
 

src/rule.cc

@@ -710,6 +710,23 @@ bool Rule::evaluate(Transaction *trans,
                 v = NULL;
                 continue;
             }
+
+	    if (exclusion.contains(v->m_key) ||
+                std::find_if(trans->m_ruleRemoveTargetByIdRange.begin(),
+                    trans->m_ruleRemoveTargetByIdRange.end(),
+                    [&, v, this](std::pair<std::pair<int, int>, std::string> &m) -> bool {
+                      if (m.first.first <= m_ruleId && m.first.second >= m_ruleId) {
+                        return m.second == v->m_key;
+                      }
+
+		      return false;
+                    }) != trans->m_ruleRemoveTargetByIdRange.end()
+            ) {
+                delete v;
+                v = NULL;
+                continue;
+            }
+
             if (exclusion.contains(v->m_key) ||
                 std::find_if(trans->m_ruleRemoveTargetByTag.begin(),
                     trans->m_ruleRemoveTargetByTag.end(),

test/test-cases/regression/issue-2110.json

@@ -0,0 +1,33 @@
+[
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing ctl:ruleRemoveTargetById - issue 1444",
+    "expected":{
+      "http_code":200
+    },
+    "client":{
+      "ip":"127.0.0.1",
+      "port":123
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*"
+      },
+      "uri":"index.php?foo=bar&z=xxx",
+      "method":"GET",
+      "body": ""
+    },
+    "server":{
+      "ip":"127.0.0.1",
+      "port":80
+    },
+    "rules":[
+        "SecRuleEngine On",
+        "SecRule ARGS:foo \"@rx ^bar$\" \"id:100,phase:1,ctl:ruleRemoveTargetById=1000-1999;ARGS:z\"",
+        "SecRule ARGS:z \"@rx ^xxx$\" \"id:1010,phase:1,deny,status:403\""
+    ]
+  }
+]