From a61b63d42056b119e061f7ebf31985887d569c79 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 28 Mar 2008 21:38:01 +0000 Subject: Avoid remote_ip spoofing git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9124 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/request.rb | 41 ++++++++++++++++++++--------- actionpack/test/controller/request_test.rb | 25 ++++++++++++++++-- 3 files changed, 53 insertions(+), 15 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 40921b8526..5dab228866 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Avoid remote_ip spoofing. [Brian Candler] + * Added support for regexp flags like ignoring case in the :requirements part of routes declarations #11421 [NeilW] * Fixed that ActionController::Base#read_multipart would fail if boundary was exactly 10240 bytes #10886 [ariejan] diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 4889d296ef..745161def8 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -124,26 +124,41 @@ module ActionController end alias xhr? :xml_http_request? + # Which IP addresses are "trusted proxies" that can be stripped from + # the right-hand-side of X-Forwarded-For + TRUSTED_PROXIES = /^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\./i + # Determine originating IP address. REMOTE_ADDR is the standard # but will fail if the user is behind a proxy. HTTP_CLIENT_IP and/or - # HTTP_X_FORWARDED_FOR are set by proxies so check for these before - # falling back to REMOTE_ADDR. HTTP_X_FORWARDED_FOR may be a comma- - # delimited list in the case of multiple chained proxies; the first is - # the originating IP. - # - # Security note: do not use if IP spoofing is a concern for your - # application. Since remote_ip checks HTTP headers for addresses forwarded - # by proxies, the client may send any IP. remote_addr can't be spoofed but - # also doesn't work behind a proxy, since it's always the proxy's IP. + # HTTP_X_FORWARDED_FOR are set by proxies so check for these if + # REMOTE_ADDR is a proxy. HTTP_X_FORWARDED_FOR may be a comma- + # delimited list in the case of multiple chained proxies; the last + # address which is not trusted is the originating IP. + def remote_ip - return @env['HTTP_CLIENT_IP'] if @env.include? 'HTTP_CLIENT_IP' + if TRUSTED_PROXIES !~ @env['REMOTE_ADDR'] + return @env['REMOTE_ADDR'] + end + + if @env.include? 'HTTP_CLIENT_IP' + if @env.include? 'HTTP_X_FORWARDED_FOR' + # We don't know which came from the proxy, and which from the user + raise ActionControllerError.new(< 1 && TRUSTED_PROXIES =~ remote_ips.last.strip + remote_ips.pop end - return remote_ips.first.strip unless remote_ips.empty? + return remote_ips.last.strip end @env['REMOTE_ADDR'] diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index 2f72f9017a..4d645f56e0 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -13,9 +13,17 @@ class RequestTest < Test::Unit::TestCase assert_equal '1.2.3.4', @request.remote_ip @request.env['HTTP_CLIENT_IP'] = '2.3.4.5' + assert_equal '1.2.3.4', @request.remote_ip + + @request.remote_addr = '192.168.0.1' assert_equal '2.3.4.5', @request.remote_ip @request.env.delete 'HTTP_CLIENT_IP' + @request.remote_addr = '1.2.3.4' + @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6' + assert_equal '1.2.3.4', @request.remote_ip + + @request.remote_addr = '127.0.0.1' @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6' assert_equal '3.4.5.6', @request.remote_ip @@ -35,10 +43,23 @@ class RequestTest < Test::Unit::TestCase assert_equal '3.4.5.6', @request.remote_ip @request.env['HTTP_X_FORWARDED_FOR'] = '127.0.0.1,3.4.5.6' - assert_equal '127.0.0.1', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip @request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,192.168.0.1' - assert_equal '1.2.3.4', @request.remote_ip + assert_equal 'unknown', @request.remote_ip + + @request.env['HTTP_X_FORWARDED_FOR'] = '9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4' + assert_equal '3.4.5.6', @request.remote_ip + + @request.env['HTTP_CLIENT_IP'] = '8.8.8.8' + e = assert_raises(ActionController::ActionControllerError) { + @request.remote_ip + } + assert_match /IP spoofing attack/, e.message + assert_match /HTTP_X_FORWARDED_FOR="9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4"/, e.message + assert_match /HTTP_CLIENT_IP="8.8.8.8"/, e.message + + @request.env.delete 'HTTP_CLIENT_IP' @request.env.delete 'HTTP_X_FORWARDED_FOR' end -- cgit v1.2.3