Jump to content



Photo
- - - - -

getIPAddress() development


  • Please log in to reply
16 replies to this topic

#1   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 29 November 2011 - 00:40

/**
 * Get the IP address of the client
 *
 * @since v3.0.0
 */
 
    public static function getIPAddress() {
      if ( isset($_SERVER['HTTP_X_FORWARDED_FOR']) ) {
        $ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
      } elseif ( isset($_SERVER['HTTP_CLIENT_IP']) ) {
        $ip = $_SERVER['HTTP_CLIENT_IP'];
      } else {
        $ip = $_SERVER['REMOTE_ADDR'];
      }
 
      return $ip;
    }

This looks like it needs a lot of work.

- HTTP_X_FORWARDED_FOR can often be stacked with a string of IP addresses
- With some 'cloud' or server farm configurations like Rackspace, the HTTP_X_CLUSTER_CLIENT_IP is the actual IP on others it is not
That means it is possible to be registering an upstream IP of a load-bearing switch or proxy server as the IP instead of the real one.
- Both CLIENT_IP and HTTP_X_FORWARDED_FOR addresses can be spoofed so need to be validated for example using FILTER_VALIDATE_IP
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#2   Debs

Debs
  • Members
  • 133 posts
  • Real Name:Debs
  • Gender:Female
  • Location:Fargo, ND UNITED STATES

Posted 29 November 2011 - 11:09

Perhaps this could be helpful to you. I have successfully used this for a script I made (to read geoip and direct or block traffic). It should catch proxy traffic etc. correctly. It has served me well for a couple years. I am not aware that it could be easily spoofed.

function getUserIP() {
$IP_seeker = '';
if (getenv('HTTP_CLIENT_IP')) {
$IP_seeker = getenv('HTTP_CLIENT_IP');
} elseif (getenv('HTTP_X_FORWARDED_FOR')) {
$IP_seeker = getenv('HTTP_X_FORWARDED_FOR');
} elseif (getenv('HTTP_X_FORWARDED')) {
$IP_seeker = getenv('HTTP_X_FORWARDED');
} elseif (getenv('HTTP_FORWARDED_FOR')) {
$IP_seeker = getenv('HTTP_FORWARDED_FOR');
} elseif (getenv('HTTP_FORWARDED')) {
$IP_seeker = getenv('HTTP_FORWARDED');
} else {
$IP_seeker = $_SERVER['REMOTE_ADDR'];
}
return $IP_seeker;
}

#3   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 29 November 2011 - 12:07

Some proxy servers will allow for client ip spoofing but most that do will only allow an actual IP address to be passed.

Example header:


GET /index.php HTTP/1.1
Host: www.testsite.local
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.24) Gecko/20111103 Firefox/3.6.24
Accept: */*
Client-IP: 111.222.333.444

Some proxy servers will pass that client-ip through even though it is a made up/forged client-ip.

Other configurations will pass through the forged client-IP but still send the correct HTTP_X_FORWARDED_FOR ip address.


The HTTP_X_FORWARDED_FOR 'can be' completely spoofed where code can be sent rather than an IP address which is why it is an absolute must to test to make sure the HTTP_X_FORWARDED_FOR - which is often a comma separated list of IPs, are actually IP addresses if it is intended to use an ip address for the HTTP_FORWARDED_FOR stack.

Example header

 
GET /index.php HTTP/1.1
Host: www.testsite.local
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.24) Gecko/20111103 Firefox/3.6.24
Accept: */*
X-FORWARDED-FOR: <script>alert(document.cookie)</script>

This will pop an alert with the cookie displayed

On top of all of that there are the following to check for as well:
- $_SERVER[ "HTTP_PROXY_USER" ]
- $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ]


Then there are the peculiar configurations where the $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] is the same as the $_SERVER[ "REMOTE_ADDR" ] then the $_SERVER[ "HTTP_X_FORWARDED_FOR" ] if set, is the actual visitors IP address. However if those two IP addresses differ then the $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] is the actual visitors IP address.

