Refactoring the cleaning of MATCHED_VAR* variables

[airween]

what

This PR changes the code behavior: now the engine cleans the MATCHED_VAR* variables after chained and non-chained rules too.

With current behavior if a rule is not chained then MATCHED_VAR* variables kept their values.

Changes:

• moved cleanMatchedVars() method from RuleWithOperator class to RuleSet
  • the reason is: RuleWithOperator represents a SecRule object, which can be a chained or non-chained rule; it's important that SecRule (in this context) has a variable and an operator at least (SecAction does not have these)
  • RuleSet is a container which holds SecRule and SecAction objects
  • when the evaluate() method is called from the transaction's phase, then RuleSet evaluates all rules and actions in a loop
  • because the cleanMatchedVars() was a part of RuleWithOperator (which is a SecRule), then it was possible to call that only from that class
  • it was difficult to decide when a rule ended (see discussion)
  • otherwise, I think this method is better suited here, because we must clean these variables after each rule, whether it's chained or non-chained - but in case of chained rule, only after the last part of it
• the location of this method call has also been moved after the point where RulesSet object evaluates the rules of phases, now we can call it after all evaluate() call - and the condition fits that it cleans the MATCHED_VAR* variables after all single rule and after all last part of chained rule
• I also added the new test cases from previous PR

why

The issue #3382 describes the problem: if a non-chained rule evaluated, and the rule filled the MATCHED_VAR* variables, then those values remained after the ending of evaluation. Then if a next rule used any of MATCHED_VAR*, it found those values and led a false match.

other notes

There was a try before this, see #3418. There - as I wrote above - wasn't clean how the solution solves the issue exactly and was a suggestion we should clean these variables before the rule's evaluation. It was not possible, because the cleanMatchedVars() was a part of the one rule's method.

references

Closes #3382.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[airween]

@MirkoDziadzka, @theseion what do you think about this PR?

[theseion]

Much better!