From 0184d278e4eaa1465fb0dcce60a48548b7578b40 Mon Sep 17 00:00:00 2001 From: Roger Campos <roger@itnig.net> Date: Sat, 31 Aug 2013 18:08:30 +0200 Subject: [PATCH] Do not trust invalid ip addresses given in request headers --- lib/geocoder.rb | 1 + lib/geocoder/ip_address.rb | 11 +++++++++++ lib/geocoder/query.rb | 4 ++-- lib/geocoder/request.rb | 15 ++++++--------- test/request_test.rb | 7 ++++++- 5 files changed, 26 insertions(+), 12 deletions(-) create mode 100644 lib/geocoder/ip_address.rb diff --git a/lib/geocoder.rb b/lib/geocoder.rb index 53e9860b..cc5dcffc 100644 --- a/lib/geocoder.rb +++ b/lib/geocoder.rb @@ -5,6 +5,7 @@ require "geocoder/exceptions" require "geocoder/cache" require "geocoder/request" require "geocoder/lookup" +require "geocoder/ip_address" require "geocoder/models/active_record" if defined?(::ActiveRecord) require "geocoder/models/mongoid" if defined?(::Mongoid) require "geocoder/models/mongo_mapper" if defined?(::MongoMapper) diff --git a/lib/geocoder/ip_address.rb b/lib/geocoder/ip_address.rb new file mode 100644 index 00000000..aabdca92 --- /dev/null +++ b/lib/geocoder/ip_address.rb @@ -0,0 +1,11 @@ +module Geocoder + class IpAddress < String + def loopback? + (self == "0.0.0.0" or self.match(/\A127/)) + end + + def valid? + !!self.match(/\A(::ffff:)?(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})\z/) + end + end +end diff --git a/lib/geocoder/query.rb b/lib/geocoder/query.rb index 3c2384e0..d909ffca 100644 --- a/lib/geocoder/query.rb +++ b/lib/geocoder/query.rb @@ -63,14 +63,14 @@ module Geocoder # dot-delimited numbers. # def ip_address? - !!text.to_s.match(/\A(::ffff:)?(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})\z/) + IpAddress.new(text).valid? rescue false end ## # Is the Query text a loopback IP address? # def loopback_ip_address? - !!(self.ip_address? and (text == "0.0.0.0" or text.to_s.match(/\A127/))) + ip_address? && IpAddress.new(text).loopback? end ## diff --git a/lib/geocoder/request.rb b/lib/geocoder/request.rb index f02aab47..b325c84a 100644 --- a/lib/geocoder/request.rb +++ b/lib/geocoder/request.rb @@ -4,16 +4,13 @@ module Geocoder module Request def location - unless defined?(@location) - if env.has_key?('HTTP_X_REAL_IP') - @location = Geocoder.search(env['HTTP_X_REAL_IP']).first - elsif env.has_key?('HTTP_X_FORWARDED_FOR') - @location = Geocoder.search(env['HTTP_X_FORWARDED_FOR'].split(/\s*,\s*/)[0]).first - else - @location = Geocoder.search(ip).first - end + @location ||= begin + detected_ip = env['HTTP_X_REAL_IP'] || + env['HTTP_X_FORWARDED_FOR'] && env['HTTP_X_FORWARDED_FOR'].split(",").first.strip + + real_ip = detected_ip && (detected_ip = IpAddress.new(detected_ip)) && detected_ip.valid? && !detected_ip.loopback? && detected_ip.to_s || self.ip + Geocoder.search(real_ip).first end - @location end end end diff --git a/test/request_test.rb b/test/request_test.rb index 3acbe284..046bca2a 100644 --- a/test/request_test.rb +++ b/test/request_test.rb @@ -26,4 +26,9 @@ class RequestTest < Test::Unit::TestCase req = MockRequest.new({}, "74.200.247.59") assert req.location.is_a?(Geocoder::Result::Freegeoip) end -end \ No newline at end of file + + def test_with_loopback_x_forwarded_for + req = MockRequest.new({"HTTP_X_FORWARDED_FOR" => "127.0.0.1"}, "74.200.247.59") + assert_equal "US", req.location.country_code + end +end -- GitLab