0
@@ -1,5 +1,7 @@
0
 *SVN*
0
 
0
+* Avoid remote_ip spoofing.  [Brian Candler]
0
+
0
 * Correct inconsistencies in RequestForgeryProtection docs.  #11032 [mislav]
0
 
0
 * Make assert_routing aware of the HTTP method used.  #8039 [mpalmer]

0
@@ -122,26 +122,41 @@ module ActionController
0
     end
0
     alias xhr? :xml_http_request?
0
 
0
+    # Which IP addresses are "trusted proxies" that can be stripped from
0
+    # the right-hand-side of X-Forwarded-For
0
+    TRUSTED_PROXIES = /^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\./i
0
+
0
     # Determine originating IP address.  REMOTE_ADDR is the standard
0
     # but will fail if the user is behind a proxy.  HTTP_CLIENT_IP and/or
0
-    # HTTP_X_FORWARDED_FOR are set by proxies so check for these before
0
-    # falling back to REMOTE_ADDR.  HTTP_X_FORWARDED_FOR may be a comma-
0
-    # delimited list in the case of multiple chained proxies; the first is
0
-    # the originating IP.
0
-    #
0
-    # Security note: do not use if IP spoofing is a concern for your
0
-    # application. Since remote_ip checks HTTP headers for addresses forwarded
0
-    # by proxies, the client may send any IP. remote_addr can't be spoofed but
0
-    # also doesn't work behind a proxy, since it's always the proxy's IP.
0
+    # HTTP_X_FORWARDED_FOR are set by proxies so check for these if
0
+    # REMOTE_ADDR is a proxy.  HTTP_X_FORWARDED_FOR may be a comma-
0
+    # delimited list in the case of multiple chained proxies; the last
0
+    # address which is not trusted is the originating IP.
0
+
0
     def remote_ip
0
-      return @env['HTTP_CLIENT_IP'] if @env.include? 'HTTP_CLIENT_IP'
0
+      if TRUSTED_PROXIES !~ @env['REMOTE_ADDR']
0
+        return @env['REMOTE_ADDR']
0
+      end
0
+
0
+      if @env.include? 'HTTP_CLIENT_IP'
0
+        if @env.include? 'HTTP_X_FORWARDED_FOR'
0
+          # We don't know which came from the proxy, and which from the user
0
+          raise ActionControllerError.new(<<EOM)
0
+IP spoofing attack?!
0
+HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}
0
+HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}
0
+EOM
0
+        end
0
+        return @env['HTTP_CLIENT_IP']
0
+      end
0
 
0
       if @env.include? 'HTTP_X_FORWARDED_FOR' then
0
-        remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',').reject do |ip|
0
-          ip.strip =~ /^unknown$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\./i
0
+        remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',')
0
+        while remote_ips.size > 1 && TRUSTED_PROXIES =~ remote_ips.last.strip
0
+          remote_ips.pop
0
         end
0
 
0
-        return remote_ips.first.strip unless remote_ips.empty?
0
+        return remote_ips.last.strip
0
       end
0
 
0
       @env['REMOTE_ADDR']

0
@@ -13,9 +13,17 @@ class RequestTest < Test::Unit::TestCase
0
     assert_equal '1.2.3.4', @request.remote_ip
0
 
0
     @request.env['HTTP_CLIENT_IP'] = '2.3.4.5'
0
+    assert_equal '1.2.3.4', @request.remote_ip
0
+
0
+    @request.remote_addr = '192.168.0.1'
0
     assert_equal '2.3.4.5', @request.remote_ip
0
     @request.env.delete 'HTTP_CLIENT_IP'
0
 
0
+    @request.remote_addr = '1.2.3.4'
0
+    @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6'
0
+    assert_equal '1.2.3.4', @request.remote_ip
0
+
0
+    @request.remote_addr = '127.0.0.1'
0
     @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6'
0
     assert_equal '3.4.5.6', @request.remote_ip
0
 
0
@@ -35,10 +43,23 @@ class RequestTest < Test::Unit::TestCase
0
     assert_equal '3.4.5.6', @request.remote_ip
0
 
0
     @request.env['HTTP_X_FORWARDED_FOR'] = '127.0.0.1,3.4.5.6'
0
-    assert_equal '127.0.0.1', @request.remote_ip
0
+    assert_equal '3.4.5.6', @request.remote_ip
0
 
0
     @request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,192.168.0.1'
0
-    assert_equal '1.2.3.4', @request.remote_ip
0
+    assert_equal 'unknown', @request.remote_ip
0
+
0
+    @request.env['HTTP_X_FORWARDED_FOR'] = '9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4'
0
+    assert_equal '3.4.5.6', @request.remote_ip
0
+
0
+    @request.env['HTTP_CLIENT_IP'] = '8.8.8.8'
0
+    e = assert_raises(ActionController::ActionControllerError) {
0
+      @request.remote_ip
0
+    }
0
+    assert_match /IP spoofing attack/, e.message
0
+    assert_match /HTTP_X_FORWARDED_FOR="9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4"/, e.message
0
+    assert_match /HTTP_CLIENT_IP="8.8.8.8"/, e.message
0
+
0
+    @request.env.delete 'HTTP_CLIENT_IP'
0
     @request.env.delete 'HTTP_X_FORWARDED_FOR'
0
   end
0