-
-
Notifications
You must be signed in to change notification settings - Fork 402
fix: Changed regex (920470) to match multiple whitespaces after Content-Type
parameters to avoid false-positives
#3818
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
Conversation
…id false-positives
Thanks for the PR and the thorough description. Please add a test case for your change to https://github.com/coreruleset/coreruleset/blob/main/tests/regression/tests/REQUEST-920-PROTOCOL-ENFORCEMENT/920470.yaml. |
@lostmann-owl-it CRS Dev on Duty reporting. As requested above, please can you add a test case(s) to cover your proposed change? We generally require a working test for a new PR. If you need any help with this, please shout! Thanks. |
To make sure if I understand those checks correctly: A test with the following output will be blocked by the WAF: output:
log:
expect_ids: [920470] A test with the following output will not be blocked by the WAF: output:
log:
no_expect_ids: [920470] Am I correct with this assumption? |
@lostmann-owl-it Yes you've got the idea the output is checking if rule 920470 is being triggered or not based on the payload your sending for the test. |
@lostmann-owl-it I wish I'd remembered to include this when I posted here on Friday, but we also have our gold standard test templates in our documentation. They can be copy and pasted to do 95% of the work! 🙂 |
@lostmann-owl-it Could you also add yourself to the contributors.md file? It's sorted in alphabetical order by last name |
I added myself. :) |
Description
This pull request addresses a false positive related to the
Content-Type
header being incorrectly flagged by rule ID 920470. The issue arises due to overly restrictive whitespace handling in the regex validation. The original regex allowed only an optional whitespace between parameters, which blocked valid headers containing additional whitespaces, even though these are permitted according to RFC standards.Problem
The original regex for rule ID 920470 did not account for multiple or flexible whitespaces between header parameters. This caused valid
Content-Type
headers, compliant with RFC 2045 and RFC 7231, to be blocked unnecessarily.Original regex
Updated regex
Changes
\s?;\s?
has been updated to\s?;\s*
to allow for flexible whitespace usage, in accordance with RFC 2045 and RFC 7231, which permit multiple whitespaces between parameters.Example
The blocked request contained the following Content-Type header:
This valid header was blocked because of extra spaces around the parameters, which are permitted under RFC specifications.
Impact
This update will prevent unnecessary blocking of valid HTTP requests while still maintaining the security provided by the rule.
Attachments
Original regex
Updated regex