<?xml version="1.0" encoding="UTF-8"?>
<commit>
  <added type="array"/>
  <modified type="array">
    <modified>
      <diff>@@ -1,5 +1,7 @@
 *SVN*
 
+* Avoid remote_ip spoofing.  [Brian Candler]
+
 * Correct inconsistencies in RequestForgeryProtection docs.  #11032 [mislav]
 
 * Make assert_routing aware of the HTTP method used.  #8039 [mpalmer]</diff>
      <filename>actionpack/CHANGELOG</filename>
    </modified>
    <modified>
      <diff>@@ -122,26 +122,41 @@ module ActionController
     end
     alias xhr? :xml_http_request?
 
+    # Which IP addresses are &quot;trusted proxies&quot; that can be stripped from
+    # the right-hand-side of X-Forwarded-For
+    TRUSTED_PROXIES = /^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\./i
+
     # Determine originating IP address.  REMOTE_ADDR is the standard
     # but will fail if the user is behind a proxy.  HTTP_CLIENT_IP and/or
-    # HTTP_X_FORWARDED_FOR are set by proxies so check for these before
-    # falling back to REMOTE_ADDR.  HTTP_X_FORWARDED_FOR may be a comma-
-    # delimited list in the case of multiple chained proxies; the first is
-    # the originating IP.
-    #
-    # Security note: do not use if IP spoofing is a concern for your
-    # application. Since remote_ip checks HTTP headers for addresses forwarded
-    # by proxies, the client may send any IP. remote_addr can't be spoofed but
-    # also doesn't work behind a proxy, since it's always the proxy's IP.
+    # HTTP_X_FORWARDED_FOR are set by proxies so check for these if
+    # 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
-      return @env['HTTP_CLIENT_IP'] if @env.include? 'HTTP_CLIENT_IP'
+      if TRUSTED_PROXIES !~ @env['REMOTE_ADDR']
+        return @env['REMOTE_ADDR']
+      end
+
+      if @env.include? 'HTTP_CLIENT_IP'
+        if @env.include? 'HTTP_X_FORWARDED_FOR'
+          # We don't know which came from the proxy, and which from the user
+          raise ActionControllerError.new(&lt;&lt;EOM)
+IP spoofing attack?!
+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(',').reject do |ip|
-          ip.strip =~ /^unknown$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\./i
+        remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',')
+        while remote_ips.size &gt; 1 &amp;&amp; TRUSTED_PROXIES =~ remote_ips.last.strip
+          remote_ips.pop
         end
 
-        return remote_ips.first.strip unless remote_ips.empty?
+        return remote_ips.last.strip
       end
 
       @env['REMOTE_ADDR']</diff>
      <filename>actionpack/lib/action_controller/request.rb</filename>
    </modified>
    <modified>
      <diff>@@ -13,9 +13,17 @@ class RequestTest &lt; Test::Unit::TestCase
     assert_equal '1.2.3.4', @request.remote_ip
 
     @request.env['HTTP_CLIENT_IP'] = '2.3.4.5'
+    assert_equal '1.2.3.4', @request.remote_ip
+
+    @request.remote_addr = '192.168.0.1'
     assert_equal '2.3.4.5', @request.remote_ip
     @request.env.delete 'HTTP_CLIENT_IP'
 
+    @request.remote_addr = '1.2.3.4'
+    @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6'
+    assert_equal '1.2.3.4', @request.remote_ip
+
+    @request.remote_addr = '127.0.0.1'
     @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6'
     assert_equal '3.4.5.6', @request.remote_ip
 
@@ -35,10 +43,23 @@ class RequestTest &lt; Test::Unit::TestCase
     assert_equal '3.4.5.6', @request.remote_ip
 
     @request.env['HTTP_X_FORWARDED_FOR'] = '127.0.0.1,3.4.5.6'
-    assert_equal '127.0.0.1', @request.remote_ip
+    assert_equal '3.4.5.6', @request.remote_ip
 
     @request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,192.168.0.1'
-    assert_equal '1.2.3.4', @request.remote_ip
+    assert_equal 'unknown', @request.remote_ip
+
+    @request.env['HTTP_X_FORWARDED_FOR'] = '9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4'
+    assert_equal '3.4.5.6', @request.remote_ip
+
+    @request.env['HTTP_CLIENT_IP'] = '8.8.8.8'
+    e = assert_raises(ActionController::ActionControllerError) {
+      @request.remote_ip
+    }
+    assert_match /IP spoofing attack/, e.message
+    assert_match /HTTP_X_FORWARDED_FOR=&quot;9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4&quot;/, e.message
+    assert_match /HTTP_CLIENT_IP=&quot;8.8.8.8&quot;/, e.message
+
+    @request.env.delete 'HTTP_CLIENT_IP'
     @request.env.delete 'HTTP_X_FORWARDED_FOR'
   end
 </diff>
      <filename>actionpack/test/controller/request_test.rb</filename>
    </modified>
  </modified>
  <removed type="array"/>
  <parents type="array">
    <parent>
      <id>1c207dfbfcd799074309ef926ac0025b73a123e3</id>
    </parent>
  </parents>
  <author>
    <name>Jeremy Kemper</name>
    <email>jeremy@bitsweat.net</email>
  </author>
  <url>http://github.com/jdelStrother/rails/commit/2c96f509a8e630bcd0a79916ae7aadfe919b49a5</url>
  <id>2c96f509a8e630bcd0a79916ae7aadfe919b49a5</id>
  <committed-date>2008-03-28T14:50:12-07:00</committed-date>
  <authored-date>2008-03-28T14:50:12-07:00</authored-date>
  <message>Merge [9124] from trunk: Avoid remote_ip spoofing.

git-svn-id: http://svn-commit.rubyonrails.org/rails/branches/2-0-stable@9125 5ecf4fe2-1ee6-0310-87b1-e25e094e27de</message>
  <tree>638f4965eef093665a29114c1408e816ac20ef45</tree>
  <committer>
    <name>Jeremy Kemper</name>
    <email>jeremy@bitsweat.net</email>
  </committer>
</commit>
