Skip to content

feat: Remove rules for lack of viable attack scenario (920220 PL1, 920221 PL1) #3969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

dune73
Copy link
Member

@dune73 dune73 commented Dec 30, 2024

Rules 920220 and 920221 caused a lot of FPs in CRS4. After discussing the rules with @franbuehler and @theseion and potential fixes in issue 3926, we decided to remove the rules. Mainly because we can not think of a generic attack scenario that uses / abuses encoding this way.

Fixes #3926

Copy link
Contributor

github-actions bot commented Dec 30, 2024

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

@dune73 dune73 added the release:new-feature This PR introduces a new feature label Dec 30, 2024
…0221 PL1)

Rules 920220 and 920221 caused a lot of FPs in CRS4. After discussing
the rules and potential fixes in issue 3926, we decided to remove the
rules. Mainly because we can not think of a generic attack scenario
that uses / abuses encoding this way.
@dune73 dune73 added release:remove-rules Remove one or more rules and removed release:new-feature This PR introduces a new feature labels Dec 30, 2024
@theseion theseion requested a review from fzipi December 30, 2024 11:29
@fzipi
Copy link
Member

fzipi commented Dec 30, 2024

Where are the rules coming from, initially? What were they supposed to protect us from? Is this coming from the BB?

@theseion
Copy link
Contributor

This is the conclusion from #3926. The two rules were initially one rule from the original Trustwave rule set. We don't know what the intent of the original rule was, which is one of the reasons we've decided to remove them.

@fzipi fzipi added this pull request to the merge queue Dec 30, 2024
Merged via the queue into coreruleset:main with commit f758804 Dec 30, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:remove-rules Remove one or more rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

920220 PL1 / 920221 PL1 have a lot of false positives with unencoded percent signs in URIs (-> query strings)
4 participants