Do not trust invalid ip addresses given in request headers

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)

lib/geocoder/ip_address.rb

+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

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
 
     ##

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

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