Example header:

 
GET /index.php HTTP/1.1
Host: www.testsite.local
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.24) Gecko/20111103 Firefox/3.6.24
Accept: */*
X-FORWARDED-FOR: 111.111.111.111,222.222.222.222
X-CLUSTER-CLIENT-IP: 123.123.123.123

If the $_SERVER[ "REMOTE_ADDR" ] is 123.123.123.123 (which is the same as the X_CLUSTER_CLIENT_IP above) then the users actual users IP address will be 222.222.222.222 (the last ip in the X-FORWARDED-FOR stack).
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#4   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 29 November 2011 - 12:09

Here is a function from osC_Sec which attempts to find the correct IP address:

 
	/**
	 * getRealIP()
	 *
	 * @return
	 */
	function getRealIP() {
		if ( isset( $_SERVER[ "HTTP_X_FORWARDED_FOR" ] ) && !empty( $_SERVER[ "HTTP_X_FORWARDED_FOR" ] ) ) {
			foreach ( array_reverse( explode( ",", $_SERVER[ "HTTP_X_FORWARDED_FOR" ] ) ) as $xff_ip ) {
				$xff_ip = trim( $xff_ip );
  
				if ( $this->check_ip( $xff_ip ) ) {
					$ip_addresses[] = $xff_ip;
				}
			}
			foreach ( $ip_addresses as $ip ) {
				if ( !empty( $ip ) && $this->check_ip( $ip ) ) {
					$HTTP_X_FORWARDED_FOR = $ip;
					break;
				}
			}
		}
		if ( isset( $_SERVER[ "HTTP_PROXY_USER" ] ) && !empty( $_SERVER[ "HTTP_PROXY_USER" ] ) ) {
			if ( false !== $this->check_ip( $_SERVER[ "HTTP_PROXY_USER" ] ) ) return $_SERVER[ "HTTP_PROXY_USER" ];
		}
		# this is to deal with the rackspace type server clouds
		if ( isset( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) && isset( $HTTP_X_FORWARDED_FOR ) ) {
			# if the cluster client ip is the same as the remote addr
			if ( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] == $_SERVER[ "REMOTE_ADDR" ] ) {
				# then the http x forwarded for will be the users IP address
				return $HTTP_X_FORWARDED_FOR;
			} else {
				# if not the same then the cluster client ip will prob be the user IP, either way
				# it wont be the remote addr which will always in this instance be the proxy
				if ( false !== $this->check_ip( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) )
					return $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ];
			}
		}
		if ( isset( $_SERVER[ "HTTP_CLIENT_IP" ] ) && !empty( $HTTP_X_FORWARDED_FOR ) ) {
			if ( false !== $this->check_ip( $_SERVER[ "HTTP_CLIENT_IP" ] ) ) return $_SERVER[ "HTTP_CLIENT_IP" ];
		}
		return $_SERVER[ "REMOTE_ADDR" ];
	}

The check_ip() function is quite simple and checks the format of IPv4 addresses:
 
	/**
	 * check_ip()
	 *
	 * @param mixed $ip
	 * @return
	 */
	function check_ip( $ip ) {
		if ( false !== ( bool )preg_match( "/^([1-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])" . "(\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])){3}$/", trim( $ip ) ) ) {
			return true;
		} else $this->senda403Header();
	}

- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#5   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 29 November 2011 - 12:22

osCommerce 2.3.1 has a good attempt at this too via the tep_get_ip_address() in includes/functions/general.php

Somewhere between all of those above include 2.3.1, will be the right balance for determining the IP address.

The 2.3.1 ip address validation function is:
 
  function tep_validate_ip_address($ip_address) {
    if (function_exists('filter_var') && defined('FILTER_VALIDATE_IP')) {
      return filter_var($ip_address, FILTER_VALIDATE_IP, array('flags' => FILTER_FLAG_IPV4));
    }
 
    if (preg_match('/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/', $ip_address)) {
      $parts = explode('.', $ip_address);
 
      foreach ($parts as $ip_parts) {
        if ( (intval($ip_parts) > 255) || (intval($ip_parts) < 0) ) {
          return false; // number is not within 0-255
        }
      }
 
      return true;
    }
 
    return false;
  }

Achieves the same as mine, but probably does a better job at determining a clean IPv4 ip address format.
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#6   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 30 November 2011 - 13:01

