Prestashop: X-Forwarded-For not supported properly

[ryandanthony]

Describe the bug
Given an installation of PrestaShop behind a reverse proxy / load balancer, I am seeing the following issues:

• When a user comes to the front office, the client IP is incorrectly evaluated as the IP address of the proxy/load balancer, and it ignores the X-Forwarded-For headers. This makes the "Visitors online" screen incorrect. (and likely other things)

• Maintenance Mode also ignores X-Forwarded-For headers. You can enable the Maintenance Mode, but you will not be able to add a IP address that will be able to see the front office. I am assuming it evaluates all client addresses as the load balancer / proxy IP address. So if you put in the load balancer IP address, you will be able to see the front office, but the public will be able to as well.

• When you click on the "Add My IP" button on the maintenance page, it ignores the X-Forwarded-For headers and uses the IP address of the proxy / load balancer IP address instead.

To Reproduce
see description.

Screenshots
n/a

Additionnal information
PrestaShop version: 1.7.1.5
PHP version: 7.2

• Changing the Tools.php's getRemoteAddr has no impact on any of the above issues.
• We are using the prestashop/prestashop:1.7-7.2-apache
• We put it behind a Nginx reverse proxy, like all the rest of our software, so that we are able to utilize the jwilder/docker-gen & jrcs/letsencrypt-nginx-proxy-companion docker images to enable SSL.

[khouloudbelguith]

Hi @ryandanthony,

Thank you for your report.
We'll first try to reproduce it and we'll come back to you if we need more information.

[ryandanthony]

I finally had a few minutes to put together a git repo that replicates the environment using docker.
https://github.com/ryandanthony/prestashop-proxy-issue

[khouloudbelguith]

Hi @ryandanthony,

Your link is not accessible, it provides me an error 404.
Thanks for checking it

[ryandanthony]

@khouloudbelguith
Sorry about that, I forgot to make it public.

[ryandanthony]

Hi @khouloudbelguith were you able to get access to that link?

[khouloudbelguith]

Hi @ryandanthony,

Yes, we'll first try to reproduce it and we'll come back to you if we need more information.

[khouloudbelguith]

Hi @ryandanthony,

I manage to reproduce the issue.
I noticed that the proxy only appears after the installation and restart of the container.
I also noticed that "IP Maintenance" takes the @ of the load balancer, but it does not work with the @
of the load balancer.

[marionf]

@PrestaShop/prestashop-core-developers wdyt ?

[snovak7]

Just a warning

https://github.com/PrestaShop/PrestaShop/blob/develop/classes/Tools.php#L390
RFC 1918 B is 172.16.0.0/12 and prestaShop matches only 172.16/16 which is incorrect 😄

Excerpt from https://tools.ietf.org/html/rfc1918
10.0.0.0 - 10.255.255.255 (10/8 prefix)
172.16.0.0 - 172.31.255.255 (172.16/12 prefix)
192.168.0.0 - 192.168.255.255 (192.168/16 prefix)

BUT cloudflare for instance sends public IP, so therefore there should by "whitelist" of public IPs, if you want to make it more secure, not locked to RFC 1918 subnets.

[matks]

Just a warning

https://github.com/PrestaShop/PrestaShop/blob/develop/classes/Tools.php#L390
RFC 1918 B is 172.16.0.0/12 and prestaShop matches only 172.16/16 which is incorrect 😄

Excerpt from https://tools.ietf.org/html/rfc1918
10.0.0.0 - 10.255.255.255 (10/8 prefix)
172.16.0.0 - 172.31.255.255 (172.16/12 prefix)
192.168.0.0 - 192.168.255.255 (192.168/16 prefix)

BUT cloudflare for instance sends public IP, so therefore there should by "whitelist" of public IPs, if you want to make it more secure, not locked to RFC 1918 subnets.

Thanks for the tip !

[davidglezz]

The Internet Assigned Numbers Authority (IANA) has reserved the
following three blocks of the IP address space for private internets:

 10.0.0.0        -   10.255.255.255  (10/8 prefix)
 172.16.0.0      -   172.31.255.255  (172.16/12 prefix)
 192.168.0.0     -   192.168.255.255 (192.168/16 prefix)

My reverse proxy has the IP 172.19.0.2, and fails because preg_match('/^172\.16.*/i', trim($_SERVER['REMOTE_ADDR'])) does not check the specified range. (172.16/12 prefix) vs (current 172.16/16 prefix)