Skip to content

Refactoring the cleaning of MATCHED_VAR* variables #3422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion headers/modsecurity/rule_with_operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class RuleWithOperator : public RuleWithActions {

static void updateMatchedVars(Transaction *trasn, const std::string &key,
const std::string &value);
static void cleanMatchedVars(Transaction *trasn);


const std::string& getOperatorName() const;
Expand Down
2 changes: 2 additions & 0 deletions headers/modsecurity/rules_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ class RulesSet : public RulesSetProperties {
void debug(int level, const std::string &id, const std::string &uri,
const std::string &msg);

static void cleanMatchedVars(Transaction *trans);

RulesSetPhases m_rulesSetPhases;
private:
#ifndef NO_LOGS
Expand Down
12 changes: 0 additions & 12 deletions src/rule_with_operator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,6 @@ void RuleWithOperator::updateMatchedVars(Transaction *trans, const std::string &
}


void RuleWithOperator::cleanMatchedVars(Transaction *trans) {
ms_dbg_a(trans, 9, "Matched vars cleaned.");
// cppcheck-suppress ctunullpointer
trans->m_variableMatchedVar.unset();
trans->m_variableMatchedVars.unset();
trans->m_variableMatchedVarName.unset();
trans->m_variableMatchedVarsNames.unset();
}



bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string &key,
const std::string &value, RuleMessage &ruleMessage) {
#if MSC_EXEC_CLOCK_ENABLED
Expand Down Expand Up @@ -324,7 +313,6 @@ bool RuleWithOperator::evaluate(Transaction *trans,

if (globalRet == false) {
ms_dbg_a(trans, 4, "Rule returned 0.");
cleanMatchedVars(trans);
goto end_clean;
}
ms_dbg_a(trans, 4, "Rule returned 1.");
Expand Down
9 changes: 9 additions & 0 deletions src/rules_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ std::string RulesSet::getParserError() {
return this->m_parserError.str();
}

void RulesSet::cleanMatchedVars(Transaction *trans) {
ms_dbg_a(trans, 9, "Matched vars cleaned.");
// cppcheck-suppress ctunullpointer
trans->m_variableMatchedVar.unset();
trans->m_variableMatchedVars.unset();
trans->m_variableMatchedVarName.unset();
trans->m_variableMatchedVarsNames.unset();
}

int RulesSet::evaluate(int phase, Transaction *t) {
if (phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
Expand Down Expand Up @@ -208,6 +216,7 @@ int RulesSet::evaluate(int phase, Transaction *t) {
}

rule->evaluate(t);
cleanMatchedVars(t);
if (t->m_it.disruptive > 0) {

ms_dbg_a(t, 8, "Skipping this phase as this " \
Expand Down
128 changes: 125 additions & 3 deletions test/test-cases/regression/variable-MATCHED_VAR.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MATCHED_VAR (1/2)",
"title":"Testing Variables :: MATCHED_VAR (1/5)",
"client":{
"ip":"200.249.12.31",
"port":123
Expand Down Expand Up @@ -42,7 +42,7 @@
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MATCHED_VAR (2/2)",
"title":"Testing Variables :: MATCHED_VAR (2/5)",
"client":{
"ip":"200.249.12.31",
"port":123
Expand Down Expand Up @@ -81,6 +81,128 @@
"SecRule MATCHED_VAR \"@contains other_value\" \"id:29,pass\"",
"SecRule MATCHED_VAR \"@contains other_value\" \"id:30,pass\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MATCHED_VAR (3/5)",
"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":"/?foo=1&bar=2&baz=2",
"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",
"SecRule ARGS \"@rx 1\" \"id:1,phase:1,pass\"",
"SecRule ARGS \"@rx 2\" \"id:2,phase:1,pass\"",
"SecRule MATCHED_VAR \"@eq 1\" \"id:3,phase:1,deny,status:403\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MATCHED_VAR (4/5)",
"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":"/?foo=1&bar=2&baz=2",
"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",
"SecRule ARGS \"@rx 1\" \"id:1,phase:1,pass\"",
"SecRule ARGS \"@rx 2\" \"id:2,phase:1,pass\"",
"SecRule MATCHED_VAR \"@eq 2\" \"id:3,phase:1,deny,status:403\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MATCHED_VAR (5/5)",
"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":"/?foo=1&bar=2&baz=2",
"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": 403
},
"rules":[
"SecRuleEngine On",
"SecRule ARGS \"@rx 1\" \"id:1,phase:1,pass\"",
"SecRule ARGS \"@rx 2\" \"id:2,phase:1,deny,status:403,chain\"",
"SecRule MATCHED_VAR \"@eq 2\""
]
}
]

Loading
Loading