Home » Download » Drupal project » Drupal » Issues

IP address from XFF header contains spaces

mindgame - September 17, 2008 - 09:44
Project:Drupal
Version:7.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Issue tags:Quick fix, XFF
Description

In "includes/bootstrap.inc", function "ip_address":
Instead of
array_pop(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']))
shouldn't it be
trim(array_pop(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])))
because when the XFF header contains multiple addresses they are separated by ", ".
Same for D7.

» Login or register to post comments

#1

mr.baileys - February 23, 2009 - 14:50
Version:6.x-dev» 7.x-dev

No RFC, but according to wikipedia:

The general format of the header is:

X-Forwarded-For: client1, proxy1, proxy2

where the value is a comma+space separated list of IP addresses, the left-most being the farthest downstream client, and each successive proxy that passed the request adding the IP address where it received the request from. In this example, the request passed proxy1, proxy2 and proxy3 (proxy3 appears as remote address of the request).

So it indeed looks as if we should either explode using ', ' instead of ',' or wrap the array_pop in trim as per your suggestion.

Login or register to post comments

#2

mr.baileys - March 16, 2009 - 08:48
Status:active» needs review

Patch.

AttachmentSize
309586_XFF_spaces.patch 685 bytes
Testbed results
309586_XFF_spaces.patchre-testing
Login or register to post comments

#3

Dries - March 17, 2009 - 15:31

Patch looks good. Maybe extend the code comment a bit? That wouldn't hurt.

Login or register to post comments

#4

mr.baileys - March 18, 2009 - 07:44

Rewrote the comment to provide some extra documentation/clarification.

AttachmentSize
309586_XFF_spaces.patch 1.19 KB
Testbed results
309586_XFF_spaces.patchpassedPassed: 10619 passes, 0 fails, 0 exceptions Detailed results
Login or register to post comments

#5

Dries - March 18, 2009 - 09:21
Status:needs review» fixed

Committed to CVS HEAD. Thanks.

Login or register to post comments

#6

System Message - April 1, 2009 - 09:30
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

Login or register to post comments
 
 

Drupal is a registered trademark of Dries Buytaert.