summary refs log tree commit
diff options
context:
space:
mode:
authorSteve Hodgkiss <steve@hodgkiss.me>2014-06-24 09:44:31 +1000
committerSteve Hodgkiss <steve@hodgkiss.me>2014-07-11 10:14:13 +1000
commit7a8efc2a270f363b85ce610ad897184d80b7a1d6 (patch)
tree814d970f42ef3c788968e9b74e1a05becff93813
parent64a286226941a14f68fb900e96841ccdd0abe0f7 (diff)
downloadrack-7a8efc2a270f363b85ce610ad897184d80b7a1d6.tar.gz
Prevent IP spoofing via X-Forwarded-For and Client-IP headers
By specifying the same IP in X-Forwarded-For and Client-IP an attacker is able to easily spoof the value of `request.ip`, unless a trusted proxy is configured to remove any user supplied Client-IP headers. The value of request.ip should be the value after the last trusted proxy IP in X-Forwarded-For (from right to left).
-rw-r--r--lib/rack/request.rb6
-rw-r--r--test/spec_request.rb24
2 files changed, 18 insertions, 12 deletions
diff --git a/lib/rack/request.rb b/lib/rack/request.rb
index 07627ddb..cc183b44 100644
--- a/lib/rack/request.rb
+++ b/lib/rack/request.rb
@@ -353,12 +353,6 @@ module Rack
 
       forwarded_ips = split_ip_addresses(@env['HTTP_X_FORWARDED_FOR'])
 
-      if client_ip = @env['HTTP_CLIENT_IP']
-        # If forwarded_ips doesn't include the client_ip, it might be an
-        # ip spoofing attempt, so we ignore HTTP_CLIENT_IP
-        return client_ip if forwarded_ips.include?(client_ip)
-      end
-
       return reject_trusted_ip_addresses(forwarded_ips).last || @env["REMOTE_ADDR"]
     end
 
diff --git a/test/spec_request.rb b/test/spec_request.rb
index bd67ce63..c83b0526 100644
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -1037,12 +1037,6 @@ EOF
       'HTTP_CLIENT_IP' => '1.1.1.1'
     res.body.should.equal '1.1.1.1'
 
-    # Spoofing attempt
-    res = mock.get '/',
-      'HTTP_X_FORWARDED_FOR' => '1.1.1.1',
-      'HTTP_CLIENT_IP' => '2.2.2.2'
-    res.body.should.equal '1.1.1.1'
-
     res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '8.8.8.8, 9.9.9.9'
     res.body.should.equal '9.9.9.9'
 
@@ -1061,6 +1055,24 @@ EOF
     res.body.should.equal '3.4.5.6'
   end
 
+  should "not allow IP spoofing via Client-IP and X-Forwarded-For headers" do
+    mock = Rack::MockRequest.new(Rack::Lint.new(ip_app))
+
+    # IP Spoofing attempt:
+    # Client sends          X-Forwarded-For: 6.6.6.6
+    #                       Client-IP: 6.6.6.6
+    # Load balancer adds    X-Forwarded-For: 2.2.2.3, 192.168.0.7
+    # App receives:         X-Forwarded-For: 6.6.6.6
+    #                       X-Forwarded-For: 2.2.2.3, 192.168.0.7
+    #                       Client-IP: 6.6.6.6
+    # Rack env:             HTTP_X_FORWARDED_FOR: '6.6.6.6, 2.2.2.3, 192.168.0.7'
+    #                       HTTP_CLIENT_IP: '6.6.6.6'
+    res = mock.get '/',
+      'HTTP_X_FORWARDED_FOR' => '6.6.6.6, 2.2.2.3, 192.168.0.7',
+      'HTTP_CLIENT_IP' => '6.6.6.6'
+    res.body.should.equal '2.2.2.3'
+  end
+
   should "regard local addresses as proxies" do
     req = Rack::Request.new(Rack::MockRequest.env_for("/"))
     req.trusted_proxy?('127.0.0.1').should.equal 0