/**
 * Get the IP address of the client
 *
 * @since v3.0.0
 */
 
    public static function getIPAddress() {
      if ( isset($_SERVER['HTTP_X_FORWARDED_FOR']) ) {
        $ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
      } elseif ( isset($_SERVER['HTTP_CLIENT_IP']) ) {
        $ip = $_SERVER['HTTP_CLIENT_IP'];
      } else {
        $ip = $_SERVER['REMOTE_ADDR'];
      }
 
      return $ip;
    }


Apart from the inherent issues with the function above, i.e. the HTTP_X_FORWARDED_FOR can often be a string of IPs separated by a comma or space, the issue is that at least two of those headers above can be spoofed by an attacker.

These headers can be spoofed:
- HTTP_PROXY_USER
- HTTP_X_FORWARDED_FOR
- HTTP_CLIENT_IP
- HTTP_PROXY_USER
- HTTP_X_CLUSTER_CLIENT_IP

Many 3rd party payment modules depend on the correct ip address as part of their authenticating their callbacks from their server IP addresses.

Chronopay for example: (ext/modules/payment/chronopay/callback.php)
$ip_address = tep_get_ip_address();
 
  if ( ($ip_address == '69.20.58.35') || ($ip_address == '207.97.201.192') ) {

If an attacker sent a spoofed HTTP_CLIENT_IP or spoofed HTTP_X_FORWARDED_FOR of any of those two ip addresses they could create a bit of mischief.

To a lesser extent/impact, ip logging via action_recorder could also be affected by this issue.

It is tricky because there are legitimate situations where the HTTP_X_FORWARDED_FOR would be the correct ip address, as mentioned, for example on clustered server configs where the HTTP_X_CLUSTER_CLIENT_IP and the REMOTE_ADDR are the same IP address (proxy servers). In that instance, where a 3rd party payment provider depended on the REMOTE_ADDR, well this would cause callbacks to fail where the REMOTE_ADDR was expected to be the IP address of the 3rd party payment processor server and not a reverse proxy server, load balancer or gateway server further up the chain within the cluster.

The REMOTE_ADDR in many of those configurations could well be a proxy server of some form further up the chain whereas the HTTP_X_FORWARDED_FOR could be the only header that contains the correct IP address 69.20.58.35.

But you just cannot depend on the HTTP_X_FORWARDED_FOR or $_SERVER['HTTP_CLIENT_IP']. By themselves, they are not dependable ways of determining the correct users IP address.

The solution may be to try and automatically determine the reverse proxy server addresses on setting up the site, and then register those in the configure.php settings.

Then you can for example on a cluster configuration, determine that because the registered reverse proxy IP (in the configure.php file) and the REMOTE_ADDR are the same, then that is at least a verifiable check that the HTTP_X_FORWARDED_FOR ip will not be spoofed (in cluster configurations).

This is because the REMOTE_ADDR cannot be spoofed, and the clusters reverse proxy/gateway/load balancers ip address is locked into the configure.php file. With those two safe vectors, the ip address determined from the HTTP_X_FORWARDED_FOR header could then be safely set as the visitors IP address.

Without at least those two checks in place, those headers and any of those spoofable headers above should not be depended on by themselves to determine the real IP address.
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#7   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 03 December 2011 - 05:05

Below is a bit of a rewrite of both the tep_validate and tep_get_ip functions

The concept here is as follows:
On install, if HTTP_X_FORWARDED_FOR is set, the HTTP_REVERSE_PROXY_IP is discovered from the HTTP_X_FORWARDED_FOR (since the site installer can be trusted not to spoof the HTTP_X_FORWARDED_FOR) then it has to be defined in the configure.php file then the 'principle ideas' in following functions would be the best method of determining the ip address.
  function tep_validate_ip_address( $ip_address ) {
	  # This has been updated to include IPv6
	  if ( function_exists( 'filter_var' )
		  && defined( 'FILTER_VALIDATE_IP' )
		  && defined( 'FILTER_FLAG_IPV4' )
		  && defined( 'FILTER_FLAG_IPV6' ) ) {
		  if ( filter_var( $ip, FILTER_VALIDATE_IP,
								FILTER_FLAG_IPV4 ||
								FILTER_FLAG_IPV6 ) !== false ) {
								return $ip;
		  }
	  }
 
	if ( preg_match( '/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/', $ip_address ) ) {
	  $parts = explode( '.', $ip_address );
 
	  foreach ( $parts as $ip_parts ) {
		if ( ( intval( $ip_parts ) > 255 ) || ( intval( $ip_parts ) < 0 ) ) {
		  return false; // number is not within 0-255
		}
	  }
	  return true;
	}
	return false;
  }
 
  function tep_get_ip_address() {
	global $_SERVER;
 
	if ( isset( $_SERVER ) ) {
	  # Cloudfare HTTP_CF_CONNECTING_IP cannot be spoofed but research needs to be
	  # done to securely determine if Cloudfare is installed. With that check in place, the
	  # following could be allowed
	  /* if ( isset( $_SERVER[ "HTTP_CF_CONNECTING_IP" ] ) && !empty( $_SERVER[ "HTTP_CF_CONNECTING_IP" ] ) ) {
		   $ip_addresses[] = $_SERVER[ "HTTP_CF_CONNECTING_IP" ];
		}*/
	  # This below is the only time an ip address can be safely
	  # determined from any header other than the _SERVER variable REMOTE_ADDR
	  # In order for this to work, HTTP_REVERSE_PROXY_IP needs to be defined in configure.php
	  # in order to avoid this function from falling for spoofed HTTP_X_FORWARDED_FOR IP addresses
	  if ( ( isset( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) // cloud configuration detected - or spoofed ip header
		&& !empty( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) )
		&& ( defined( 'HTTP_REVERSE_PROXY_IP' ) ) ) { // defined in configure.php
		
				# where the HTTP_X_CLUSTER_CLIENT_IP = the remote address but differs from the reverse proxy IP
				# and HTTPS is off, then the remote_addr is the users ip address
				# see: http://goo.gl/xfVls
		if ( ( getenv( "HTTPS" ) == "off" )
		  && ( tep_validate_ip_address( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) == $_SERVER[ "REMOTE_ADDR" ] )
		  && ( $_SERVER[ "REMOTE_ADDR" ] !== HTTP_REVERSE_PROXY_IP )
		  && ( tep_validate_ip_address( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) !== HTTP_REVERSE_PROXY_IP ) ) {
			  # ignore HTTP_X_FORWARDED_FOR even if it is set
			  # The users ip in this instance is still the REMOTE_ADDR
			  $ip_addresses[] = $_SERVER[ "REMOTE_ADDR" ];
		}
		# When using https, many cloud configurations employ HTTP_X_FORWARDED_FOR for the
		# user ip address. See: http://goo.gl/xfVls
		# However the HTTP_X_FORWARDED_FOR can be spoofed so relying on it could
		# result in an attacker exploiting this by sending both an HTTP_X_CLUSTER_CLIENT_IP
		# and an HTTP_X_FORWARDED_FOR to trigger this function to accept their spoofed ip
		# address.
		if ( ( getenv( "HTTPS" ) == "on" )
		  && ( isset( $_SERVER[ "HTTP_X_FORWARDED_FOR" ] )
		  && !empty( $_SERVER[ "HTTP_X_FORWARDED_FOR" ] ) )
		  && ( tep_validate_ip_address( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) == HTTP_REVERSE_PROXY_IP )
		  && ( $_SERVER[ "REMOTE_ADDR" ] == HTTP_REVERSE_PROXY_IP )
		  && ( false !== strpos( $_SERVER[ "HTTP_X_FORWARDED_FOR" ], "," ) ) ) {
			$ip = explode( ',', $_SERVER[ "HTTP_X_FORWARDED_FOR" ] );
			$ip_addresses[] = tep_validate_ip_address( trim( $ip[ count( $ip ) - 1 ] ) ); // <= UNRELIABLE!!! but a step up from the current method
		}
		  
	  }
	  # Get the REMOTE_ADDR
	  $ip_addresses[] = $_SERVER[ "REMOTE_ADDR" ];
	}
	  
		
	  # Set the first IP address in the array as the users IP
	  foreach ( $ip_addresses as $ip ) {
		if ( !empty( $ip ) && tep_validate_ip_address( $ip ) ) {
		  $ip_address = $ip;
		  break;
		}
	  }
	return $ip_address;
  }

In the concept above, an attacker could still spoof the HTTP_X_FORWARDED_FOR but it would need several other parameters to be true in order for it to work, i.e the server must be running behind SSL which 'can' be triggered on some configurations by calling the url with https, but many webservers do not allow this if HTTPS is set to off, and, HTTP_REVERSE_PROXY_IP has to be defined as well in order for the function to accept an ip address from the HTTP_X_FORWARDED_FOR.

Its not a perfect idea by any stretch of the imagination and it needs work, but it could be a step in the right direction?

I do not have a test site set up on a distributed load baring webserver, so the concepts above are not tested in the wild and are mostly theories derived from reading: http://goo.gl/xfVls

Anyone have an opinion on this. I am all ears.
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#8   Harald Ponce de Leon

Harald Ponce de Leon

    Healthy Giraffe

  • Core Team
  • 4,893 posts
  • Real Name:Harald Ponce de Leon
  • Gender:Male
  • Location:Solingen, Germany

Posted 03 December 2011 - 13:32

Hi Te..

Great concept! It would be nice to have a generalized solution other projects can also use. Perhaps publishing an initial repository on Github and posting it to the PHP general mailing list could help bring in others to work on and finalize a standard routine.

The logic could be slightly improved:

1) tep_validate_ip_address() should either return a boolean or a string (ip address), not both; perhaps with isValidIpAddress() and getFilteredIpAddress() functions.

