Skip to content
Browse files

make sure both headers are set before checking for ip spoofing

  • Loading branch information...
1 parent ccd11d5 commit 85106decc41f1695ff6fe54452168237fd0f98d0 @tamird tamird committed Jun 4, 2013
View
8 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.
View
2 actionpack/lib/action_dispatch/middleware/remote_ip.rb
@@ -49,7 +49,7 @@ def calculate_ip
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?!" \
View
10 railties/test/application/middleware/remote_ip_test.rb
@@ -46,6 +46,16 @@ def remote_ip(env = {})
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

0 comments on commit 85106de

Please sign in to comment.
Something went wrong with that request. Please try again.