SSL Labs Score

Log in to participate

There is no cost to join RicheyWeb, and membership is a requirement to submit bug reports and participate in the support forums.

public static function getIP(): BUG report and Cloudflare compatibility

More
2 months 2 weeks ago - 2 months 2 weeks ago #322 by Markantonio
Markantonio created the topic: public static function getIP(): BUG report and Cloudflare compatibility
HI,
I was modifying the function "public static function getIP()" (which can be found inside the file "aehelper.class.php") for adding support to Cloudflare, when I found a weird bug.
First things first, Cloudflare is a service for protecting websites: since it requires webmasters to modify DNS records on the host, the traffic passes thru cloudflare, therefore the IP the host sees is not the one of the user but that of Cloudflare. To circumvent this. Cloudflare adds a HTTP header HTTP_CF_CONNECTING_IP containing the IP address of the user (I found some info on www.drupal.org/node/1138300#comment-4399384 which, anyway, is about another CMS, but that's irrelevant).
Sometimes Joomla seems to be able to fix this (the developers probably know very well what Cloudflare is), so there should be no need to take this into account, but, since it's better safe than sorry, I added a first line in the function for taking it into account.
Creating a simple info.php file will show all the variables:
<?php phpinfo(); ?>

I changed the code in the function from:
$ip = getenv("HTTP_CLIENT_IP") ? :
	getenv("HTTP_X_FORWARDED_FOR") ? :
		getenv("HTTP_X_FORWARDED") ? :
			getenv("HTTP_FORWARDED_FOR") ? :
				getenv("HTTP_FORWARDED") ? :
					getenv("REMOTE_ADDR");

to:
$ip = getenv("HTTP_CF_CONNECTING_IP") ? :
	getenv("HTTP_CLIENT_IP") ? :
		getenv("HTTP_X_FORWARDED_FOR") ? :
			getenv("HTTP_X_FORWARDED") ? :
				getenv("HTTP_FORWARDED_FOR") ? :
					getenv("HTTP_FORWARDED") ? :
						getenv("REMOTE_ADDR");

Another problem is about getenv("HTTP_X_FORWARDED_FOR"): it can return multiple IP addresses, separated by a comma. Since the first one is the only useful one, I added a line to remove the others:
if(($cp = strpos($ip, ',')) > 0) $ip = substr($ip, 0, $cp);

Finally, I found a bug in the line
if (!filter_var($ip, FILTER_VALIDATE_IP) === false) {

As we can see, we have both a ! before filter_val, which of course negates its logic value, and then a strict comparison === false. One of them must be removed.

In short, the function should look like
public static function getIP() {
	$ip = getenv("HTTP_CF_CONNECTING_IP") ? :	// CloudFlare
		getenv("HTTP_CLIENT_IP") ? :
			getenv("HTTP_X_FORWARDED_FOR") ? :
				getenv("HTTP_X_FORWARDED") ? :
					getenv("HTTP_FORWARDED_FOR") ? :
						getenv("HTTP_FORWARDED") ? :
							getenv("REMOTE_ADDR");
				
	if(($cp = strpos($ip, ',')) > 0) $ip = substr($ip, 0, $cp);	// Removing comma and whatever follows

	if (!filter_var($ip, FILTER_VALIDATE_IP)) {
		$ip = getenv("REMOTE_ADDR");
	}
	return $ip;
}
Last Edit: 2 months 2 weeks ago by Markantonio.

Please Log in or Create an account to join the conversation.

More
2 months 1 week ago #323 by michael
michael replied the topic: public static function getIP(): BUG report and Cloudflare compatibility
I like it!

I'll have a closer look at that and see what I can do for future versions. I'm fairly certain that requesting HTTP_CF_CONNECTING_IP should be benign on systems not hosted with cloudflair, but I want to be sure before I use that.

Also, regarding HTTP_X_FORWARDED_FOR - I think it would be easier (read, less processor/memory intensive) to do something like this:

trim(array_shift(explode(',',getenv('HTTP_X_FORWARDED_FOR'))))

Even if it has a comma, it will return the first IP, if it doesn't - it will return the entire string.

I'll play with it. I'd like to keep everything in the nested ternary if I can.....or not...whatever.

Please Log in or Create an account to join the conversation.

  • Not Allowed: to create new topic.
  • Not Allowed: to reply.
  • Not Allowed: to add attachements.
  • Not Allowed: to edit your message.
Kunena Forum