Fix lexer rejection of '@' in ctl:ruleRemoveTarget actions (#3565)

[Jitterx69]

what

• Expands the REMOVE_RULE_BY regular expression macro in the Flex scanner (src/parser/seclang-scanner.ll) to include characters essential for XPath selectors.
• Specifically adds @, =, (, ), and ' to the allowed character class.

why

• Previously, the lexer rejected rule targets containing the @ character (such as XML://@* or ARGS:@foo) because the macro lacked these characters.
• This premature termination of the match caused the scanner to fall back and generate the syntax error Expecting an action, got: @*.
• By explicitly including these characters, the lexer can now accurately tokenize XPath attribute axes and predicates without inadvertently consuming action list delimiters (such as commas, spaces, or double-quotes).
• This restores parity with ModSecurity v2 functionality, enabling granular rule exclusions on specific XML attributes.

references

• closes ctl:ruleRemoveTarget* rejects any target containing @ (e.g. XPath attribute selectors) #3565

[airween]

@Jitterx69,

thanks - please add test cases to prove the correct behavior of this patch.

See regression tests this or this one.

[airween]

Thanks - sorry, I forgot to mention that you should regenerate Bison related files. For this, please install the latest Bison and Flex versions, run ./configure with your options, plus add --enable-parser-generation. Then you will see that there will be new files under src/parser with .cc and with .hh extensions - you must add them too to the commit.

Without this, the new tests will be failed.

You can find a similar solution here.

[Jitterx69]

here

Yeah...I just realized that before you qoute...I will imply as you say.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
10 Accepted issues

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

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed

Issues
17 New issues
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

[Copilot]

Pull request overview

This PR fixes SecLang lexer tokenization for ctl:ruleRemove* action values so that XPath-like selectors (and other variable selectors) containing @, =, (, ), and ' are not prematurely rejected, restoring ModSecurity v2 parity for granular target removals.

Changes:

• Expand the Flex macro used to lex ctl:ruleRemoveBy* / ctl:ruleRemoveTargetBy* payloads to allow additional selector characters (notably @).
• Add regression coverage for ctl:ruleRemoveTargetByTag and ctl:ruleRemoveTargetById when selectors include @, =, (, ), and '.
• Harden ctl action parsing by extracting payloads via = detection instead of fixed substring offsets, improving error handling for malformed inputs.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/parser/seclang-scanner.ll	Expands the lexer character class for ctl rule removal payloads to accept XPath/selector characters.
src/actions/ctl/rule_remove_target_by_tag.cc	Parses ctl payload using = delimiter and adds validation instead of fixed offsets.
src/actions/ctl/rule_remove_target_by_id.cc	Same =-based payload parsing/validation for ID-based target removal.
src/actions/ctl/rule_remove_by_tag.cc	Switches to =-based parsing for tag-based rule removal payloads.
src/actions/ctl/rule_remove_by_id.cc	Switches to =-based parsing for ID/range-based rule removal payloads.
test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json	Adds regression cases for special characters in ctl:ruleRemoveTargetByTag targets.
test/test-cases/regression/action-ctl_rule_remove_target_by_id.json	Adds regression cases for special characters in ctl:ruleRemoveTargetById targets.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[airween]

@Jitterx69,

thanks for this PR again.

I saw you added syntax handling, which is definitely a very good step:

• ruleRemoveById
• ruleRemoveByTag
• ruleRemoveTargetById
• ruleRemoveTargetByTag

Would you mind to add more tests to check these conditions too?

Please note that our regression test framework handles the parser error too, eg. you can add a wrong syntax in a test and expect a parser error. For eg. see this block: here the SecAction does not have a disruptive action.

In the planned "negative" tests you could construct invalid exclusions that you handle in those conditions.

Unfortunately my update merge failed you PR, could you rebase it?

But may be you might wait to merge of this PR.

Let us know if you need any help.

[airween]

Also please take a look at the Copilot suggestions.