Skip to content

Commit 08b70e0

Browse files
airweenairween
authored
Merge pull request #3422 from airween/v3/matchedvarsfix2
Refactoring the cleaning of MATCHED_VAR* variables
2 parents cf24aea + 79d5503 commit 08b70e0

File tree

8 files changed

+594
-26
lines changed

8 files changed

+594
-26
lines changed

‎headers/modsecurity/rule_with_operator.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ class RuleWithOperator : public RuleWithActions {
5959

6060
static void updateMatchedVars(Transaction *trasn, const std::string &key,
6161
const std::string &value);
62-
static void cleanMatchedVars(Transaction *trasn);
6362

6463

6564
const std::string& getOperatorName() const;

‎headers/modsecurity/rules_set.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ class RulesSet : public RulesSetProperties {
8080
void debug(int level, const std::string &id, const std::string &uri,
8181
const std::string &msg);
8282

83+
static void cleanMatchedVars(Transaction *trans);
84+
8385
RulesSetPhases m_rulesSetPhases;
8486
private:
8587
#ifndef NO_LOGS

‎src/rule_with_operator.cc‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,6 @@ void RuleWithOperator::updateMatchedVars(Transaction *trans, const std::string &
9090
}
9191

9292

93-
void RuleWithOperator::cleanMatchedVars(Transaction *trans) {
94-
ms_dbg_a(trans, 9, "Matched vars cleaned.");
95-
// cppcheck-suppress ctunullpointer
96-
trans->m_variableMatchedVar.unset();
97-
trans->m_variableMatchedVars.unset();
98-
trans->m_variableMatchedVarName.unset();
99-
trans->m_variableMatchedVarsNames.unset();
100-
}
101-
102-
103-
10493
bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string &key,
10594
const std::string &value, RuleMessage &ruleMessage) {
10695
#if MSC_EXEC_CLOCK_ENABLED
@@ -324,7 +313,6 @@ bool RuleWithOperator::evaluate(Transaction *trans,
324313

325314
if (globalRet == false) {
326315
ms_dbg_a(trans, 4, "Rule returned 0.");
327-
cleanMatchedVars(trans);
328316
goto end_clean;
329317
}
330318
ms_dbg_a(trans, 4, "Rule returned 1.");

‎src/rules_set.cc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,14 @@ std::string RulesSet::getParserError() {
105105
return this->m_parserError.str();
106106
}
107107

108+
void RulesSet::cleanMatchedVars(Transaction *trans) {
109+
ms_dbg_a(trans, 9, "Matched vars cleaned.");
110+
// cppcheck-suppress ctunullpointer
111+
trans->m_variableMatchedVar.unset();
112+
trans->m_variableMatchedVars.unset();
113+
trans->m_variableMatchedVarName.unset();
114+
trans->m_variableMatchedVarsNames.unset();
115+
}
108116

109117
int RulesSet::evaluate(int phase, Transaction *t) {
110118
if (phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
@@ -208,6 +216,7 @@ int RulesSet::evaluate(int phase, Transaction *t) {
208216
}
209217

210218
rule->evaluate(t);
219+
cleanMatchedVars(t);
211220
if (t->m_it.disruptive > 0) {
212221

213222
ms_dbg_a(t, 8, "Skipping this phase as this " \

‎test/test-cases/regression/variable-MATCHED_VAR.json‎

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"enabled":1,
44
"version_min":300000,
5-
"title":"Testing Variables :: MATCHED_VAR (1/2)",
5+
"title":"Testing Variables :: MATCHED_VAR (1/5)",
66
"client":{
77
"ip":"200.249.12.31",
88
"port":123
@@ -42,7 +42,7 @@
4242
{
4343
"enabled":1,
4444
"version_min":300000,
45-
"title":"Testing Variables :: MATCHED_VAR (2/2)",
45+
"title":"Testing Variables :: MATCHED_VAR (2/5)",
4646
"client":{
4747
"ip":"200.249.12.31",
4848
"port":123
@@ -81,6 +81,128 @@
8181
"SecRule MATCHED_VAR \"@contains other_value\" \"id:29,pass\"",
8282
"SecRule MATCHED_VAR \"@contains other_value\" \"id:30,pass\""
8383
]
84+
},
85+
{
86+
"enabled":1,
87+
"version_min":300000,
88+
"title":"Testing Variables :: MATCHED_VAR (3/5)",
89+
"client":{
90+
"ip":"200.249.12.31",
91+
"port":123
92+
},
93+
"server":{
94+
"ip":"200.249.12.31",
95+
"port":80
96+
},
97+
"request":{
98+
"headers":{
99+
"Host":"localhost",
100+
"User-Agent":"curl/7.38.0",
101+
"Accept":"*/*"
102+
},
103+
"uri":"/?foo=1&bar=2&baz=2",
104+
"method":"GET"
105+
},
106+
"response":{
107+
"headers":{
108+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
109+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
110+
"Content-Type":"text/html"
111+
},
112+
"body":[
113+
"no need."
114+
]
115+
},
116+
"expected":{
117+
"http_code": 200
118+
},
119+
"rules":[
120+
"SecRuleEngine On",
121+
"SecRule ARGS \"@rx 1\" \"id:1,phase:1,pass\"",
122+
"SecRule ARGS \"@rx 2\" \"id:2,phase:1,pass\"",
123+
"SecRule MATCHED_VAR \"@eq 1\" \"id:3,phase:1,deny,status:403\""
124+
]
125+
},
126+
{
127+
"enabled":1,
128+
"version_min":300000,
129+
"title":"Testing Variables :: MATCHED_VAR (4/5)",
130+
"client":{
131+
"ip":"200.249.12.31",
132+
"port":123
133+
},
134+
"server":{
135+
"ip":"200.249.12.31",
136+
"port":80
137+
},
138+
"request":{
139+
"headers":{
140+
"Host":"localhost",
141+
"User-Agent":"curl/7.38.0",
142+
"Accept":"*/*"
143+
},
144+
"uri":"/?foo=1&bar=2&baz=2",
145+
"method":"GET"
146+
},
147+
"response":{
148+
"headers":{
149+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
150+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
151+
"Content-Type":"text/html"
152+
},
153+
"body":[
154+
"no need."
155+
]
156+
},
157+
"expected":{
158+
"http_code": 200
159+
},
160+
"rules":[
161+
"SecRuleEngine On",
162+
"SecRule ARGS \"@rx 1\" \"id:1,phase:1,pass\"",
163+
"SecRule ARGS \"@rx 2\" \"id:2,phase:1,pass\"",
164+
"SecRule MATCHED_VAR \"@eq 2\" \"id:3,phase:1,deny,status:403\""
165+
]
166+
},
167+
{
168+
"enabled":1,
169+
"version_min":300000,
170+
"title":"Testing Variables :: MATCHED_VAR (5/5)",
171+
"client":{
172+
"ip":"200.249.12.31",
173+
"port":123
174+
},
175+
"server":{
176+
"ip":"200.249.12.31",
177+
"port":80
178+
},
179+
"request":{
180+
"headers":{
181+
"Host":"localhost",
182+
"User-Agent":"curl/7.38.0",
183+
"Accept":"*/*"
184+
},
185+
"uri":"/?foo=1&bar=2&baz=2",
186+
"method":"GET"
187+
},
188+
"response":{
189+
"headers":{
190+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
191+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
192+
"Content-Type":"text/html"
193+
},
194+
"body":[
195+
"no need."
196+
]
197+
},
198+
"expected":{
199+
"http_code": 403
200+
},
201+
"rules":[
202+
"SecRuleEngine On",
203+
"SecRule ARGS \"@rx 1\" \"id:1,phase:1,pass\"",
204+
"SecRule ARGS \"@rx 2\" \"id:2,phase:1,deny,status:403,chain\"",
205+
"SecRule MATCHED_VAR \"@eq 2\""
206+
]
84207
}
85208
]
86-

0 commit comments

Comments
 (0)