akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

Wrong part of HTTP_X_FORWARDED_FOR is determined as the IP-address #627

Closed Radek-Suski closed 8 years ago

Radek-Suski commented 8 years ago

I am not sure if it is valid for every case but in case of Varnish this header looks like this: 91.0.119.54, ::1

Therefore the function "detectAndCleanIP" keeps returning ::1 instead of the IP

Regards, Radek

wilsonge commented 8 years ago

I think you've got a bug in your varnish set up. You should have an array of comma separated IP's if you've got it set up something like in here: http://easyos.net/articles/bsd/freebsd/getting_real_ip_through_varnish_for_apache_and_php

Radek-Suski commented 8 years ago

::1 is a valid IP AFAIR but even if there will be usual IPv4 the result will still be wrong as the X-Forwarded-For looks by definition like:

X-Forwarded-For: client, proxy1, proxy2

So it would return always the proxy IP and not the client's one

nikosdion commented 8 years ago

The thing is, if I keep the FIRST IP address I am actually allowing a very simple IP spoofing attack to take place: send a request with an X-Forwarded-For header with the spoofed IP to the first proxy and watch the world burn. Therefore I only trust the last proxy IP address (which is the one added by your own equipment).

The problem I do see, however, is that your setup not only adds the client IP address but also itself. This sounds wrong. Do you have two opaque, localhost proxies?! Why? Can you reconfigure Varnish to not include the localhost address in the header?

Radek-Suski commented 8 years ago

It passes 2 proxies Apache(443) to Varnish to Apache (8080). But as I understand the wikipedia entry this is the correct way how the "X-Forwarded-For" should looks like:

X-Forwarded-For: client, proxy1, proxy2

nikosdion commented 8 years ago

You CANNOT trust anything older than the last IP address since it can be injected by anything on the network, including the user himself. Between fucking up security for all (especially everyone behind a CDN or proxy) and making life hard for the very uncommon case of multiple proxies in series I choose the latter. Sorry.

Radek-Suski commented 8 years ago

But this is exactly how the header is defined. The first ip is the one from client. It has nothing to do with the configuration.

Well, never mind. I will have to keep hacking FoF then

wilsonge commented 8 years ago

The first IP doesn't have to be from the client. It's easily possible to create a large chain of headers or start with a large chain of ip's. If you have say 4/5 ip's in this chain why does the first have to be from the client?

Radek-Suski commented 8 years ago

If you have say 4/5 ip's in this chain why does the first have to be from the client?

Because that's what the specification says.

https://en.wikipedia.org/wiki/X-Forwarded-For#Format:

The general format of the field is:

X-Forwarded-For: client, proxy1, proxy2

where the value is a comma+space separated list of IP addresses, the left-most being the original client, and each successive proxy that passed the request adding the IP address where it received the request from.

@nikosdion

You CANNOT trust anything older than the last IP address since it can be injected by anything on the network

Which means the way you are doing it, you actually never receive the client IP but always the IP of the last proxy the request passed.

It is clear for me that this header can be falsified (as every other user input) but it is also fairly easy to filter the data with a very simple regex

nikosdion commented 8 years ago

YOU CANNOT CLEAN OR TRUST THIS HEADER AS I HAVE BEEN SAYING ALL ALONG.

YOU CAN ONLY, REPEAT: ONLY, TRUST THE VERY LAST PART OF THIS HEADER WHICH IS THE ONLY, REPEAT: ONLY, PART OF THIS HEADER WHICH IS GUARANTEED TO HAVE BEEN ADDED BY YOUR OWN EQUIPMENT, THEREFORE GUARANTEED TO HAVE NOT BEEN FALSIFIED.

Example attack vector:

Your only mitigation against these attacks is to drop the X-Forwarded-For headers when the request enters your network. Then and ONLY then will you ever be able to trust that header's content before the last record.