Security: close narrow window of exposure to DNS rebinding

Author: jeremy

app/models/ssrf_protection.rb

@@ -0,0 +1,37 @@
+module SsrfProtection
+  extend self
+
+  DNS_RESOLUTION_TIMEOUT = 2
+
+  DISALLOWED_IP_RANGES = [
+    IPAddr.new("0.0.0.0/8") # Broadcasts
+  ].freeze
+
+  def resolve_public_ip(hostname)
+    ip_addresses = resolve_dns(hostname)
+    public_ips = ip_addresses.reject { |ip| private_address?(ip) }
+    public_ips.first&.to_s
+  end
+
+  def private_address?(ip)
+    ip = IPAddr.new(ip.to_s) unless ip.is_a?(IPAddr)
+    ip.private? || ip.loopback? || ip.link_local? || ip.ipv4_mapped? || in_disallowed_range?(ip)
+  end
+
+  private
+    def resolve_dns(hostname)
+      ip_addresses = []
+
+      Resolv::DNS.open(timeouts: DNS_RESOLUTION_TIMEOUT) do |dns|
+        dns.each_address(hostname) do |ip_address|
+          ip_addresses << IPAddr.new(ip_address.to_s)
+        end
+      end
+
+      ip_addresses
+    end
+
+    def in_disallowed_range?(ip)
+      DISALLOWED_IP_RANGES.any? { |range| range.include?(ip) }
+    end
+end

app/models/webhook/delivery.rb

@@ -2,11 +2,6 @@ class Webhook::Delivery < ApplicationRecord
   STALE_TRESHOLD = 7.days
   USER_AGENT = "fizzy/1.0.0 Webhook"
   ENDPOINT_TIMEOUT = 7.seconds
-  DNS_RESOLUTION_TIMEOUT = 2
-  DISALLOWED_IP_RANGES = [
-    # Broadcasts
-    IPAddr.new("0.0.0.0/8")
-  ].freeze
 
   belongs_to :account, default: -> { webhook.account }
   belongs_to :webhook
@@ -54,14 +49,14 @@ def succeeded?
 
   private
     def perform_request
-      if private_uri?
+      if resolved_ip.nil?
         { error: :private_uri }
       else
         response = http.request(
           Net::HTTP::Post.new(uri, headers).tap { |request| request.body = payload }
         )
 
-      { code: response.code.to_i }
+        { code: response.code.to_i }
       end
     rescue Resolv::ResolvTimeout, Resolv::ResolvError, SocketError
       { error: :dns_lookup_failed }
@@ -73,26 +68,17 @@ def perform_request
       { error: :failed_tls }
     end
 
-    def private_uri?
-      ip_addresses = []
-
-      Resolv::DNS.open(timeouts: DNS_RESOLUTION_TIMEOUT) do |dns|
-        dns.each_address(uri.host) do |ip_address|
-          ip_addresses << IPAddr.new(ip_address)
-        end
-      end
-
-      ip_addresses.any? do |ip|
-        ip.private? || ip.loopback? || ip.link_local? || ip.ipv4_mapped? || DISALLOWED_IP_RANGES.any? { |range| range.include?(ip) }
-      end
+    def resolved_ip
+      return @resolved_ip if defined?(@resolved_ip)
+      @resolved_ip = SsrfProtection.resolve_public_ip(uri.host)
     end
 
     def uri
       @uri ||= URI(webhook.url)
     end
 
     def http
-      Net::HTTP.new(uri.host, uri.port).tap do |http|
+      Net::HTTP.new(uri.host, uri.port, ipaddr: resolved_ip).tap do |http|
         http.use_ssl = (uri.scheme == "https")
         http.open_timeout = ENDPOINT_TIMEOUT
         http.read_timeout = ENDPOINT_TIMEOUT

test/models/webhook/delivery_test.rb

@@ -1,6 +1,12 @@
 require "test_helper"
 
 class Webhook::DeliveryTest < ActiveSupport::TestCase
+  PUBLIC_TEST_IP = "93.184.216.34" # example.com's real IP, used as a public IP stand-in
+
+  setup do
+    stub_dns_resolution(PUBLIC_TEST_IP)
+  end
+
   test "create" do
     webhook = webhooks(:active)
     event = events(:layout_commented)
@@ -258,4 +264,55 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase
 
     assert_requested request_stub
   end
+
+  test "blocks DNS rebinding attack where hostname resolves to private IP after validation" do
+    webhook = Webhook.create!(
+      board: boards(:writebook),
+      name: "Rebind Attack",
+      url: "https://rebind.attacker.example/webhook"
+    )
+    event = events(:layout_commented)
+    delivery = Webhook::Delivery.create!(webhook: webhook, event: event)
+
+    # Stub DNS to return a private IP (simulating rebind to internal host)
+    stub_dns_resolution("169.254.169.254") # AWS IMDS link-local address
+
+    delivery.deliver
+
+    assert_equal "completed", delivery.state
+    assert_equal "private_uri", delivery.response[:error]
+    assert_not delivery.succeeded?
+  end
+
+  test "connects to the pinned IP address preventing DNS re-resolution" do
+    webhook = Webhook.create!(
+      board: boards(:writebook),
+      name: "Pinned IP",
+      url: "https://example.com/webhook"
+    )
+    event = events(:layout_commented)
+    delivery = Webhook::Delivery.create!(webhook: webhook, event: event)
+
+    stub_dns_resolution(PUBLIC_TEST_IP)
+
+    # Verify Net::HTTP.new is called with the pinned IP
+    http_mock = mock("http")
+    http_mock.stubs(:use_ssl=)
+    http_mock.stubs(:open_timeout=)
+    http_mock.stubs(:read_timeout=)
+    http_mock.stubs(:request).returns(stub(code: "200"))
+
+    Net::HTTP.expects(:new).with("example.com", 443, ipaddr: PUBLIC_TEST_IP).returns(http_mock)
+
+    delivery.deliver
+
+    assert delivery.succeeded?
+  end
+
+  private
+    def stub_dns_resolution(*ips)
+      dns_mock = mock("dns")
+      dns_mock.stubs(:each_address).multiple_yields(*ips)
+      Resolv::DNS.stubs(:open).yields(dns_mock)
+    end
 end