ctl:removeTargetById doesn't know how to work with regex

[odesk2dot2by]

Main problem: ctl:removeTargetById doesn't know how to work with regex . For instance:

ctl:ruleRemoveTargetByID=981248;ARGS:widget-text[4][text] - OK ctl:ruleRemoveTargetByID=981248;ARGS:/^widget/ - BAD

[lifeforms]

I would loooooove this!

[Yyuzu]

As I do, kinda missing it right now !

[gmetaxo]

Hello everyone,

We have performed an investigation of the changes that would be required in order to support regular expressions in the target list of the ctl:ruleRemoveTargetById and ctl:ruleRemoveTargetByTag actions. Currently, this functionality is not implemented at all for those actions. I will be providing a summary of our findings.

Whenever the parser encounters an action, it creates an instance of the class associated to the respective action (e.g.: actions::ctl::RuleRemoveTargetById when encountering the ctl:ruleRemoveTargetById action) and ultimately calls the init function of the new object. In particular, when creating the instance, the value passed as an argument to the respective constructor is a string that contains the whole action string (e.g.: ctl:ruleRemoveTargetById=1;ARGS:arg1 for the ctl:ruleRemoveTargetById action). This string is then parsed in the constructor (of the parent actions::Action class) and the init function (e.g. populating the integer m_id and string m_target for the ctl:ruleRemoveTargetById action). It can be seen that the parser does not further parse the actions and their argument, but they are manually handled in the respective class implementations.

Therefore, in the case of the ctl:ruleRemoveTargetById action, the parser does not further parse the rule ID or the targets. For example, if the action is ctl:ruleRemoveTargetById=1;ARGS:arg1, the parser treats ctl:ruleRemoveTargetById=1;ARGS:arg1 as a single argument passed to the constructor of the actions::ctl::RuleRemoveTargetById class. The constructor of the parent actions::Action class will remove the ctl: prefix and store the remaining string in the object and then the init function of the actions::ctl::RuleRemoveTargetById class will split the remaining string and store the ID (integer) and the target (string) in the object.

In contrast, the SecRuleUpdateTargetById directive (which supports regular expressions in the targets), performs all the operations related to the identification of the type of each target and its associated arguments in the context of the parser. For example, when the parser parses the SecRuleUpdateTargetById directive and encounters the ARGS target variable with a regular expression after the selection operator, it creates an instance of the variables::Args_DictElementRegexp class (which is a subclass of VariableRegex). Thus, the identification of the target type and its arguments (parameter name or regular expression) happens at parse time.

Given the above, the proper way to support regular expressions in the target of the ctl:ruleRemoveTargetById and ctl:ruleRemoveTargetByTag actions would be to further analyze the targets and the corresponding arguments and generate the corresponding class instances at parse time (similar to the SecRuleUpdateTargetById directive).

However, this is not a trivial task due to the current implementation of the parsing of actions. In particular, regardless of the action encountered, the parser will eventually invoke the init function of the respective Action subclass, passing only a string argument (as described above). Therefore, it is not possible to just modify the init function of the actions::ctl::RuleRemoveTargetById and actions::ctl::RuleRemoveTargetByTag classes to accept more arguments (derived by further analyzing the targets at parse time), due to the uniform handling of all Actions by the current implementation of the parser. A proper solution would require modifications in the parser to further analyze the arguments of all Actions and the refactoring of the actions::Action subclasses to accept multiple arguments (instead of parsing the single string argument in their init function).

[gmetaxo]

We have also come up with an alternative that would not require changes to the current parser implementation, but may not be the preferred way of tackling the issue. This workaround consists of the following:

• Modifying the RuleRemoveTargetById::init (1) and RuleRemoveTargetByTag::init (2) functions to further parse param[1] and identify the type of the variable (e.g. ARGS) and the type of the defined parameter (no parameter, variable, regular expression), so that m_target is a Variable object instead of a string (and also modify the definition and instantiation of m_target in RuleRemoveTargetById (3) and RuleRemoveTargetByTag (4) accordingly). To achieve that, we would need to implement a function that identifies the types of the variable and the parameter (if any) and then instantiate the appropriate Variable subclass (e.g. Args_DictElementRegexp, Args_DictElement, Args_NoDictElement, etc.).
• Modifying the m_ruleRemoveTargetById (5) and m_ruleRemoveTargetByTag (6) properties of the Transaction class, so that they define pairs of (id, Variable) and (tag, Variable), respectively.
• Modifying all the string comparison operations that are currently present in the evaluation of actions of rules with operators (7, 8, 9, 10), to proper comparisons of the respective Variable objects (similar to how these comparisons are implemented in the return statements of the contains functions (11, 12) of the Variables class). This could be further refactored to utilize the Variables class in that case, too.

We are looking forward to your opinion regarding both the analysis and the possible workaround, in order to shape a common approach in tackling the issue.