2) If the filter_var() function exists in tep_validate_ip_address(), a check on $ip is performed and it's value returned - this should be $ip_address as $ip is not defined.

3) In tep_get_ip_address(), if $_SERVER is not set, a foreach on $ip_addresses is performed although the variable has not been set.

Kind regards,
Harald Ponce de Leon

#9   Harald Ponce de Leon

Harald Ponce de Leon

    Healthy Giraffe

  • Core Team
  • 4,893 posts
  • Real Name:Harald Ponce de Leon
  • Gender:Male
  • Location:Solingen, Germany

Posted 03 December 2011 - 13:51

Hi Te..

Some good logic optimizations can also be found here:

http://stackoverflow.com/a/2031935

Kind regards,
Harald Ponce de Leon

#10   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 03 December 2011 - 19:15

Thanks for that Harald, here is a rejig of the code:

 
  function validate_ipaddress( $ip_address ) {
	  # This has been updated to include IPv6
	  if ( function_exists( 'filter_var' )
		  && defined( 'FILTER_VALIDATE_IP' )
		  && defined( 'FILTER_FLAG_IPV4' )
		  && defined( 'FILTER_FLAG_IPV6' ) ) {
		  if ( filter_var( $ip_address, FILTER_VALIDATE_IP,
								FILTER_FLAG_IPV4 ||
								FILTER_FLAG_IPV6 ) === false ) {
								# do something here, perhaps die();
		  } else return $ip_address;
	  }
 
	if ( preg_match( '/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/', $ip_address ) ) {
	  $parts = explode( '.', $ip_address );
 
	  foreach ( $parts as $ip_parts ) {
		if ( ( intval( $ip_parts ) > 255 ) || ( intval( $ip_parts ) < 0 ) ) {
		  # do something here, perhaps die(); because number is not within 0-255
		}
	  }
	  return $ip_address;
	}
  }
 
  function getIPAddress() {
	# Cloudfare HTTP_CF_CONNECTING_IP cannot be spoofed but research needs to be
	# done to securely determine if Cloudfare is installed. With that check in place, the
	# following could be allowed
	/* if ( isset( $_SERVER[ "HTTP_CF_CONNECTING_IP" ] ) && !empty( $_SERVER[ "HTTP_CF_CONNECTING_IP" ] ) ) {
		 $ip_addresses[] = $_SERVER[ "HTTP_CF_CONNECTING_IP" ];
	  }*/
	# This below is the only time an ip address can be safely
	# determined from any header other than the _SERVER variable REMOTE_ADDR
	# In order for this to work, HTTP_REVERSE_PROXY_IP needs to be defined in configure.php
	# in order to avoid this function from falling for spoofed HTTP_X_FORWARDED_FOR IP addresses
	if ( ( isset( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) // cloud configuration detected - or spoofed ip header
	  && !empty( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) )
	  && ( defined( 'HTTP_REVERSE_PROXY_IP' ) ) ) { // defined in configure.php
	  
			  # where the HTTP_X_CLUSTER_CLIENT_IP = the remote address but differs from the reverse proxy IP
			  # and HTTPS is off, then the remote_addr is the users ip address
			  # see: http://goo.gl/xfVls
	  if ( ( getenv( "HTTPS" ) == "off" )
		&& ( validate_ipaddress( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) == $_SERVER[ "REMOTE_ADDR" ] )
		&& ( $_SERVER[ "REMOTE_ADDR" ] !== HTTP_REVERSE_PROXY_IP )
		&& ( validate_ipaddress( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) !== HTTP_REVERSE_PROXY_IP ) ) {
			# ignore HTTP_X_FORWARDED_FOR even if it is set
			# The users ip in this instance is still the REMOTE_ADDR
			$ip_addresses[] = $_SERVER[ "REMOTE_ADDR" ];
	  }
	  # When using https, many cloud configurations employ HTTP_X_FORWARDED_FOR for the
	  # user ip address. See: http://goo.gl/xfVls
	  # However the HTTP_X_FORWARDED_FOR can be spoofed so relying on it could
	  # result in an attacker exploiting this by sending both an HTTP_X_CLUSTER_CLIENT_IP
	  # and an HTTP_X_FORWARDED_FOR to trigger this function to accept their spoofed ip
	  # address.
	  if ( ( getenv( "HTTPS" ) == "on" )
		&& ( isset( $_SERVER[ "HTTP_X_FORWARDED_FOR" ] )
		&& !empty( $_SERVER[ "HTTP_X_FORWARDED_FOR" ] ) )
		&& ( validate_ipaddress( $_SERVER[ "HTTP_X_CLUSTER_CLIENT_IP" ] ) == HTTP_REVERSE_PROXY_IP )
		&& ( $_SERVER[ "REMOTE_ADDR" ] == HTTP_REVERSE_PROXY_IP )
		&& ( false !== strpos( $_SERVER[ "HTTP_X_FORWARDED_FOR" ], "," ) ) ) {
		  $ip = explode( ',', $_SERVER[ "HTTP_X_FORWARDED_FOR" ] );
		  $ip_addresses[] = validate_ipaddress( trim( $ip[ count( $ip ) - 1 ] ) ); // <= UNRELIABLE!!! but a step up from the current method
	  }
		
	}
	# Get the REMOTE_ADDR
	$ip_addresses[] = $_SERVER[ "REMOTE_ADDR" ];
	  
	# Set the first IP address in the array as the users IP
	foreach ( $ip_addresses as $ip ) {
	  if ( !empty( $ip ) && validate_ipaddress( $ip ) ) {
		$ip_address = $ip;
		break;
	  }
	}
  return $ip_address;
  }

