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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

feat: added detection for RCE via Referer header #3993

wants to merge 30 commits into from
+202 −3

Conversation

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

Fixes #3498

@github-actions GitHub Actions
Copy link
Contributor
github-actions bot commented Feb 9, 2025 •
edited
Loading

📊 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
theseion
theseion requested changes Feb 15, 2025
View reviewed changes
Xhoenix and others added 6 commits February 15, 2025 12:30
@Xhoenix @theseion
Update regex-assembly/932207-chain1.ra
5a0eb68 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix @theseion
Update regex-assembly/932207-chain1.ra
d90eec7 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix @theseion
Update rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
eca01b3 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix
added rule chain and tests
407f4d6
@Xhoenix
fix syntax error
9d5027c
@Xhoenix @theseion
Update rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
95a593d 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
theseion
theseion requested changes Feb 16, 2025
View reviewed changes
Xhoenix and others added 4 commits February 16, 2025 18:31
@Xhoenix @theseion
Update rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
fa8b61a 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix @theseion
Update rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
290f964 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix @theseion
Update rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
c3b084e 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix
added tests
e77e9ad
@fzipi fzipi mentioned this pull request Mar 3, 2025
Closed
@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 •
edited
Loading

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 •
edited
Loading

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.

@fzipi fzipi mentioned this pull request Apr 7, 2025
Closed
@Xhoenix Xhoenix added the release:new-detection In this PR we introduce a new detection label Apr 9, 2025
@fzipi fzipi mentioned this pull request May 5, 2025
Closed
@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.

@fzipi fzipi mentioned this pull request Jun 2, 2025
Closed
@github-actions github-actions bot added the Stale label Jun 24, 2025
@Xhoenix Xhoenix removed the Stale label Jun 27, 2025
@Xhoenix
Copy link
Member Author
Xhoenix commented Jun 27, 2025

You probably misread: 932205.

@theseion theseion mentioned this pull request Jun 28, 2025
Open
@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.

theseion
theseion requested changes Jun 28, 2025
View reviewed changes
@Xhoenix Xhoenix requested a review from theseion July 3, 2025 05:02
@fzipi fzipi mentioned this pull request Jul 7, 2025
Open
theseion
theseion requested changes Jul 9, 2025
View reviewed changes
tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932207.yaml
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
@Xhoenix @theseion
Update rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
49f65ed 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@Xhoenix
remove duplicate comment
73FB
0448259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theseion theseion theseion requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
release:new-detection In this PR we introduce a new detection
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Detect RCE in fragments of URLs in Referer header (932205)
4 participants
@Xhoenix @theseion @azurit @fzipi
0