8000 feat: refactoring of rule 941310 (PL1 941310) by azurit · Pull Request #3700 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: refactoring of rule 941310 (PL1 941310) #3700

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 3 commits into from
May 10, 2024

Conversation

azurit
Copy link
Member
@azurit azurit commented May 10, 2024

Rule 941310 is matching regex:
\xbc[^\xbe>]*[\xbe>]|<[^\xbe]*\xbe

To variables:
REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|ARGS_NAMES|ARGS|REQUEST_FILENAME|XML:/*

If a match is found, it continues into chained rule and matches regex:
(?:\xbc\s*/\s*[^\xbe>]*[\xbe>])|(?:<\s*/\s*[^\xbe]*\xbe)

Again, to the same list of variables:
REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|ARGS_NAMES|ARGS|REQUEST_FILENAME|XML:/*

This is:

  1. Not required.
  2. Ineffective.
  3. Possible creating more FPs.

The list of variables in the chained rule should be replaced with a MATCHED_VARS variable as we only need to match against variables matched by the main rule. Also, the current behavior may create more FPs as main rule may match variable1 and the chained rule may match variable2 (which wasn't matched by main rule).

As a bonus, we may remove all transformations from the chained rule.

Copy link
Contributor
@theseion theseion left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@theseion theseion added this pull request to the merge queue May 10, 2024
Merged via the queue into coreruleset:main with commit 041c9ed May 10, 2024
4 checks passed
@azurit azurit deleted the Fix_941310 branch May 17, 2024 15:50
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.

3 participants
0