fix(lexer): rejection of '@' in ctl:ruleRemoveTarget actions (#3565)

src/actions/ctl/rule_remove_by_id.cc

@@ -27,7 +27,12 @@ namespace ctl {
 
 
 bool RuleRemoveById::init(std::string *error) {
-    std::string what(m_parser_payload, 15, m_parser_payload.size() - 15);
+    size_t pos = m_parser_payload.find("=");
+    if (pos == std::string::npos) {
+        error->assign(m_parser_payload + " is not a valid ID or range");
+        return false;
+    }
+    std::string what = m_parser_payload.substr(pos + 1);
     bool added = false;
     std::vector<std::string> toRemove = utils::string::ssplit(what, ' ');
     for (const std::string &a : toRemove) {

src/actions/ctl/rule_remove_by_tag.cc

@@ -26,7 +26,12 @@ namespace ctl {
 
 
 bool RuleRemoveByTag::init(std::string *error) {
-    std::string what(m_parser_payload, 16, m_parser_payload.size() - 16);
+    size_t pos = m_parser_payload.find("=");
+    if (pos == std::string::npos) {
+        error->assign(m_parser_payload + " is not a valid tag");
+        return false;
+    }
+    std::string what = m_parser_payload.substr(pos + 1);
     m_tag = what;
 
     return true;

src/actions/ctl/rule_remove_target_by_id.cc

@@ -1,6 +1,7 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
  * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *               2024 - 2026 OWASP (https://owasp.org)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -30,11 +31,16 @@ namespace ctl {
 
 
 bool RuleRemoveTargetById::init(std::string *error) {
-    std::string what(m_parser_payload, 21, m_parser_payload.size() - 21);
+    size_t pos = m_parser_payload.find("=");
+    if (pos == std::string::npos) {
+        error->assign(m_parser_payload + " is not a valid 'ID;VARIABLE'");
+        return false;
+    }
+    std::string what = m_parser_payload.substr(pos + 1);
     std::vector<std::string> param = utils::string::split(what, ';');
 
     if (param.size() < 2) {
-        error->assign(what + " is not a valid `ID;VARIABLE'");
+        error->assign(what + " is not a valid 'ID;VARIABLE'");
         return false;
     }
 

src/actions/ctl/rule_remove_target_by_tag.cc

@@ -1,6 +1,7 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
  * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *               2024 - 2026 OWASP (https://owasp.org)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -30,11 +31,16 @@ namespace ctl {
 
 
 bool RuleRemoveTargetByTag::init(std::string *error) {
-    std::string what(m_parser_payload, 22, m_parser_payload.size() - 22);
+    size_t pos = m_parser_payload.find("=");
+    if (pos == std::string::npos) {
+        error->assign(m_parser_payload + " is not a valid 'TAG;VARIABLE'");
+        return false;
+    }
+    std::string what = m_parser_payload.substr(pos + 1);
     std::vector<std::string> param = utils::string::split(what, ';');
 
     if (param.size() < 2) {
-        error->assign(what + " is not a valid `TAG;VARIABLE'");
+        error->assign(what + " is not a valid 'TAG;VARIABLE'");
         return false;
     }
 

src/parser/seclang-scanner.ll

@@ -457,7 +457,7 @@ FREE_TEXT_COMMA_QUOTE                   [^,\"\\n\\r]+
 NEW_LINE_FREE_TEXT                      [^, \t\"\n\r]+
 NOT !
 FREE_TEXT                               ([^\"]|([^\\]\\\"))+
-REMOVE_RULE_BY                          [0-9A-Za-z_\/\.\-\*\:\;\]\[\$]+
+REMOVE_RULE_BY                          [a-zA-Z0-9_\-\.\*\/\:\;\$@\=\(\)\[\]\']+
 
 VAR_FREE_TEXT_QUOTE                     ([^\']|([^\\]\\\'))+
 VAR_FREE_TEXT_SPACE                     [^ \t\"]+

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

@@ -86,5 +86,48 @@
       "SecRule REQUEST_FILENAME \"@endsWith /wp-login.php\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveById=123\"",
       "SecRule ARGS_NAMES \"@contains whee\" \"id:1,phase:3,t:none,nolog,pass,tag:'CRS2'\""
     ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveById syntax check (Issue 3565)",
+    "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": "*/*",
+        "Cookie": "PHPSESSID=rAAAAAAA2t5uvjq435r4q7ib3vtdjq120",
+        "Content-Type": "text/xml",
+        "Content-Length": "0"
+      },
+      "uri": "/wp-login.php?whee=something&pwd=lhebs",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "response": {
+      "headers": {
+        "Content-Length": "0"
+      },
+      "body": [
+        ""
+      ]
+    },
+    "expected": {
+      "parser_error": "Line: 1. Column: 113. Expecting an action, got:  ctl:ruleRemoveById,123"
+    },
+    "rules": [
+      "SecRule REQUEST_FILENAME \"@endsWith /wp-login.php\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveById,123\"",
+      "SecRule ARGS_NAMES \"@contains whee\" \"id:1,phase:3,t:none,nolog,pass,tag:'CRS2'\""
+    ]
   }
 ]

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

@@ -92,5 +92,48 @@
       "SecRule ARGS \"@contains test\" \"id:2,pass,t:trim\"",
       "SecRule ARGS \"@contains test\" \"id:3,pass,t:trim,tag:whee\""
     ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing ctl:ruleRemoveByTag syntax check",
+    "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": "*/*",
+        "Content-Length": "0"
+      },
+      "uri": "/?key=value&key=other_value",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "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",
+        "Content-Length": "8"
+      },
+      "body": [
+        "no need."
+      ]
+    },
+    "expected": {
+      "parser_error": "Line: 1. Column: 54. Expecting an action, got:  ctl:ruleRemoveByTag,whee"
+    },
+    "rules": [
+      "SecRule ARGS:key \".\" \"id:4,ctl:ruleRemoveByTag,whee\""
+    ]
   }
 ]

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

@@ -219,5 +219,209 @@
       "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:1,phase:1,pass,t:none,ctl:ruleRemoveTargetById=2;REQUEST_HEADERS:referer\"",
       "SecRule REQUEST_HEADERS:Referer \"@contains attack\" \"id:2,phase:1,deny,t:none,log\""
     ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveTargetById syntax I (Issue 3565)",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "Accept": "*/*",
+        "Content-Type": "application/x-www-form-urlencoded"
+      },
+      "uri": "/index.html?@foo=attack",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "response": {
+      "headers": {},
+      "body": [
+        ""
+      ]
+    },
+    "expected": {
+      "parser_error": "Line: 2. Column: 47. 1 is not a valid 'ID;VARIABLE'"
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@endsWith /index.html\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetById=1,ARGS\"",
+      "SecRule ARGS \"@contains attack\" \"id:1,phase:3,t:none,status:403,block,deny\""
+    ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveTargetById syntax II (Issue 3565)",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "Accept": "*/*",
+        "Content-Type": "application/x-www-form-urlencoded"
+      },
+      "uri": "/index.html?@foo=attack",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "response": {
+      "headers": {},
+      "body": [
+        ""
+      ]
+    },
+    "expected": {
+      "parser_error": "Line: 2. Column: 120. Expecting an action, got:  ctl:ruleRemoveTargetById,1,ARGS"
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@endsWith /index.html\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetById,1,ARGS\"",
+      "SecRule ARGS \"@contains attack\" \"id:1,phase:3,t:none,status:403,block,deny\""
+    ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveTargetById with @ character (Issue 3565)",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "Accept": "*/*",
+        "Content-Type": "application/x-www-form-urlencoded"
+      },
+      "uri": "/index.html?@foo=attack",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "response": {
+      "headers": {},
+      "body": [
+        ""
+      ]
+    },
+    "expected": {
+      "http_code": 200
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@endsWith /index.html\" \"id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetById=1;ARGS:@foo\"",
+      "SecRule ARGS \"@contains attack\" \"id:1,phase:3,t:none,status:403,block,deny\""
+    ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveTargetById with = character",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "Accept": "*/*"
+      },
+      "uri": "/index.html?foo%3Dbar=attack",
+      "method": "GET"
+    },
+    "expected": {
+      "http_code": 200
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:9002100,phase:2,pass,ctl:ruleRemoveTargetById=1;ARGS:foo=bar\"",
+      "SecRule ARGS \"@contains attack\" \"id:1,phase:3,status:403,block,deny\""
+    ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveTargetById with ( and ) characters",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "Accept": "*/*"
+      },
+      "uri": "/index.html?foo(bar)=attack",
+      "method": "GET"
+    },
+    "expected": {
+      "http_code": 200
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:9002100,phase:2,pass,ctl:ruleRemoveTargetById=1;ARGS:foo(bar)\"",
+      "SecRule ARGS \"@contains attack\" \"id:1,phase:3,status:403,block,deny\""
+    ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Testing CtlRuleRemoveTargetById with ' character",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "Accept": "*/*"
+      },
+      "uri": "/index.html?foo'bar=attack",
+      "method": "GET"
+    },
+    "expected": {
+      "http_code": 200
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:9002100,phase:2,pass,ctl:ruleRemoveTargetById=1;ARGS:foo'bar\"",
+      "SecRule ARGS \"@contains attack\" \"id:1,phase:3,status:403,block,deny\""
+    ]
   }
 ]