Skip to content

fix(920539): prefer a bypass on a named rule rather than n+1 bypass#4610

Merged
EsadCetiner merged 3 commits into
coreruleset:mainfrom
touchweb-vincent:patch-1
Apr 11, 2026
Merged

fix(920539): prefer a bypass on a named rule rather than n+1 bypass#4610
EsadCetiner merged 3 commits into
coreruleset:mainfrom
touchweb-vincent:patch-1

Conversation

@touchweb-vincent
Copy link
Copy Markdown
Contributor

Proposed changes

Hello,

I find this idea #4405 (comment) risky, and I’m sure it will cause us issues someday.

I therefore suggest naming the bypass as we usually do on a named rule using ctl:ruleRemoveById, rather than using skip:1.

What do you think ?

PR Checklist

  • I have read the CONTRIBUTING doc
  • I have added positive tests proving my fix/feature works as intended.
  • I have added negative tests that prove my fix/feature considers common cases that might end in false positives
  • In case you changed a regular expression, you are not adding a ReDOS for pcre. You can check this using regexploit
  • My test use the comment field to write the expected behavior
  • I have added documentation for the rule or change (when appropriate)

Further comments

For the reviewer

  • Positive and negative tests were added
  • Tests cover the intended fix/feature properly
  • No usage of dangerous constructs like ctl:requestBodyAccess=Off were used in the rule
  • In case a regular expression was changed, there is no ReDOS
  • Documentation is clear for the rule/change

@touchweb-vincent touchweb-vincent changed the title fix(920539): can we prefer a bypass on a named rule rather than relyi… fix(920539): prefer a bypass on a named rule rather than n+1 bypass Apr 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 10, 2026

📊 Quantitative test results for language: eng, year: 2023, size: 10K, paranoia level: 1:
🚀 Quantitative testing did not detect new false positives

@HackingRepo
Copy link
Copy Markdown
Contributor

@touchweb-vincent checks have failed, see "action "ctl" at pos 7 is in the wrong order: "ctl" at pos 8; rule id: 920539" can you fix it?

@fzipi
Copy link
Copy Markdown
Member

fzipi commented Apr 10, 2026

@EsadCetiner I think there might be a point here. Using skip:1 implies file ordering, which makes things difficult when we start generating rulesets. Just removing the rule makes stuff more resilient.

Copy link
Copy Markdown
Member

@EsadCetiner EsadCetiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

On a sidenote, there are tests to check for when the rule should and shouldn't be skipped but this is a cleaner approach.

@EsadCetiner EsadCetiner added this pull request to the merge queue Apr 11, 2026
Merged via the queue into coreruleset:main with commit f99c91e Apr 11, 2026
8 checks passed
@touchweb-vincent touchweb-vincent deleted the patch-1 branch April 15, 2026 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants