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 30 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.

@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?

@theseion
Copy link
Contributor

No, but we need to finish it.

@Xhoenix
Copy link
Member Author
Xhoenix commented May 24, 2025

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

@theseion You forgot this comment.

@theseion
Copy link
Contributor

No, as I wrote above, I think we need an additional rule because 932200 doesn't check the Referer header:

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 Jun 27, 2025

You probably misread: 932205.

@theseion
Copy link
Contributor
  • 932200 doesn't look at the Referer header.
  • 932205 looks at the Referer header, but at the query string only.
  • 932206 looks at the Referer header, but only at values that don't look like URLs
  • 932207 will look at the Referer header, but only at the fragment

So we'll have everything covered except for the URI itself in the Referer header. You were right @Xhoenix.

@Xhoenix Xhoenix requested a review from theseion July 3, 2025 05:02
version: HTTP/1.0
output:
log:
match_regex: "Matched Data: .*? found within REQUEST_HEADERS:Referer: .*"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use wildcards here but the full string to match. With the wildcards, we won't see the value of TX.0 and won't know whether it is what we expect it to be.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the regex is matching REQUEST_HEADERS:Referer, as per logdata, which is what you asked to implement.

Xhoenix and others added 2 commits July 9, 2025 11:20
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
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