Fatal error with reverse proxy setting on – getRemoteIp() on null on line 597

Hi doob8!

Please reinstall the plugin. I’ve uploaded updated release.

Thanks. It doesn’t crash the wp-admin any longer, which is a good thing. ??

But it doesn’t work properly either. Now, in Access Lists Tab it shows:
Your IP: 127.0.0.1

HTTP_X_FORWARDED_FOR is properly comma-seperated filled with both IPs: of the reverse proxy and the real IP.

I assume it could hardly recognize and block any attackers IP in current state? ;o)

I’ve updated and uploaded new release. Please reinstall the plugin and let me know how it goes.

Many thanks for your fast reply.

With your fix I get the reverse proxy IP shown instead of my real IP. Maybe this could be the hint for some additional checking. I looked into the phpinfo-variables a little closer. Possibly this could help:

HTTP_X_REAL_IP = reverse proxy IP
REMOTE_ADDR = reverse proxy IP
HTTP_X_FORWARDED_FOR = my client IP, reverse proxy IP (order may possibly change)

It is a bit strange that the HTTP_X_REAL_IP is the same as the reverse proxy IP and not the real client IP, what it should be in my understanding.

I assume the currently determined IP out of HTTP_X_FORWARDED_FOR must be different to the IP in REMOTE_ADDR.

Hope this helps. Thanks again.

Yes, it helps. I will update the plugin soon.

Many thanks for the v2.7.2. Now, it gets the correct IP. But only if the order in HTTP_X_FORWARDED_FOR is unchanged.

So, I dived into the code and would like to make a suggestion. I made a little private function to determine the real client IP. I’ve tried to make this process more reliable to get the real IP under different circumstances. I hope the inline-comments are understandable.
I guess, it is not perfect, but maybe an improvement. What do you think?

Here comes the function to get the real client IP:

private function getRealRemoteIp() {
	$return_ip = null;
	// reverse proxy environments
	if (cerber_get_options('proxy')  && isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
		$list = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']);
		foreach ($list as $maybe_ip) {
			$return_ip = filter_var(trim($maybe_ip), FILTER_VALIDATE_IP);
			// in reverse proxy environment REMOTE_ADDR has the proxy IP
			// in IP is different to REMOTE_ADDR this should be the client IP
			// In case that there is only one IP in HTTP_X_FORWARDED_FOR this
			// could be the server IP anyways.
			if ($return_ip && $return_ip != $_SERVER['REMOTE_ADDR']) break;
        }
		// HTTP_X_REAL_IP should only be set behind proxy.
		// Some servers fills its own IP, so we have to check this.
		if (isset($_SERVER['HTTP_X_REAL_IP']) &&
			$_SERVER['HTTP_X_REAL_IP'] != $_SERVER['REMOTE_ADDR'] &&
			$_SERVER['HTTP_X_REAL_IP'] != $return_ip) {
				$tmp = filter_var($_SERVER['HTTP_X_REAL_IP'], FILTER_VALIDATE_IP);
				// only overwrite the already set variable if we have a valid IP from HTTP_X_REAL_IP
				$return_ip = ($tmp) ? $tmp : $return_ip;
		}
	// non-proxy environment
	} else {
		if (isset($_SERVER['REMOTE_ADDR'])) 		$return_ip = $_SERVER['REMOTE_ADDR'];
		elseif (isset($_SERVER['HTTP_CLIENT_IP']))	$return_ip = $_SERVER['HTTP_CLIENT_IP'];
		elseif (isset($_SERVER['SERVER_ADDR']))		$return_ip = $_SERVER['SERVER_ADDR'];
		$return_ip = filter_var($return_ip, FILTER_VALIDATE_IP);
	}
	// No IP address was found? Roll back to localhost.
	if (!$return_ip) $return_ip = '127.0.0.1';
	return $return_ip;
}

Now, the __construct could be shrinked like this:

function __construct() {
	$this->remote_ip = $this->getRealRemoteIp();

	$this->status = 0; // Default: OK!
	if (cerber_is_citadel()) $this->status = 3;
	elseif (!cerber_is_allowed($this->remote_ip)) {
		if (cerber_acl_check($this->remote_ip,'B')) $this->status = 1;
			else $this->status = 2;
	}
}

Thanks for your interesting idea! I need some time to consider it. I am concerned about “HTTP_X_REAL_IP should only be set behind proxy”. I am concerned about compatibility with other environments. Because every new step in the algorithm adds complexity, but nobody takes it away. More complex solution will generate more complex troubles in the future we can run into.

It is just a proposal, because the current version 2.7.2 will not handle the client address properly, if the proxy IP is listed first in HTTP_X_FORWARDED_FOR. Furthermore I made some research and came to the point that HTTP_X_REAL_IP is only be useful behind proxies. But this don’t have to be true. So, I moved that part from the no-proxy-part to the proxy case.

I fully agree, that reliability should be the first goal. Complex software means always some side effects and/or undetected software flaws.
I made myself various tests and tried to cheat the method with different values and lists in all of the server variables. In my humble opinion, i found the method very reliable for different environments. But I absolutely agree, please do your own code testing.

I would be very glad if you consider it as useful as I do. But I would also be happy, if you show me some of its disadvantages, too.

Wish you the best.

Thanks for you thoughts, doob8! I added constant CERBER_IP_KEY for pro users and HTTP_X_REAL_IP for proxy environment as last chance to get IP address. I think that is more then enough.
If some weird hosting provider has weird values in $_SERVER variable, it will not help. I will not waste my time on those insane cases. It doesn’t worth it.