Better validations against internal URLs

lib/validate_url.rb

@@ -1,16 +1,33 @@
 require 'active_model'
 require 'active_support/i18n'
+require 'ipaddr'
+require 'resolv'
 I18n.load_path += Dir[File.dirname(__FILE__) + "/locale/*.yml"]
 
 module ActiveModel
   module Validations
     class UrlValidator < ActiveModel::EachValidator
-      RESERVED_OPTIONS = [:schemes, :no_local]
+      RESERVED_OPTIONS = [:schemes, :no_local, :blacklisted_domains]
+
+      BLACKLISTED_INTERNAL_IPS = [
+        IPAddr.new('10.0.0.0/8'),
+        IPAddr.new('127.0.0.0/8'),
+        IPAddr.new('192.168.0.0/16'),
+        IPAddr.new('169.254.0.0/16'),
+        IPAddr.new('224.0.0.0/4'),
+        IPAddr.new('0.0.0.0/8'),
+        IPAddr.new('255.255.255.255'),
+        IPAddr.new('::1'),
+        IPAddr.new('fc00::/7'),
+        IPAddr.new('fd00::/8'),
+        IPAddr.new('fe80::/10')
+      ].freeze
 
       def initialize(options)
         options.reverse_merge!(:schemes => %w(http https))
         options.reverse_merge!(:message => :url)
         options.reverse_merge!(:no_local => false)
+        options.reverse_merge!(:blacklisted_domains => [])
 
         super(options)
       end
@@ -19,14 +36,56 @@ def validate_each(record, attribute, value)
         schemes = [*options.fetch(:schemes)].map(&:to_s)
         begin
           uri = URI.parse(value)
-          unless uri && uri.host && schemes.include?(uri.scheme) && (!options.fetch(:no_local) || uri.host.include?('.'))
+          hostname = uri.host || uri.to_s
+
+          unless uri && uri.host && schemes.include?(uri.scheme)
+            record.errors.add(attribute, :url, filtered_options(value))
+            return
+          end
+
+          if options.fetch(:no_local) && self.class.local?(hostname)
             record.errors.add(attribute, :url, filtered_options(value))
+            return
+          end
+
+          if options.fetch(:blacklisted_domains).any? && blacklisted_domain?(hostname)
+            record.errors.add(attribute, :url, filtered_options(value))
+            return
           end
         rescue URI::InvalidURIError
           record.errors.add(attribute, :url, filtered_options(value))
         end
       end
 
+      def blacklisted_domain?(hostname)
+        blacklisted_domains_regex = Regexp.new(
+          options[:blacklisted_domains].map do |blacklisted_domain|
+            %r{^([\w-]+\.)?#{Regexp.escape(blacklisted_domain)}}
+          end.join('|'),
+          Regexp::IGNORECASE
+        )
+
+        (hostname =~ blacklisted_domains_regex).present?
+      end
+
+      def self.local?(hostname)
+        ip = begin
+               Resolv.getaddress(hostname)
+             rescue Resolv::ResolvError
+               nil
+             end
+
+        return false unless ip
+
+        ip_addr = IPAddr.new(ip)
+
+        BLACKLISTED_INTERNAL_IPS.any? do |blacklisted_ip|
+          # note 1: explicit usage of triple equals operator
+          # note 2: a === b is not equal to b === a when dealing with IPAddr
+          blacklisted_ip === ip_addr
+        end
+      end
+
       protected
 
       def filtered_options(value)

spec/resources/user_with_blacklisted_domains.rb

@@ -0,0 +1,9 @@
+require 'active_model/validations'
+
+class UserWithBlacklistedDomains
+  include ActiveModel::Validations
+
+  attr_accessor :homepage
+
+  validates :homepage, :url => {:blacklisted_domains => ['example.net']}
+end

spec/spec_helper.rb

@@ -16,12 +16,13 @@
 
 require File.join(File.dirname(__FILE__), '..', 'init')
 
-autoload :User,                  'resources/user'
-autoload :UserWithNil,           'resources/user_with_nil'
-autoload :UserWithBlank,         'resources/user_with_blank'
-autoload :UserWithLegacySyntax,  'resources/user_with_legacy_syntax'
-autoload :UserWithAr,            'resources/user_with_ar'
-autoload :UserWithArLegacy,      'resources/user_with_ar_legacy'
-autoload :UserWithCustomScheme,  'resources/user_with_custom_scheme'
-autoload :UserWithCustomMessage, 'resources/user_with_custom_message'
-autoload :UserWithNoLocal,       'resources/user_with_no_local'
+autoload :User,                       'resources/user'
+autoload :UserWithNil,                'resources/user_with_nil'
+autoload :UserWithBlank,              'resources/user_with_blank'
+autoload :UserWithLegacySyntax,       'resources/user_with_legacy_syntax'
+autoload :UserWithAr,                 'resources/user_with_ar'
+autoload :UserWithArLegacy,           'resources/user_with_ar_legacy'
+autoload :UserWithCustomScheme,       'resources/user_with_custom_scheme'
+autoload :UserWithCustomMessage,      'resources/user_with_custom_message'
+autoload :UserWithNoLocal,            'resources/user_with_no_local'
+autoload :UserWithBlacklistedDomains, 'resources/user_with_blacklisted_domains'

spec/validate_url_spec.rb

@@ -158,8 +158,40 @@
       expect(@user).not_to be_valid
     end
 
-    it "should not allow weird urls that get interpreted as local hostnames" do
-      @user.homepage = "http://http://example.com"
+    [
+      'https://127.0.0.1',
+      'https://192.168.1.13',
+      'https://127.0.254.254',
+      'https://10.100.103.243',
+      'https://127.0.0.1:5555',
+      'https://127.0.0.1.xip.io',
+      'https://169.254.1.1',
+      'https://0:0:0:0:0:ffff:7f00:1'
+    ].each do |url|
+      it "should not allow #{url}" do
+        @user.homepage = url
+        expect(@user).not_to be_valid
+      end
+    end
+  end
+
+  context "with blacklisted_domains" do
+    before do
+      @user = UserWithBlacklistedDomains.new
+    end
+
+    it "should allow a valid internet url" do
+      @user.homepage = "http://www.example.com"
+      expect(@user).to be_valid
+    end
+
+    it "should not allow a blacklisted domain" do
+      @user.homepage = "http://example.net"
+      expect(@user).not_to be_valid
+    end
+
+    it "should not allow a blacklisted subdomain" do
+      @user.homepage = "http://sub.example.net"
       expect(@user).not_to be_valid
     end
   end