Download & Extend
Issues

IP address from XFF header contains spaces

Posted by mindgame on September 17, 2008 at 9:44am
Project:Drupal core
Version:7.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:Quick fix, XFF

Issue Summary

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

Comments

#1

Posted by mr.baileys on February 23, 2009 at 2:50pm
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.

#2

Posted by mr.baileys on March 16, 2009 at 8:48am
Status:active» needs review

Patch.

AttachmentSizeStatusTest resultOperations
309586_XFF_spaces.patch685 bytesIdleUnable to apply patch 309586_XFF_spaces.patchView details

#3

Posted by Dries on March 17, 2009 at 3:31pm

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

#4

Posted by mr.baileys on March 18, 2009 at 7:44am

Rewrote the comment to provide some extra documentation/clarification.

AttachmentSizeStatusTest resultOperations
309586_XFF_spaces.patch1.19 KBIdlePassed: 10619 passes, 0 fails, 0 exceptionsView details

#5

Posted by Dries on March 18, 2009 at 9:21am
Status:needs review» fixed

Committed to CVS HEAD. Thanks.

#6

Posted by System Message on April 1, 2009 at 9:30am
Status:fixed» closed (fixed)

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

#7

Posted by teastburn85 on June 7, 2010 at 9:09pm

Why does ip_address() use the rightmost entry from X-Forwarded-For header? Isn't the purpose of ip_address() to get the end-user/client's IP address? The documentation for X-Forwarded-For on wikipedia seems to state that the client's IP is the first mentioned IP (leftmost) and any other servers that touch the request append their IPs to the end of the list (rightmost). Am I misunderstanding how X-Forwarded-For is used in this situation?

Selecting the rightmost IP is breaking our Drupal setup where our servers are behind a CDN that passes the client's IP as the first IP in the X-Forwarded-For header.

Also posted this question on http://drupal.org/node/258397 since they are newest issues I could find.

Thanks in advance

nobody click here