Fix rule-update-target exclusions for plain (non-regex) variables

CHANGES

@@ -1,6 +1,8 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 -------------------------------------
  
+  - Fix rule-update-target for non-regex
+    [Issue 2251 - @martinhsv]
   - Fix configure script when packaging for Buildroot
     [Issue 2235 - @frankvanbever]
   - modsecurity.pc.in: add Libs.private

src/rule.cc

@@ -697,7 +697,7 @@ bool Rule::evaluate(Transaction *trans,
             const std::string &value = v->getValue();
             const std::string &key = v->getKeyWithCollection();
 
-            if (exclusion.contains(v->getKey()) ||
+            if (exclusion.contains(v) ||
                 std::find_if(trans->m_ruleRemoveTargetById.begin(),
                     trans->m_ruleRemoveTargetById.end(),
                     [&, v, this](std::pair<int, std::string> &m) -> bool {
@@ -708,7 +708,7 @@ bool Rule::evaluate(Transaction *trans,
                 v = NULL;
                 continue;
             }
-            if (exclusion.contains(v->getKey()) ||
+            if (exclusion.contains(v) ||
                 std::find_if(trans->m_ruleRemoveTargetByTag.begin(),
                     trans->m_ruleRemoveTargetByTag.end(),
                     [&, v, trans, this](std::pair<std::string, std::string> &m) -> bool {

src/variables/variable.h

@@ -610,14 +610,14 @@ class Variables : public std::vector<Variable *> {
         return std::find_if(begin(), end(),
             [v](Variable *m) -> bool { return *v == *m; }) != end();
     };
-    bool contains(const std::string &v) {
+    bool contains(const VariableValue *v) {
         return std::find_if(begin(), end(),
             [v](Variable *m) -> bool {
                 VariableRegex *r = dynamic_cast<VariableRegex *>(m);
                 if (r) {
-                    return r->m_r.searchAll(v).size() > 0;
+                    return r->m_r.searchAll(v->getKey()).size() > 0;
                 }
-                return v == *m->m_fullName.get();
+                return v->getKeyWithCollection() == *m->m_fullName.get();
             }) != end();
     };
 };

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

@@ -2,7 +2,7 @@
   {
     "enabled":1,
     "version_min":300000,
-    "title":"SecRuleUpdateTargetById",
+    "title":"SecRuleUpdateTargetById - exclude whole collection",
     "client":{
       "ip":"200.249.12.31",
       "port":123
@@ -34,6 +34,7 @@
       "http_code": 200
     },
     "rules":[
+      "SecRuleEngine On",
       "SecRuleUpdateTargetById 1 !ARGS",
       "SecRule ARGS \"@contains value\" \"id:1,pass,t:trim,tag:'test',deny\""
     ]
@@ -41,7 +42,7 @@
   {
     "enabled":1,
     "version_min":300000,
-    "title":"SecRuleUpdateTargetById",
+    "title":"SecRuleUpdateTargetById - exclude using regex",
     "client":{
       "ip":"200.249.12.31",
       "port":123
@@ -56,7 +57,7 @@
         "User-Agent":"curl/7.38.0",
         "Accept":"*/*"
       },
-      "uri":"/?mixpanel$=value&mixpanel$=other_value",
+      "uri":"/?mixpanel=value&mixpanel=other_value",
       "method":"GET"
     },
     "response":{
@@ -73,8 +74,49 @@
       "http_code": 200
     },
     "rules":[
+      "SecRuleEngine On",
       "SecRuleUpdateTargetById 1 !ARGS:/mixpanel$/",
       "SecRule ARGS \"@contains value\" \"id:1,pass,t:trim,tag:'test',deny\""
     ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"SecRuleUpdateTargetById - exclude using full name",
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*"
+      },
+      "uri":"/?mixpanel=value",
+      "method":"GET"
+    },
+    "response":{
+      "headers":{
+        "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+        "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+        "Content-Type":"text/html"
+      },
+      "body":[
+        "no need."
+      ]
+    },
+    "expected":{
+      "http_code": 200
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecRuleUpdateTargetById 1 !ARGS:mixpanel",
+      "SecRule ARGS \"@contains value\" \"id:1,t:trim,tag:'test',deny\""
+    ]
   }
 ]

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

@@ -238,5 +238,45 @@
          "SecRuleUpdateTargetByTag test !ARGS:'/^ref/'",
          "SecRule ARGS \"@contains something\" \"id:1,pass,t:trim,tag:'test',deny\""
        ]
+  },
+  {
+       "enabled":1,
+       "version_min":300000,
+       "title":"SecRuleUpdateTargetByTag Test (7/6) Exclusion by full name",
+       "client":{
+         "ip":"200.249.12.31",
+         "port":123
+       },
+       "server":{
+         "ip":"200.249.12.31",
+         "port":80
+       },
+       "request":{
+         "headers":{
+              "Host":"localhost",
+              "User-Agent":"curl/7.38.0",
+              "Accept":"*/*"
+         },
+         "uri":"/?key=value&ref=something",
+         "method":"GET"
+       },
+       "response":{
+         "headers":{
+              "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+              "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+              "Content-Type":"text/html"
+         },
+         "body":[
+              "no need."
+         ]
+       },
+       "expected":{
+         "http_code": 200
+       },
+       "rules":[
+         "SecRuleEngine On",
+         "SecRuleUpdateTargetByTag test !ARGS:ref",
+         "SecRule ARGS \"@contains something\" \"id:1,pass,t:trim,tag:'test',deny\""
+       ]
   }
 ]