Strictly validate exotic IP formats in webhook URLs. As a side-effect of this, we now reject URLs where we can't resolve the host (#43911, #29664).

Patch by Holger Just (user:hjust).


git-svn-id: https://svn.redmine.org/redmine/trunk@24540 e93f8b46-1217-0410-a6f0-8f06a7374b81

lib/webhook_endpoint_validator.rb

@@ -84,8 +84,9 @@ class WebhookEndpointValidator < ActiveModel::EachValidator
 
     return false if blocked_hosts[:host]&.match?(host)
 
-    Resolv.each_address(host) do |ip|
-      ipaddr = IPAddr.new(ip)
+    Addrinfo.foreach(host, nil, nil, :STREAM) do |addrinfo|
+      return false unless addrinfo.ip?
+      ipaddr = IPAddr.new(addrinfo.ip_address)
       return false if ipaddr.to_i.zero? # 0.0.0.0 and ::
       return false if ipaddr.link_local? || ipaddr.loopback?
       return false if IPAddr.new('224.0.0.0/24').include?(ipaddr) # multicast

test/unit/lib/webhook_endpoint_validator_test.rb

@@ -46,6 +46,7 @@ class WebhookEndpointValidatorTest < ActiveSupport::TestCase
         file://example.com
         https://x.example.org/
         http://x.example.org/
+        http://missinghost.invalid
       ].each do |url|
         assert_not WebhookEndpointValidator.safe_webhook_uri?(url), "#{url} should be invalid"
         record = TestModel.new url
@@ -53,8 +54,8 @@ class WebhookEndpointValidatorTest < ActiveSupport::TestCase
         assert record.errors[:url].any?
       end
 
-      assert WebhookEndpointValidator.safe_webhook_uri? 'https://acme.com/some/webhook?foo=bar'
-      record = TestModel.new 'https://acme.com/some/webhook?foo=bar'
+      assert WebhookEndpointValidator.safe_webhook_uri? 'https://example.com/some/webhook?foo=bar'
+      record = TestModel.new 'https://example.com/some/webhook?foo=bar'
       assert record.valid?, record.errors.inspect
     end
   end
@@ -66,13 +67,14 @@ class WebhookEndpointValidatorTest < ActiveSupport::TestCase
     ].each do |url|
       assert_not WebhookEndpointValidator.safe_webhook_uri?(url), "#{url} should be invalid"
     end
+
     %w[
       http://example.com
       http://example.com:80
       http://example.com:443
       http://example.com:8080
     ].each do |url|
-      assert WebhookEndpointValidator.safe_webhook_uri? url
+      assert WebhookEndpointValidator.safe_webhook_uri?(url), "#{url} should be valid"
     end
   end
 
@@ -81,6 +83,12 @@ class WebhookEndpointValidatorTest < ActiveSupport::TestCase
       %w[
         127.0.0.0
         127.0.0.1
+        2130706433
+        0177.0.1
+        0x7f000001
+        127.0.0.01
+        127.1
+
         10.0.0.0
         10.0.1.0
         169.254.1.9