Skip to content

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

Thanks @Xhoenix!

@theseion theseion added this pull request to the merge queue Jul 12, 2025
Merged via the queue into coreruleset:main with commit f17e65e Jul 12, 2025
7 checks passed
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