WordPress.org

Ready to get started?Download WordPress

Forums

WP Statistics
[resolved] statistics.class.php get_IP() X-Forwarded-For Header Bug (5 posts)

  1. Stephanos Io
    Member
    Posted 2 months ago #

    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/

  2. Greg Ross
    Member
    Plugin Author

    Posted 2 months ago #

    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;
    }
  3. Stephanos Io
    Member
    Posted 2 months ago #

    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];
    }
  4. Greg Ross
    Member
    Plugin Author

    Posted 2 months ago #

    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.

  5. Greg Ross
    Member
    Plugin Author

    Posted 1 month ago #

    Closing due to inactivity.

Reply

You must log in to post.

About this Plugin

About this Topic

Tags

No tags yet.