From ff6d4bc0fdf27a9d7c0ef86f642d19722404c080 Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Sat, 28 Mar 2026 12:41:18 +0000 Subject: [PATCH] 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 | 5 +++-- test/unit/lib/webhook_endpoint_validator_test.rb | 14 +++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/webhook_endpoint_validator.rb b/lib/webhook_endpoint_validator.rb index 25e893be8..4c0750d33 100644 --- a/lib/webhook_endpoint_validator.rb +++ b/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 diff --git a/test/unit/lib/webhook_endpoint_validator_test.rb b/test/unit/lib/webhook_endpoint_validator_test.rb index 6de386819..f22876ee1 100644 --- a/test/unit/lib/webhook_endpoint_validator_test.rb +++ b/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