diff --git a/headers/modsecurity/rule.h b/headers/modsecurity/rule.h index 38476817..66a6ccbf 100644 --- a/headers/modsecurity/rule.h +++ b/headers/modsecurity/rule.h @@ -76,7 +76,7 @@ class Rule { std::vector getActionNames(); std::vector getActionsByName(const std::string& name); bool containsTag(const std::string& name, Transaction *t); - + bool containsDisruptiveAction(); int refCountDecreaseAndCheck() { m_referenceCount--; diff --git a/src/actions/disruptive/allow.h b/src/actions/disruptive/allow.h index b28d63ba..8c129091 100644 --- a/src/actions/disruptive/allow.h +++ b/src/actions/disruptive/allow.h @@ -60,6 +60,7 @@ class Allow : public Action { bool init(std::string *error) override; bool evaluate(Rule *rule, Transaction *transaction) override; + bool isDisruptive() override { return true; } AllowType m_allowType; diff --git a/src/parser/driver.cc b/src/parser/driver.cc index 53dca136..f6188edd 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -70,22 +70,33 @@ int Driver::addSecRule(Rule *rule) { return false; } - if (lastRule && lastRule->m_chained && lastRule->m_chainedRule == NULL) { - rule->m_phase = lastRule->m_phase; - lastRule->m_chainedRule = rule; - return true; + if (lastRule && lastRule->m_chained) { + if (lastRule->m_chainedRule == NULL) { + rule->m_phase = lastRule->m_phase; + lastRule->m_chainedRule = rule; + if (rule->containsDisruptiveAction()) { + m_parserError << "Disruptive actions can only be specified by"; + m_parserError << " chain starter rules."; + return false; + } + return true; + } else { + Rule *a = lastRule->m_chainedRule; + while (a->m_chained && a->m_chainedRule != NULL) { + a = a->m_chainedRule; + } + if (a->m_chained && a->m_chainedRule == NULL) { + a->m_chainedRule = rule; + if (a->containsDisruptiveAction()) { + m_parserError << "Disruptive actions can only be "; + m_parserError << "specified by chain starter rules."; + return false; + } + return true; + } + } } - if (lastRule && lastRule->m_chained && lastRule->m_chainedRule != NULL) { - Rule *a = lastRule->m_chainedRule; - while (a->m_chained && a->m_chainedRule != NULL) { - a = a->m_chainedRule; - } - if (a->m_chained && a->m_chainedRule == NULL) { - a->m_chainedRule = rule; - return true; - } - } /* * Checking if the rule has an ID and also checking if this ID is not used diff --git a/src/rule.cc b/src/rule.cc index 946a4e1a..1e7c8ce0 100644 --- a/src/rule.cc +++ b/src/rule.cc @@ -706,6 +706,27 @@ end_exec: } +bool Rule::containsDisruptiveAction() { + for (Action *a : m_actionsRuntimePos) { + if (a->isDisruptive() == true) { + return true; + } + } + for (Action *a : m_actionsRuntimePre) { + if (a->isDisruptive() == true) { + return true; + } + } + for (Action *a : m_actionsConf) { + if (a->isDisruptive() == true) { + return true; + } + } + + return false; +} + + std::vector Rule::getActionsByName(const std::string& name) { std::vector ret; for (auto &z : m_actionsRuntimePos) { diff --git a/test/test-cases/regression/config-calling_phases_by_name.json b/test/test-cases/regression/config-calling_phases_by_name.json index 8e1be359..56f10b90 100644 --- a/test/test-cases/regression/config-calling_phases_by_name.json +++ b/test/test-cases/regression/config-calling_phases_by_name.json @@ -2,7 +2,7 @@ { "enabled":1, "version_min":300000, - "title":"Testing Variables :: MATCHED_VAR (1/2)", + "title":"Testing Config :: Phases by name (1/2)", "client":{ "ip":"200.249.12.31", "port":123 @@ -35,14 +35,14 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:key \"@contains other_value\" \"id:1,phase:request,chain\"", - "SecRule MATCHED_VAR \"@contains asdf\" \"phase:request,pass\"" + "SecRule ARGS:key \"@contains other_value\" \"id:1,phase:request,pass,chain\"", + "SecRule MATCHED_VAR \"@contains asdf\" \"\"" ] }, { "enabled":1, "version_min":300000, - "title":"Testing Variables :: MATCHED_VAR (2/2)", + "title":"Testing Config :: Phases by name (2/2)", "client":{ "ip":"200.249.12.31", "port":123 @@ -75,8 +75,8 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:key \"@contains other_value\" \"chain,phase:response,id:28\"", - "SecRule MATCHED_VAR \"@contains Aasdf\" \"pass\"", + "SecRule ARGS:key \"@contains other_value\" \"chain,pass,phase:response,id:28\"", + "SecRule MATCHED_VAR \"@contains Aasdf\" \"\"", "SecRule MATCHED_VAR \"@contains other_value\" \"id:29,phase:response,pass\"", "SecRule MATCHED_VAR \"@contains other_value\" \"id:30,phase:response,pass\"" ] diff --git a/test/test-cases/regression/variable-MATCHED_VAR.json b/test/test-cases/regression/variable-MATCHED_VAR.json index 46ed401e..84ee940e 100644 --- a/test/test-cases/regression/variable-MATCHED_VAR.json +++ b/test/test-cases/regression/variable-MATCHED_VAR.json @@ -35,8 +35,8 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:key \"@contains other_value\" \"chain,id:28\"", - "SecRule MATCHED_VAR \"@contains asdf\" \"pass\"" + "SecRule ARGS:key \"@contains other_value\" \"chain,id:28,pass\"", + "SecRule MATCHED_VAR \"@contains asdf\" \"\"" ] }, { @@ -75,8 +75,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:key \"@contains other_value\" \"chain,id:28\"", - "SecRule MATCHED_VAR \"@contains Aasdf\" \"pass\"", + "SecRule ARGS:key \"@contains other_value\" \"chain,id:28,pass\"", + "SecRule MATCHED_VAR \"@contains Aasdf\" \"\"", + "SecRule MATCHED_VAR \"@contains other_value\" \"id:29,pass\"", "SecRule MATCHED_VAR \"@contains other_value\" \"id:30,pass\"" ] diff --git a/test/test-cases/regression/variable-MATCHED_VARS.json b/test/test-cases/regression/variable-MATCHED_VARS.json index ecece68e..7d469a0f 100644 --- a/test/test-cases/regression/variable-MATCHED_VARS.json +++ b/test/test-cases/regression/variable-MATCHED_VARS.json @@ -35,9 +35,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:keyI \"@contains value\" \"chain,id:28\"", + "SecRule ARGS:keyI \"@contains value\" \"chain,id:28,pass\"", "SecRule ARGS:keyII \"@contains other_value\" \"chain\"", - "SecRule MATCHED_VARS \"@contains asdf\" \"pass\"" + "SecRule MATCHED_VARS \"@contains asdf\" \"\"" ] }, { @@ -76,9 +76,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:keyI \"@contains value\" \"chain,id:28\"", + "SecRule ARGS:keyI \"@contains value\" \"chain,id:28,pass\"", "SecRule ARGS:keyII \"@contains other_value\" \"chain\"", - "SecRule MATCHED_VARS \"@contains asdf\" \"pass\"", + "SecRule MATCHED_VARS \"@contains asdf\" \"\"", "SecRule MATCHED_VARS \"@contains value\" \"id:29\"" ] } diff --git a/test/test-cases/regression/variable-MATCHED_VARS_NAMES.json b/test/test-cases/regression/variable-MATCHED_VARS_NAMES.json index 6fa33920..a734e71b 100644 --- a/test/test-cases/regression/variable-MATCHED_VARS_NAMES.json +++ b/test/test-cases/regression/variable-MATCHED_VARS_NAMES.json @@ -35,9 +35,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:keyI \"@contains value\" \"chain,id:28\"", + "SecRule ARGS:keyI \"@contains value\" \"chain,id:28,pass\"", "SecRule ARGS:keyII \"@contains other_value\" \"chain\"", - "SecRule MATCHED_VARS_NAMES \"@contains asdf\" \"pass\"" + "SecRule MATCHED_VARS_NAMES \"@contains asdf\" \"\"" ] }, { @@ -76,9 +76,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:keyI \"@contains value\" \"chain,id:28\"", + "SecRule ARGS:keyI \"@contains value\" \"chain,id:28,pass\"", "SecRule ARGS:keyII \"@contains other_value\" \"chain\"", - "SecRule MATCHED_VARS_NAMES \"@contains asdf\" \"pass\"", + "SecRule MATCHED_VARS_NAMES \"@contains asdf\" \"\"", "SecRule MATCHED_VARS_NAMES \"@contains value\" \"id:29\"" ] } diff --git a/test/test-cases/regression/variable-MATCHED_VAR_NAME.json b/test/test-cases/regression/variable-MATCHED_VAR_NAME.json index f6792a21..318a3206 100644 --- a/test/test-cases/regression/variable-MATCHED_VAR_NAME.json +++ b/test/test-cases/regression/variable-MATCHED_VAR_NAME.json @@ -35,9 +35,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:keyI \"@contains value\" \"chain,id:28\"", + "SecRule ARGS:keyI \"@contains value\" \"chain,id:28,pass\"", "SecRule ARGS:keyII \"@contains other_value\" \"chain\"", - "SecRule MATCHED_VAR_NAME \"@contains asdf\" \"pass\"" + "SecRule MATCHED_VAR_NAME \"@contains asdf\" \"\"" ] }, { @@ -76,9 +76,9 @@ }, "rules":[ "SecRuleEngine On", - "SecRule ARGS:keyI \"@contains value\" \"chain,id:28\"", + "SecRule ARGS:keyI \"@contains value\" \"chain,id:28,pass\"", "SecRule ARGS:keyII \"@contains other_value\" \"chain\"", - "SecRule MATCHED_VAR_NAME \"@contains asdf\" \"pass\"", + "SecRule MATCHED_VAR_NAME \"@contains asdf\" \"\"", "SecRule MATCHED_VAR_NAME \"@contains value\" \"id:29\"" ] }