From 29c7926307f55028ac9380d893371938928657c1 Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Tue, 9 Dec 2025 11:25:12 +0100
Subject: [PATCH] Stream response in webhook request manually to check size

This addresses a DoS vulnerability where the response might be massive
leading to OOM errors, as the response is read in full in memory by
default.

To prevent this, we need to read the body in chunks, checking the
size of the chunks we've read and raising if we go over a certain limit.

I've set the limit to 100 KB because the responses to these requests
should be fairly small or even empty, and we only care about the status
code in the end.
---
 app/models/webhook/delivery.rb       | 21 ++++++++++++++++---
 test/models/webhook/delivery_test.rb | 31 +++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/app/models/webhook/delivery.rb b/app/models/webhook/delivery.rb
index c5dab1497..3ebe49b06 100644
--- a/app/models/webhook/delivery.rb
+++ b/app/models/webhook/delivery.rb
@@ -1,7 +1,10 @@
 class Webhook::Delivery < ApplicationRecord
+  class ResponseTooLarge < StandardError; end
+
   STALE_TRESHOLD = 7.days
   USER_AGENT = "fizzy/1.0.0 Webhook"
   ENDPOINT_TIMEOUT = 7.seconds
+  MAX_RESPONSE_SIZE = 100.kilobytes
 
   belongs_to :account, default: -> { webhook.account }
   belongs_to :webhook
@@ -52,12 +55,16 @@ class Webhook::Delivery < ApplicationRecord
       if resolved_ip.nil?
         { error: :private_uri }
       else
-        response = http.request(
-          Net::HTTP::Post.new(uri, headers).tap { |request| request.body = payload }
-        )
+        request = Net::HTTP::Post.new(uri, headers).tap { |request| request.body = payload }
+
+        response = http.request(request) do |net_http_response|
+          stream_body_with_limit(net_http_response)
+        end
 
         { code: response.code.to_i }
       end
+    rescue ResponseTooLarge
+      { error: :response_too_large }
     rescue Resolv::ResolvTimeout, Resolv::ResolvError, SocketError
       { error: :dns_lookup_failed }
     rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ETIMEDOUT
@@ -68,6 +75,14 @@ class Webhook::Delivery < ApplicationRecord
       { error: :failed_tls }
     end
 
+    def stream_body_with_limit(response)
+      bytes_read = 0
+      response.read_body do |chunk|
+        bytes_read += chunk.bytesize
+        raise ResponseTooLarge if bytes_read > MAX_RESPONSE_SIZE
+      end
+    end
+
     def resolved_ip
       return @resolved_ip if defined?(@resolved_ip)
       @resolved_ip = SsrfProtection.resolve_public_ip(uri.host)
diff --git a/test/models/webhook/delivery_test.rb b/test/models/webhook/delivery_test.rb
index 30f91390e..d4d95528c 100644
--- a/test/models/webhook/delivery_test.rb
+++ b/test/models/webhook/delivery_test.rb
@@ -296,12 +296,15 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase
     stub_dns_resolution(PUBLIC_TEST_IP)
 
     # Verify Net::HTTP.new is called with the pinned IP
+    response_mock = stub(code: "200")
+    response_mock.stubs(:read_body)
+
     http_mock = mock("http")
     http_mock.stubs(:use_ssl=)
     http_mock.stubs(:ipaddr=)
     http_mock.stubs(:open_timeout=)
     http_mock.stubs(:read_timeout=)
-    http_mock.stubs(:request).returns(stub(code: "200"))
+    http_mock.stubs(:request).yields(response_mock).returns(response_mock)
 
     Net::HTTP.expects(:new).with("example.com", 443).returns(http_mock)
 
@@ -310,6 +313,32 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase
     assert delivery.succeeded?
   end
 
+  test "handles response too large error" do
+    delivery = webhook_deliveries(:pending)
+
+    large_body = "x" * 200.kilobytes
+    stub_request(:post, delivery.webhook.url).to_return(status: 200, body: large_body)
+
+    delivery.deliver
+
+    assert_equal "completed", delivery.state
+    assert_equal "response_too_large", delivery.response[:error]
+    assert_not delivery.succeeded?
+  end
+
+  test "allows responses within size limit" do
+    delivery = webhook_deliveries(:pending)
+
+    small_body = "x" * 50.kilobytes
+    stub_request(:post, delivery.webhook.url).to_return(status: 200, body: small_body)
+
+    delivery.deliver
+
+    assert_equal "completed", delivery.state
+    assert_equal 200, delivery.response[:code]
+    assert delivery.succeeded?
+  end
+
   private
     def stub_dns_resolution(*ips)
       dns_mock = mock("dns")