Taipo
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#11   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 03 December 2011 - 20:03

Hi Te..

Some good logic optimizations can also be found here:

http://stackoverflow.com/a/2031935

Kind regards,


Thanks for that.

The primary issue I am wanting to address is the inherent risk presented by IP Address spoofing when using any other $_SERVER array key other than REMOTE_ADDR to determine the visitors ip address.

Again taking the Chronopay callback as an example: (ext/modules/payment/chronopay/callback.php)

  $ip_address = tep_get_ip_address();
 
  if ( ($ip_address == '69.20.58.35') || ($ip_address == '207.97.201.192') ) {
 
Spoofed ip header:
POST /ext/modules/payment/chronopay/callback.php HTTP/1.1
Host: localhost
Accept: */*
Content-Type: application/x-www-form-urlencoded
X-CLUSTER-CLIENT-IP: 69.20.58.35
CLIENT-IP: 207.97.201.192
X-FORWARDED-FOR: 69.20.58.35
 
cs1=1&cs2=1&cs3=1........

By spoofing the IP headers an attacker could quite easily break through the first few lines of defense in callback.php to start creating havoc.

Any 3rd party addon for 2.3.1 and earlier, for example, that currently check the database to match a stored ip address against the visitors ip will fail if ips are spoofed and they are depending on any other array key in $_SERVER other than REMOTE_ADDR, and if it were possible to prevent coders from coding in that manner then it wouldn't matter as much, but that isn't going to happen anytime soon.

All those examples at stackoverflow fail on this issue of ip spoofing.

Unfortunately the only time the string of ips in X-Forwarded-For are completely reliable is when installing osCommerce onto a server...for obvious reasons.

Taipo
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#12   Harald Ponce de Leon

Harald Ponce de Leon

    Healthy Giraffe

  • Core Team
  • 4,893 posts
  • Real Name:Harald Ponce de Leon
  • Gender:Male
  • Location:Solingen, Germany

Posted 03 December 2011 - 20:20

Hi Te..

So only the value of $_SERVER['REMOTE_ADDR'] should be used as the clients IP address, regardless if it's a proxy?

Kind regards,
Harald Ponce de Leon

#13   Harald Ponce de Leon

Harald Ponce de Leon

    Healthy Giraffe

  • Core Team
  • 4,893 posts
  • Real Name:Harald Ponce de Leon
  • Gender:Male
  • Location:Solingen, Germany

Posted 03 December 2011 - 20:24

Hi Te..

Regarding the callback from payment modules, these also check against a secret signature so a request from a spoofed IP address is not going to get far.

Kind regards,
Harald Ponce de Leon

#14   Harald Ponce de Leon

Harald Ponce de Leon

    Healthy Giraffe

  • Core Team
  • 4,893 posts
  • Real Name:Harald Ponce de Leon
  • Gender:Male
  • Location:Solingen, Germany

Posted 03 December 2011 - 20:34

Hi Te..

As there are valid reasons to check against proxies, an alternative method is to use the logic of v2.3.1 for general checks, and for more stricter checks when the origin of the request is known (eg, payment gateways), to bypass the proxies and use REMOTE_ADDR only.

Do you think that would be sufficient?

Kind regards,
Harald Ponce de Leon

#15   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 04 December 2011 - 06:17

Hi Te..

Regarding the callback from payment modules, these also check against a secret signature so a request from a spoofed IP address is not going to get far.

Kind regards,


Sure, and there are 5 or 6 other callback scripts that use tep_get_ip_address() as well, some of which I have looked at.

The addon that immediately looked at risk to me was PHPIDS addon for which I notified the developer.

The point of that issue though was that there is a false sense of security being employed by callback coders if they believe they can rely on HTTP_X_FORWARDED_FOR or HTTP_CLIENT_IP for self identification. More than likely though they are just depending on the function to provide them with an accurate ip address.

To an attacker it presents about 10 seconds worth of extra work to get around that first layer of security if ips are derived from the HTTP_X_FORWARDED_FOR or HTTP_CLIENT_IP without some other true test.

Hi Te..

As there are valid reasons to check against proxies, an alternative method is to use the logic of v2.3.1 for general checks, and for more stricter checks when the origin of the request is known (eg, payment gateways), to bypass the proxies and use REMOTE_ADDR only.

Do you think that would be sufficient?

Kind regards,


The logic in 2.3.1 depends a lot on correct configuration of proxy servers ip string. The order of ips in the HTTP_X_FORWARDED_FOR for example is sometimes in reverse order.

Sometimes its client, proxy1, proxy2, and sometimes its proxy2, proxy1, client.

With the advent of 'cloud' serving, there is a growing assumption that HTTP_X_CLUSTER_CLIENT_IP = the clients ip, but as you can see from that rackspace information, that is not always the case (when HTTPS is on)...and again, HTTP_X_CLUSTER_CLIENT_IP can also be spoofed.

The reason I focussed on the HTTP_X_CLUSTER_CLIENT_IP in the example above is because I think in the near future the majority of osCommerce websites will be on distributed server clusters, because free and cheap webhosting is moving in that direction.

I didn't really intend on taking this much further code wise due to my own time constraints with other work on, but am hoping to get some debate going around the issue of hardening the getIPAddress() (some of which could also be applied to tep_get_ip_address() in any future updates to it in 2.4).

I can imagine the end result will still have its weaknesses but it would be a big step up from this type of approach which is common and also flawed:

if HTTP_X_FORWARDED_FOR is set and not empty then ip = validate( HTTP_X_FORWARDED_FOR )
if HTTP_CLIENT_IP is set and not empty then ip = validate( HTTP_CLIENT_IP )
if HTTP_X_CLUSTER_CLIENT_IP is set and not empty then ip = validate( HTTP_X_CLUSTER_CLIENT_IP )
if REMOTE_ADDR is set and not empty then ip = validate( REMOTE_ADDR )
return ( first ip in array)

The approach I think could be best would be:

1/ have a 'hardened' (or whatever other name) and normal switch option ex: function getIPAddress($hardened=false);

2/ use a function like validate_ipaddress() to check every ip in the HTTP headers including IPv6 formats for future proofing

3/ when the HTTP_X_CLUSTER_CLIENT_IP is present and HTTPS is not on, set the REMOTE_ADDR as the ip address (rather than the HTTP_X_CLUSTER_CLIENT_IP) except when HTTPS is on for which with hardened set to false, the untrustworthy HTTP_X_FORWARDED_FOR will have to be called upon to extract the clients ip (or proxy they are behind).

4/ for everything else, with hardened switched to false, use the logic from 2.3.1 tep_get_ip_address() minus the HTTP_X_CLUSTER_CLIENT_IP which would be already taken care of above.

5/ for gateways, they could register their proxy server ips (if they are behind them) in their callback code so that it can be verified via the REMOTE_ADDR. But the same rules would apply when the site is installed on a cluster/cloud therefore the REMOTE_ADDR would be accurate if HTTPS is off but the only other place you will find the client callback ip would be in the HTTP_X_FORWARDED_FOR if HTTPS is on (again going only by what is gleaned from http://goo.gl/xfVls).

6/ with hardened on, any other method of accurately determining the client ip - registering the reverse proxies on install for example, failing that, REMOTE_ADDR for everything even if it reports wrong.

That would allow for some flexibility around clusters, which as mentioned, will be the norm, and therefore there is a higher need to get those right.

This issue with clusters and HTTPS could also explain why some current users are experiencing callback issues where their sites are hosted on clustered servers. Where HTTPS is on, tep_get_ip_address() would be reporting the wrong ip address of the client/call back server.

Taipo
- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW

#16   Harald Ponce de Leon

Harald Ponce de Leon

    Healthy Giraffe

  • Core Team
  • 4,893 posts
  • Real Name:Harald Ponce de Leon
  • Gender:Male
  • Location:Solingen, Germany

Posted 04 December 2011 - 11:47

Hi Te..

This issue with clusters and HTTPS could also explain why some current users are experiencing callback issues where their sites are hosted on clustered servers. Where HTTPS is on, tep_get_ip_address() would be reporting the wrong ip address of the client/call back server.


Specifically for Rackspace, it should still work unless HTTP_X_FORWARDED_FOR contains a value in the HTTP request. In HTTPS requests, the value from HTTP_X_FORWARDED_FOR is the first to be used in tep_get_ip_address().

Kind regards,
Harald Ponce de Leon

#17   Taipo

Taipo
  • Members
  • 796 posts
  • Real Name:Te Taipo
  • Gender:Male

Posted 12 December 2011 - 20:17

I guess if the ip address is only used for general logging purposes, and callback companies do not exclusively depend on the ip address as their method of security, then yes, in the case when its a callback, the REMOTE_ADDR should be the only ip address used....assuming 3rd party processors IPs are the REMOTE_ADDR...

The point there is to keep in mind that the REMOTE_ADDR can often be (as is the case with rackspace when https is on) the upline load balancing proxy, so that would introduce more problems than it solves if left merely to calling the REMOTE_ADDR for callbacks, for example on the rackspace type cluster configurations.

Edited by Taipo, 12 December 2011 - 20:18.

- Stop Oscommerce hacks dead in their tracks with osC_Sec (see discussion here)
- Another discussion about infected files ::here::
- A discussion on file permissions ::here::
- Site hacked? Should you upgrade or not, some thoughts ::here::
- Fix the admin login bypass exploit here
- Pareto Security: New security addon I am developing, a remake of osC_Sec in PHP 5 with a number of fixes
- BTC:1BkbNA1tK3q7ZRkCJj6f1ELK2A152eEtoW