8000 feat: added detection for RCE via Referer header by Xhoenix · Pull Request #3993 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: added detection for RCE via Referer header #3993

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

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

Xhoenix
Copy link
Member
@Xhoenix Xhoenix commented Feb 9, 2025

Fixes #3498

Copy link
Contributor
github-actions bot commented Feb 9, 2025

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

@Xhoenix Xhoenix requested a review from theseion February 13, 2025 12:46
Xhoenix and others added 6 commits February 15, 2025 12:30
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Xhoenix and others added 4 commits February 16, 2025 18:31
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix
Copy link
Member Author
Xhoenix commented Mar 28, 2025

@theseion LGTM :)

@fzipi fzipi requested a review from theseion March 29, 2025 12:17
@theseion
Copy link
Contributor

Will check, need a little time though.

@azurit
Copy link
Member
azurit commented Mar 31, 2025

@Xhoenix You are doing double URL decode using t:urlDecodeUni action: First one in the first rule and second one in the last rule. All rules are using the same data which comes from first rule - and those data are already URL decoded (in first rule).

@azurit
Copy link
Member
azurit commented Mar 31, 2025

Also, i don't think we need such a complex rule for this. According to RFC 9110, Referer header cannot include a fragment (#). We should create a simple rule which blocks all requests where a fragment is part of Referer header or include Referer within variables of rule 920610.

@Xhoenix
Copy link
Member Author
Xhoenix commented Mar 31, 2025

I checked the RFC 9110 and Referer header must not contain fragments. I think we should just update rule 920610 to include the referer header, which will make things simpler.

@theseion
Copy link
Contributor
theseion commented Apr 1, 2025

The RFC does not reflect our reality. You can easily send a Referer header with a fragment and then it's up to the server how it handles that URL. We need to anticipate attacks that abuse this.

@azurit
Copy link
Member
azurit commented Apr 1, 2025

The RFC does not reflect our reality. You can easily send a Referer header with a fragment and then it's up to the server how it handles that URL. We need to anticipate attacks that abuse this.

Do you know about any client which is doing it?

@theseion
Copy link
Contributor
theseion commented Apr 2, 2025

No. But it's easy to do manually. I'm not worried about benign clients but about deliberate attack requests.

@azurit
Copy link
Member
azurit commented Apr 2, 2025

No. But it's easy to do manually. I'm not worried about benign clients but about deliberate attack requests.

I don't get you. This applies to every single attack - all of them are easy to send manualy but that's not a reason to NOT block it.

@theseion
Copy link
Contributor
theseion commented Apr 3, 2025

Also, i don't think we need such a complex rule for this. According to RFC 9110, Referer header cannot include a fragment (#). We should create a simple rule which blocks all requests where a fragment is part of Referer header or include Referer within variables of rule 920610.

If I had read your comment carefully, @azurit, instead of just skimming it, I would have realised that you are suggesting the same thing. I'm sorry. I was under the impression that you were arguing for ignoring fragments in the Referer header. Please ignore what I've said above.

I checked the RFC 9110 and Referer header must not contain fragments. I think we should just update rule 920610 to include the referer header, which will make things simpler.

That's not a bad idea @Xhoenix.

@theseion
Copy link
Contributor
theseion commented Apr 4, 2025

In #3485 we saw that fragments in the Referer header are common and we decided. We also saw that we needed to address the Referer header separately from other rules, because the semantics of the header can lead to many FPs with the detections used in other rules. Similarly, fragments can contain almost anything and need to be handled separately to reduce FPs to a minimum.

The new rule now only looks at the fragment, while 932206 only looks at values that do not look like URLs. What's still missing is a rule that looks at URLs (skipping the fragment) in the Referer header.

@Xhoenix
Copy link
Member Author
Xhoenix commented Apr 4, 2025

The new rule now only looks at the fragment, while 932206 only looks at values that do not look like URLs. What's still missing is a rule that looks at URLs (skipping the fragment) in the Referer header.

I think rule 932200 covers that. This is after all based off of 932200.

@theseion
Copy link
Contributor
theseion commented Apr 5, 2025

I think rule 932200 covers that. This is after all based off of 932200.

932200 doesn't check the Referer header.

@Xhoenix
Copy link
Member Author
Xhoenix commented Apr 7, 2025

I checked and rule 932205 does check the Referer header in the URL part.

@Xhoenix Xhoenix added the release:new-detection In this PR we introduce a new detection label Apr 9, 2025
@Xhoenix
Copy link
Member Author
Xhoenix commented May 23, 2025

Should we close this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:new-detection In this PR we introduce a new detection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detect RCE in fragments of URLs in Referer header (932205)
4 participants
0