Skip to content
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

Remove X-Forwarded-For header special processing #103

Merged
merged 4 commits into from
Oct 9, 2023

Conversation

lubomudr
Copy link
Contributor

@lubomudr lubomudr commented Oct 7, 2023

Hi

The special handling of X-Forwarded-For in runtime.c is a security hole and VERY DANGEROUS.

If the ngx_http_realip_module module configuration is enabled, the NGINX $remote_addr variable is replaced with X-Forwarded-For if (and only if) the IP packet came from any trusted host in set_real_ip_from.
If the IP packet arrived from any other hosts or the ngx_http_realip_module module is not enabled, processing of the X-Forwarded-For header is ignored.

Handling of the X-Forwarded-For header must be completely transparent to NAXSI

@lubomudr lubomudr changed the title Remove X-Forwarder-For header special processing Remove X-Forwarded-For header special processing Oct 7, 2023
@wargio
Copy link
Owner

wargio commented Oct 8, 2023

i think the issue should be solved by checking the actual trusted list of IPs.
if you have 3 reverse proxy then you should be able to know which IPs it should be in the headers and block bad requests.
The original feature has always been problematic, and i do agree on the issue you are raising.

@lubomudr
Copy link
Contributor Author

lubomudr commented Oct 8, 2023

This problem does not exist in NGINX
It handles everything correctly

But NAXSI in its current implementation additionally tries to check the X-Forwarded-For header itself and, regardless of the settings of trusted hosts, passes requests if the header contains a value from IgnoreCIDR / IgnoreIP.

For example, in our organization, internal addresses are not checked by NAXSI. Specified IgnoreCIDR 192.168.0.0/16, 172.16.0.0/16
But now any packet from any host with X-Forfarded-For: 192.168.1.1 will be passed by NAXSI

@wargio
Copy link
Owner

wargio commented Oct 9, 2023

Probably i'm still stuck with issues that afflicted very old versions of nginx.

@wargio wargio merged commit 1b71252 into wargio:main Oct 9, 2023
@wargio
Copy link
Owner

wargio commented Oct 11, 2023

I have made a special release due this security bug. i have requested a CVE at your name :) @lubomudr

GHSA-7qjc-q4j9-pc8x

@lubomudr
Copy link
Contributor Author

Thank you
I just haven't figured out what it is yet ;-)

@wargio
Copy link
Owner

wargio commented Oct 11, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants