Support » Plugin: WP Statistics » statistics.class.php get_IP() X-Forwarded-For Header Bug

  • Resolved Stephanos Io

    (@stephanosanio)


    Description:
    When a server network is configured to re-route HTTP requests through a reverse proxy (e.g. IIS Application Request Routing), $_SERVER[‘REMOTE_ADDR’] contains the reverse proxy server’s IP while $_SERVER[‘HTTP_X_FORWARDED_FOR’] contains the original client IP and the remote port number that connected to the reverse proxy server.

    For example, if the original client address is 143.112.38.52 and the reverse proxy server address is 10.0.0.1, $_SERVER[‘REMOTE_ADDR’] = ‘10.0.0.1’ and $_SERVER[‘HTTP_X_FORWARDED_FOR’] = ‘143.112.38.52:12345’.

    In this case, get_IP() fails to retrieve the original client IP 143.112.38.52 from HTTP_X_FORWARDED_FOR and returns REMOTE_ADDR 10.0.0.1.

    This leads to all visitors showing up as the reverse proxy server IP 10.0.0.1.

    Analysis:
    The primary source of problem is the following section of the code:

    // If http headers exist, use them.
    if( $temp_ip != $_SERVER['REMOTE_ADDR'] ) {
      // Check to make sure the http header is actually an IP address and not some kind of SQL injection attack.
      $long = ip2long($this->ip);
    
      // ip2long returns either -1 or FALSE if it is not a valid IP address depending on the PHP version, so check for both.
      if($long == -1 || $long === FALSE) {
        // If the headers are invalid, use the server variable which should be good always.
        $temp_ip = $_SERVER['REMOTE_ADDR'];
      }
    }

    Before this section of code, $temp_ip is initially assigned $_SERVER[‘REMOTE_ADDR’] and assigned to an alternate header value (in this case, HTTP_X_FORWARDED_FOR) if it exists.

    The first if statement in the bugged section checks if an alternate header exists, and if it exists ($temp_ip != REMOTE_ADDR), it attempts to verify the value of the alternate header by using ip2long function.

    Note that the alternate header IP address value is contained in $temp_ip, yet the following line checks $this->ip for the header value.
    $long = ip2long($this->ip);

    Since the value of the alternate header to be verified is contained in $temp_ip and $this->ip is never assigned a value prior to this, this effectively leads to $temp_ip always getting assigned REMOTE_HOST value regardless of what is contained in the alternate header.

    This line needs to be modified to:
    $long = ip2long($temp_ip);

    Another problem is that IIS Application Request Routing reverse proxy implementation provides X-Forwarded-For header in IPADDR:PORT format rather than simple IPADDR format as shown in the description above.

    In order to extract the IPADDR from X-Forwarded-For header,
    $temp_ip = getenv('HTTP_X_FORWARDED_FOR'); should be changed to,
    $temp_ip = explode(':', getenv('HTTP_X_FORWARDED_FOR'))[0];

    https://wordpress.org/plugins/wp-statistics/

Viewing 4 replies - 1 through 4 (of 4 total)
  • Plugin Contributor Greg Ross

    (@gregross)

    Good catch, I’ve made a couple of changes as follows, give it a try and make sure it catches all of your issues and I’ll include it in the next release:

    public function get_IP() {
    
    	// By default we use the remote address the server has.
    	$temp_ip = $_SERVER['REMOTE_ADDR'];
    
    	// Check to see if any of the HTTP headers are set to identify the remote user.
    	// These often give better results as they can identify the remote user even through firewalls etc,
    	// but are sometimes used in SQL injection attacks.
    	if (getenv('HTTP_CLIENT_IP')) {
    		$temp_ip = getenv('HTTP_CLIENT_IP');
    	} elseif (getenv('HTTP_X_FORWARDED_FOR')) {
    		$temp_ip = getenv('HTTP_X_FORWARDED_FOR');
    	} elseif (getenv('HTTP_X_FORWARDED')) {
    		$temp_ip = getenv('HTTP_X_FORWARDED');
    	} elseif (getenv('HTTP_FORWARDED_FOR')) {
    		$temp_ip = getenv('HTTP_FORWARDED_FOR');
    	} elseif (getenv('HTTP_FORWARDED')) {
    		$temp_ip = getenv('HTTP_FORWARDED');
    	} 
    
    	// Trim off any port values that exist.
    	if( strstr( $temp_ip, ':' ) !== FALSE ) {
    		$temp_a = explode(':', $temp_ip);
    		$temp_ip = $temp_a[0];
    	}
    
    	// Check to make sure the http header is actually an IP address and not some kind of SQL injection attack.
    	$long = ip2long($temp_ip);
    
    	// ip2long returns either -1 or FALSE if it is not a valid IP address depending on the PHP version, so check for both.
    	if($long == -1 || $long === FALSE) {
    		// If the headers are invalid, use the server variable which should be good always.
    		$temp_ip = $_SERVER['REMOTE_ADDR'];
    	}
    
    	$this->ip = $temp_ip;
    
    	return $this->ip;
    }
    Thread Starter Stephanos Io

    (@stephanosanio)

    Thanks for the prompt reply. The fix looks good except in the following part:

    // Trim off any port values that exist.
    if( strstr( ':', $temp_ip ) !== FALSE ) {
      $temp_ip = explode($temp_ip, ':'))[0];
    }

    string strstr ( string $haystack , mixed $needle [, bool $before_needle = false ] )
    – Returns the portion of string, or FALSE if needle is not found.

    array explode ( string $delimiter , string $string [, int $limit ] )

    I believe the code above should be changed to:

    // Trim off any port values that exist.
    if( strstr( $temp_ip, ':' ) !== FALSE ) {
      $temp_ip = explode(':', $temp_ip)[0];
    }

    Plugin Contributor Greg Ross

    (@gregross)

    Yea, I already edited the post but you must have gotten to it before I reposted it 😉

    The explode()[] format also doesn’t work on older version of PHP so I used a temp array variable as well. See the above updated function and give it a try.

    Plugin Contributor Greg Ross

    (@gregross)

    Closing due to inactivity.

Viewing 4 replies - 1 through 4 (of 4 total)
  • The topic ‘statistics.class.php get_IP() X-Forwarded-For Header Bug’ is closed to new replies.