Merge pull request #12410 from tamird/fix-ip-spoof-errors

Fix ip spoof errors

3 files changed, 19 insertions, 1 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 906592874d..31136d91b3 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,13 @@
 ## unreleased ##
 
+*   Fix `ActionDispatch::RemoteIp::GetIp#calculate_ip` to only check for spoofing
+    attacks if both `HTTP_CLIENT_IP` and `HTTP_X_FORWARDED_FOR` are set.
+
+    Fixes #12410
+    Backports #10844
+
+    *Tamir Duberstein*
+
 *   Fix the assert_recognizes test method so that it works when there are
     constraints on the querystring.
 
diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
index 66ece60860..f4545bd95e 100644
--- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb
+++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
@@ -49,7 +49,7 @@ module ActionDispatch
         forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR')
         remote_addrs  = ips_from('REMOTE_ADDR')
 
-        check_ip = client_ip && @middleware.check_ip
+        check_ip = client_ip && forwarded_ips.present? && @middleware.check_ip
         if check_ip && !forwarded_ips.include?(client_ip)
           # We don't know which came from the proxy, and which from the user
           raise IpSpoofAttackError, "IP spoofing attack?!" \
diff --git a/railties/test/application/middleware/remote_ip_test.rb b/railties/test/application/middleware/remote_ip_test.rb
index da291f061c..126f4832c4 100644
--- a/railties/test/application/middleware/remote_ip_test.rb
+++ b/railties/test/application/middleware/remote_ip_test.rb
@@ -46,6 +46,16 @@ module ApplicationTests
       end
     end
 
+    test "works with both headers individually" do
+      make_basic_app
+      assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+        assert_equal "1.1.1.1", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1")
+      end
+      assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+        assert_equal "1.1.1.2", remote_ip("HTTP_CLIENT_IP" => "1.1.1.2")
+      end
+    end
+
     test "can disable IP spoofing check" do
       make_basic_app do |app|
         app.config.action_dispatch.ip_spoofing_check = false