diff --git a/src/actions/tag.cc b/src/actions/tag.cc index 120aba18d..e00513534 100644 --- a/src/actions/tag.cc +++ b/src/actions/tag.cc @@ -57,6 +57,14 @@ std::string Tag::getName(Transaction *transaction) { } +bool Tag::hasName(std::string_view name, Transaction *transaction) { + if (m_staticName) { + return *m_staticName == name; + } + return getName(transaction) == name; +} + + bool Tag::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) { std::string tag = getName(transaction); ms_dbg_a(transaction, 9, "Rule tag: " + tag); diff --git a/src/actions/tag.h b/src/actions/tag.h index eb643cd5f..1d1528ebf 100644 --- a/src/actions/tag.h +++ b/src/actions/tag.h @@ -14,7 +14,9 @@ */ #include +#include #include +#include #include #include "modsecurity/actions/action.h" @@ -34,14 +36,23 @@ class Tag : public Action { public: explicit Tag(std::unique_ptr z) : Action("tag"), - m_string(std::move(z)) { } + m_string(std::move(z)), + m_staticName(m_string->containsMacro() + ? std::nullopt + : std::optional(m_string->evaluate())) { } std::string getName(Transaction *transaction); + bool hasName(std::string_view name, Transaction *transaction); bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override; protected: std::unique_ptr m_string; + + private: + // Without macros, the name is the same in every transaction and is + // evaluated once. Empty if the tag contains macros. + const std::optional m_staticName; }; diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc index f6642b67e..07477b07f 100644 --- a/src/rule_with_actions.cc +++ b/src/rule_with_actions.cc @@ -443,7 +443,7 @@ void RuleWithActions::executeTransformations( bool RuleWithActions::containsTag(const std::string& name, Transaction *t) { for (auto &tag : m_actionsTag) { - if (tag != NULL && tag->getName(t) == name) { + if (tag != NULL && tag->hasName(name, t)) { return true; } } diff --git a/test/test-cases/regression/action-ctl_rule_remove_by_tag.json b/test/test-cases/regression/action-ctl_rule_remove_by_tag.json index 1e8edb7f2..017185f85 100644 --- a/test/test-cases/regression/action-ctl_rule_remove_by_tag.json +++ b/test/test-cases/regression/action-ctl_rule_remove_by_tag.json @@ -135,5 +135,97 @@ "rules": [ "SecRule ARGS:key \".\" \"id:4,ctl:ruleRemoveByTag,whee\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing ctl:ruleRemoveByTag with a macro in the tag, removal applies", + "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", + "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": { + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=a\"", + "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveByTag=test-a\"", + "SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing ctl:ruleRemoveByTag with a macro in the tag, removal does not apply", + "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", + "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": { + "http_code": 403 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=b\"", + "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveByTag=test-a\"", + "SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\"" + ] } ] diff --git a/test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json b/test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json index 9f92cae4f..08996caab 100644 --- a/test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json +++ b/test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json @@ -334,5 +334,97 @@ "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:9002100,phase:2,pass,ctl:ruleRemoveTargetByTag=CRS;ARGS:foo'bar\"", "SecRule ARGS \"@contains attack\" \"id:1,phase:3,status:403,block,deny,tag:'CRS'\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing CtlRuleRemoveTargetByTag with a macro in the tag, removal applies", + "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", + "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": { + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=a\"", + "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveTargetByTag=test-a;ARGS:key\"", + "SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing CtlRuleRemoveTargetByTag with a macro in the tag, removal does not apply", + "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", + "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": { + "http_code": 403 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=b\"", + "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveTargetByTag=test-a;ARGS:key\"", + "SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\"" + ] } ] diff --git a/test/test-cases/regression/config-update-target-by-tag.json b/test/test-cases/regression/config-update-target-by-tag.json index 32167c08f..090691120 100644 --- a/test/test-cases/regression/config-update-target-by-tag.json +++ b/test/test-cases/regression/config-update-target-by-tag.json @@ -313,5 +313,97 @@ "SecRuleUpdateTargetByTag test !ARGS:ref", "SecRule ARGS \"@contains something\" \"id:1,pass,t:trim,tag:'test',deny\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "SecRuleUpdateTargetByTag Test (8/6) Tag with a macro, exclusion applies", + "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", + "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": { + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=a\"", + "SecRuleUpdateTargetByTag test-a !ARGS", + "SecRule ARGS \"@contains value\" \"id:2,phase:2,t:trim,tag:'test-%{tx.suffix}',deny\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "SecRuleUpdateTargetByTag Test (9/6) Tag with a macro, exclusion does not apply", + "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", + "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": { + "http_code": 403 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=b\"", + "SecRuleUpdateTargetByTag test-a !ARGS", + "SecRule ARGS \"@contains value\" \"id:2,phase:2,t:trim,tag:'test-%{tx.suffix}',deny\"" + ] } ]