Fixed Request#remote_ip to only raise hell if the HTTP_CLIENT_IP and HTTP_X_FORWARDED_FOR doesnt match (not just if theyre both present) [Mark Imbriaco, Bradford Folkens]

3 files changed, 12 insertions, 4 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index ba2b16849c..861597701c 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,3 +1,7 @@
+*Edge*
+
+* Fixed Request#remote_ip to only raise hell if the HTTP_CLIENT_IP and HTTP_X_FORWARDED_FOR doesn't match (not just if they're both present) [Mark Imbriaco, Bradford Folkens]
+
 * Allow caches_action to accept a layout option [José Valim]
 
 * Added Rack processor [Ezra Zygmuntowicz, Josh Peek]
diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb
index a35b904194..9b02f2c8a1 100755
--- a/actionpack/lib/action_controller/request.rb
+++ b/actionpack/lib/action_controller/request.rb
@@ -134,14 +134,15 @@ module ActionController
     # REMOTE_ADDR is a proxy.  HTTP_X_FORWARDED_FOR may be a comma-
     # delimited list in the case of multiple chained proxies; the last
     # address which is not trusted is the originating IP.
-
     def remote_ip
       if TRUSTED_PROXIES !~ @env['REMOTE_ADDR']
         return @env['REMOTE_ADDR']
       end
 
+      remote_ips = @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_X_FORWARDED_FOR'].split(',')
+
       if @env.include? 'HTTP_CLIENT_IP'
-        if @env.include? 'HTTP_X_FORWARDED_FOR'
+        if remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP'])
           # We don't know which came from the proxy, and which from the user
           raise ActionControllerError.new(<<EOM)
 IP spoofing attack?!
@@ -149,11 +150,11 @@ HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}
 HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}
 EOM
         end
+
         return @env['HTTP_CLIENT_IP']
       end
 
-      if @env.include? 'HTTP_X_FORWARDED_FOR' then
-        remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',')
+      if remote_ips
         while remote_ips.size > 1 && TRUSTED_PROXIES =~ remote_ips.last.strip
           remote_ips.pop
         end
diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb
index 82ddfec8e8..2bd489b2c7 100644
--- a/actionpack/test/controller/request_test.rb
+++ b/actionpack/test/controller/request_test.rb
@@ -59,6 +59,9 @@ class RequestTest < Test::Unit::TestCase
     assert_match /HTTP_X_FORWARDED_FOR="9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4"/, e.message
     assert_match /HTTP_CLIENT_IP="8.8.8.8"/, e.message
 
+    @request.env['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 9.9.9.9'
+    assert_equal '8.8.8.8', @request.remote_ip
+
     @request.env.delete 'HTTP_CLIENT_IP'
     @request.env.delete 'HTTP_X_FORWARDED_FOR'
   end