Skip to content

Commit

Permalink
Change ActionDispatch::RemoteIp to not use HTTP_X_FORWARDED_FOR w…
Browse files Browse the repository at this point in the history
…hen `REMOTE_ADDR` is not a trusted proxy

As pointed out in the documentation, using `RemoteIp` while accessible without a reverse proxy opens you to IP
spoofing. Returning `REMOTE_ADDR` when that is not considered a trusted proxy should make that less likely.
  • Loading branch information
ClearlyClaire committed Apr 19, 2024
1 parent adf0c73 commit 30e14eb
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/middleware/remote_ip.rb
Expand Up @@ -164,7 +164,7 @@ def calculate_ip

# If every single IP option is in the trusted list, return the IP that's
# furthest away
filter_proxies(ips + [remote_addr]).first || ips.last || remote_addr
filter_proxies([remote_addr] + ips).first || ips.last || remote_addr
end

# Memoizes the value returned by #calculate_ip and returns it for
Expand All @@ -191,7 +191,7 @@ def ips_from(header) # :doc:

def filter_proxies(ips) # :doc:
ips.reject do |ip|
@proxies.any? { |proxy| proxy === ip }
ip.blank? || @proxies.any? { |proxy| proxy === ip }
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/dispatch/request_test.rb
Expand Up @@ -74,7 +74,7 @@ class RequestIP < BaseRequestTest

request = stub_request "REMOTE_ADDR" => "1.2.3.4",
"HTTP_X_FORWARDED_FOR" => "3.4.5.6"
assert_equal "3.4.5.6", request.remote_ip
assert_equal "1.2.3.4", request.remote_ip

request = stub_request "REMOTE_ADDR" => "127.0.0.1",
"HTTP_X_FORWARDED_FOR" => "3.4.5.6"
Expand Down Expand Up @@ -155,7 +155,7 @@ class RequestIP < BaseRequestTest

request = stub_request "REMOTE_ADDR" => "2001:0db8:85a3:0000:0000:8a2e:0370:7334",
"HTTP_X_FORWARDED_FOR" => "fe80:0000:0000:0000:0202:b3ff:fe1e:8329"
assert_equal "fe80:0000:0000:0000:0202:b3ff:fe1e:8329", request.remote_ip
assert_equal "2001:0db8:85a3:0000:0000:8a2e:0370:7334", request.remote_ip

request = stub_request "REMOTE_ADDR" => "::1",
"HTTP_X_FORWARDED_FOR" => "fe80:0000:0000:0000:0202:b3ff:fe1e:8329"
Expand Down
9 changes: 7 additions & 2 deletions railties/test/application/middleware/remote_ip_test.rb
Expand Up @@ -60,7 +60,12 @@ def remote_ip(env = {})

test "remote_ip works with HTTP_X_FORWARDED_FOR" do
make_basic_app
assert_equal "4.2.42.42", remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "4.2.42.42")
assert_equal "4.2.42.42", remote_ip("REMOTE_ADDR" => "10.0.1.1", "HTTP_X_FORWARDED_FOR" => "4.2.42.42")
end

test "remote_ip does not trust HTTP_X_FORWARDED_FOR for untrusted REMOTE_ADDR" do
make_basic_app
assert_equal "4.2.42.1", remote_ip("REMOTE_ADDR" => "4.2.42.1", "HTTP_X_FORWARDED_FOR" => "4.2.42.42")
end

test "the user can set trusted proxies with an enumerable of case equality comparable values" do
Expand All @@ -80,7 +85,7 @@ def remote_ip(env = {})
end

assert_equal "10.0.0.0",
remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "10.0.0.0,4.2.42.42")
remote_ip("REMOTE_ADDR" => "4.2.42.1", "HTTP_X_FORWARDED_FOR" => "10.0.0.0,4.2.42.42")
end

test "setting trusted proxies to a single value (deprecated) adds value to default trusted proxies" do
Expand Down

0 comments on commit 30e14eb

Please sign in to comment.