Security concerns over IP replacement done in rt-config.php

[gagan0123]

Bug Report

Current Behavior Assumption: While creating rt-config.php it was assumed that if HTTP_CF_CONNECTING_IP header is there, it means the request is coming from Cloudflare and we replace REMOTE_ADDR and HTTP_X_REAL_IP with HTTP_CF_CONNECTING_IP.

Though true, most of the time, this assumption can lead to security risks, because, if the site is not behind Cloudflare, and a user simply edits his/her headers, to make it look like it is, then there's no check for its validity.

There can be any number of plugins in the site, that might be using the values of HTTP_X_REAL_IP or REMOTE_ADDR without checking if its even an IP address or not, and since, anyone can change that value, we can insert anything in DB(in case that plugin is saving the data in DB) 😈

Expected behavior/code Before replacing the values, it must be validated that the request is coming from Cloudflare's IP and even after that, it must be validated that the header actually contains an IP address. Something like this: https://github.com/cloudflare/cf-ip-rewrite/blob/master/src/CloudFlare/IpRewrite.php#L48-L87

Steps to reproduce the bug On any client site, that was built using fork of this repo, which is not using Cloudflare, and has a plugin that stores HTTP_X_REAL_IP or REMOTE_ADDR without validating(many plugins do that unfortunately, since no one expects someone will replace these values without validation).

Eg: In this site, I can fool this function, that validates IP address from a pool of zombaio_ips https://github.com/rtCamp/freebiemom.com/blob/2261a62713eb6201d152bc83118a7e5825122fcd/wp-content/plugins/mycred/addons/buy-creds/gateways/zombaio.php#L91

Even Sucuri is taking in REMOTE_ADDR, but its validation is useless since, we've set the value to whatever user wanted. https://github.com/rtCamp/greaterkashmir.com/blob/df214c786f5f6895d352522a245ccebfd29a7627/plugins/sucuri-scanner/src/base.lib.php#L508

This even allows anyone to bypass any security measure any plugin would have took to ensure, that the request came from Cloudflare or not, for instance sucuri scanner, redirection plugin, defender security etc...

Possible Solution Use something like this: https://github.com/cloudflare/cf-ip-rewrite/blob/master/src/CloudFlare/IpRewrite.php#L48-L87

instead of direct replacement of IPs, in global $_SERVER variable.

[pradeep910]

@gagan0123 I think we can check server variables using filter_input, so even if someone tries to add malicious code / string it can be sanitized with FILTER_SANITIZE_STRING.

https://github.com/rtCamp/wordpress-skeleton/blob/main/rt-config/rt-config.php#L18-L21

[gagan0123]

@pradeep910 filter_input with FILTER_VALIDATE_IP can help a bit validating that its an IP, but it does not rectify the issue, because the user will still be able to mask or manipulate his/her IP address, which is undesirable/still a security concern. So if we need to replace that value in Super Global variable, we need to ensure that the request really came from Cloudflare, and not someone else.

So, either

1. Do not replace and simply remove those lines.
2. Ensure the request originated from Cloudflare.