From 3ace4bce429fe83635221bad5b74ddb315117dd6 Mon Sep 17 00:00:00 2001 From: "Marcelo M. Maciel" <4993482+marcelo-maciel@users.noreply.github.com> Date: Sat, 11 Jul 2026 16:10:42 -0300 Subject: [PATCH 1/2] fix(webhooks): block SSRF targets on outbound delivery Webhook subscriptions take a tenant-supplied destination URL, so the target is untrusted input. The create validator only checked "absolute URI + http/https scheme", and both delivery sinks (WebhookDeliveryService, WebhookDispatchJob) POSTed to new Uri(url) with AllowAutoRedirect=true and no address screening. Any authenticated tenant with Webhooks.Create could register 169.254.169.254, loopback, or an RFC1918 host and use the delivery log as a blind-SSRF oracle. - WebhookUrlGuard: central screen for loopback / link-local (cloud metadata) / RFC1918 / CGNAT / IPv6 ULA, plus "localhost". - Validator rejects blocked hosts at the create boundary (fast feedback). - "Webhooks" HttpClient: SocketsHttpHandler with AllowAutoRedirect=false and a ConnectCallback that screens the resolved IP at connect time, closing the DNS-rebinding window left by a create-time hostname check. One handler guards both sinks (shared named client). --- ...eateWebhookSubscriptionCommandValidator.cs | 5 +- .../Services/WebhookUrlGuard.cs | 112 ++++++++++++++++++ .../Modules.Webhooks/WebhooksModule.cs | 9 ++ ...teWebhookSubscriptionSsrfValidatorTests.cs | 57 +++++++++ 4 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 src/Modules/Webhooks/Modules.Webhooks/Services/WebhookUrlGuard.cs create mode 100644 src/Tests/Webhooks.Tests/CreateWebhookSubscriptionSsrfValidatorTests.cs diff --git a/src/Modules/Webhooks/Modules.Webhooks/Features/v1/CreateWebhookSubscription/CreateWebhookSubscriptionCommandValidator.cs b/src/Modules/Webhooks/Modules.Webhooks/Features/v1/CreateWebhookSubscription/CreateWebhookSubscriptionCommandValidator.cs index 1922aa8d1a..651f842910 100644 --- a/src/Modules/Webhooks/Modules.Webhooks/Features/v1/CreateWebhookSubscription/CreateWebhookSubscriptionCommandValidator.cs +++ b/src/Modules/Webhooks/Modules.Webhooks/Features/v1/CreateWebhookSubscription/CreateWebhookSubscriptionCommandValidator.cs @@ -1,5 +1,6 @@ using FluentValidation; using FSH.Modules.Webhooks.Contracts.v1.CreateWebhookSubscription; +using FSH.Modules.Webhooks.Services; namespace FSH.Modules.Webhooks.Features.v1.CreateWebhookSubscription; @@ -10,7 +11,9 @@ public CreateWebhookSubscriptionCommandValidator() RuleFor(x => x.Url).NotEmpty() .Must(url => Uri.TryCreate(url, UriKind.Absolute, out var uri) && (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps)) - .WithMessage("A valid absolute URL is required."); + .WithMessage("A valid absolute URL is required.") + .Must(url => !Uri.TryCreate(url, UriKind.Absolute, out var uri) || !WebhookUrlGuard.IsBlockedHost(uri.Host)) + .WithMessage("The URL must not target a private, loopback, link-local, or metadata address."); RuleFor(x => x.Events).NotEmpty().WithMessage("At least one event type is required."); } } diff --git a/src/Modules/Webhooks/Modules.Webhooks/Services/WebhookUrlGuard.cs b/src/Modules/Webhooks/Modules.Webhooks/Services/WebhookUrlGuard.cs new file mode 100644 index 0000000000..9e1d839192 --- /dev/null +++ b/src/Modules/Webhooks/Modules.Webhooks/Services/WebhookUrlGuard.cs @@ -0,0 +1,112 @@ +using System.Net; +using System.Net.Http; +using System.Net.Sockets; + +namespace FSH.Modules.Webhooks.Services; + +/// +/// SSRF guard for outbound webhook delivery. A tenant registers the destination URL, so the +/// target is untrusted input: without a guard a tenant can point a subscription at the cloud +/// metadata endpoint, loopback, or an internal RFC1918 host and use the delivery log as a +/// blind-SSRF oracle. gives fast feedback at the create boundary; +/// is the authoritative check applied at connect time (after DNS +/// resolution) so DNS-rebinding cannot slip a public hostname past the create check and then +/// resolve to an internal address at delivery time. +/// +public static class WebhookUrlGuard +{ + public static bool IsBlockedHost(string? host) + { + if (string.IsNullOrWhiteSpace(host)) + { + return true; + } + + // "localhost" and any *.localhost never leave the box — block by name; a literal IP is + // checked against the address ranges, real hostnames defer to the connect-time check. + if (host.Equals("localhost", StringComparison.OrdinalIgnoreCase) + || host.EndsWith(".localhost", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + return IPAddress.TryParse(host.Trim('[', ']'), out var ip) && IsBlockedAddress(ip); + } + + public static bool IsBlockedAddress(IPAddress address) + { + ArgumentNullException.ThrowIfNull(address); + + if (address.IsIPv4MappedToIPv6) + { + address = address.MapToIPv4(); + } + + if (IPAddress.IsLoopback(address) + || address.Equals(IPAddress.Any) + || address.Equals(IPAddress.IPv6Any)) + { + return true; + } + + if (address.AddressFamily == AddressFamily.InterNetwork) + { + var b = address.GetAddressBytes(); + return b[0] switch + { + 10 => true, // 10.0.0.0/8 private + 127 => true, // 127.0.0.0/8 loopback + 169 when b[1] == 254 => true, // 169.254.0.0/16 link-local (cloud metadata) + 172 when b[1] >= 16 && b[1] <= 31 => true, // 172.16.0.0/12 private + 192 when b[1] == 168 => true, // 192.168.0.0/16 private + 100 when b[1] >= 64 && b[1] <= 127 => true, // 100.64.0.0/10 carrier-grade NAT + 0 => true, // 0.0.0.0/8 unspecified + >= 224 => true, // multicast / reserved + _ => false, + }; + } + + return address.IsIPv6LinkLocal + || address.IsIPv6SiteLocal + || address.IsIPv6Multicast + || IsIPv6UniqueLocal(address); + } + + // fc00::/7 — unique local addresses (the IPv6 equivalent of RFC1918). + private static bool IsIPv6UniqueLocal(IPAddress address) => + (address.GetAddressBytes()[0] & 0xFE) == 0xFC; + + /// + /// that resolves the destination host and + /// refuses to connect to any non-routable/internal address. This is the authoritative SSRF + /// gate: it runs at delivery time on the real resolved IP, closing the DNS-rebinding window + /// left open by a create-time hostname check. + /// + public static async ValueTask ConnectAsync(SocketsHttpConnectionContext context, CancellationToken ct) + { + ArgumentNullException.ThrowIfNull(context); + + var host = context.DnsEndPoint.Host; + var addresses = await Dns.GetHostAddressesAsync(host, ct).ConfigureAwait(false); + var target = Array.Find(addresses, a => !IsBlockedAddress(a)); + if (target is null) + { + throw new HttpRequestException( + $"Blocked webhook target '{host}': it resolves only to non-routable or internal addresses."); + } + + Socket? socket = null; + try + { + socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true }; + await socket.ConnectAsync(new IPEndPoint(target, context.DnsEndPoint.Port), ct).ConfigureAwait(false); + var stream = new NetworkStream(socket, ownsSocket: true); + socket = null; // ownership transferred to the stream + return stream; + } + finally + { + socket?.Dispose(); + } + } +} diff --git a/src/Modules/Webhooks/Modules.Webhooks/WebhooksModule.cs b/src/Modules/Webhooks/Modules.Webhooks/WebhooksModule.cs index 693ea2e1d5..cf38141f20 100644 --- a/src/Modules/Webhooks/Modules.Webhooks/WebhooksModule.cs +++ b/src/Modules/Webhooks/Modules.Webhooks/WebhooksModule.cs @@ -19,6 +19,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Hosting; +using System.Net.Http; [assembly: FshModule(typeof(FSH.Modules.Webhooks.WebhooksModule), 400)] @@ -46,6 +47,14 @@ public void ConfigureServices(IHostApplicationBuilder builder) typeof(WebhookFanoutHandler<>)); builder.Services.AddHttpClient("Webhooks") + .ConfigurePrimaryHttpMessageHandler(() => new SocketsHttpHandler + { + // Untrusted tenant-supplied destination: never follow redirects (a 302 could bounce + // to an internal host) and screen the resolved IP at connect time so DNS-rebinding + // cannot map a public hostname to an internal address after the create-time check. + AllowAutoRedirect = false, + ConnectCallback = WebhookUrlGuard.ConnectAsync, + }) .AddHeroResilience(builder.Configuration); builder.Services.AddHealthChecks() diff --git a/src/Tests/Webhooks.Tests/CreateWebhookSubscriptionSsrfValidatorTests.cs b/src/Tests/Webhooks.Tests/CreateWebhookSubscriptionSsrfValidatorTests.cs new file mode 100644 index 0000000000..e4bef7aa96 --- /dev/null +++ b/src/Tests/Webhooks.Tests/CreateWebhookSubscriptionSsrfValidatorTests.cs @@ -0,0 +1,57 @@ +using System.Net; +using FSH.Modules.Webhooks.Contracts.v1.CreateWebhookSubscription; +using FSH.Modules.Webhooks.Features.v1.CreateWebhookSubscription; +using FSH.Modules.Webhooks.Services; + +namespace Webhooks.Tests; + +/// +/// Regression for audit finding SEC-01 (SSRF: outbound webhook delivery must have an egress guard). +/// The create-boundary validator rejects URLs that point at loopback / link-local (cloud metadata) / +/// private ranges, and is the authoritative connect-time +/// gate applied to the resolved IP (defeats DNS rebinding) shared by both delivery sinks. +/// +public sealed class CreateWebhookSubscriptionSsrfValidatorTests +{ + private readonly CreateWebhookSubscriptionCommandValidator _validator = new(); + + [Theory] + [InlineData("http://169.254.169.254/latest/meta-data/")] // cloud instance metadata + [InlineData("http://127.0.0.1/")] // loopback + [InlineData("http://10.0.0.1/")] // RFC1918 private range + [InlineData("http://localhost/hook")] // loopback by name + public void Validate_Should_Reject_SsrfTargetUrl(string url) + { + var command = new CreateWebhookSubscriptionCommand(url, ["ticket.created"], Secret: null); + + var result = _validator.Validate(command); + + result.IsValid.ShouldBeFalse( + $"webhook target '{url}' resolves to a private/loopback/metadata address and must be rejected " + + "at the create boundary to prevent SSRF."); + } + + [Fact] + public void Validate_Should_Accept_PublicHttpsUrl() + { + var command = new CreateWebhookSubscriptionCommand("https://hooks.example.com/receive", ["ticket.created"], Secret: null); + + var result = _validator.Validate(command); + + result.IsValid.ShouldBeTrue(); + } + + [Theory] + [InlineData("127.0.0.1", true)] // loopback + [InlineData("10.5.6.7", true)] // RFC1918 + [InlineData("172.16.0.1", true)] // RFC1918 + [InlineData("192.168.1.1", true)] // RFC1918 + [InlineData("169.254.169.254", true)] // link-local / metadata + [InlineData("100.64.0.1", true)] // carrier-grade NAT + [InlineData("::1", true)] // IPv6 loopback + [InlineData("fd00::1", true)] // IPv6 unique-local + [InlineData("8.8.8.8", false)] // public + [InlineData("1.1.1.1", false)] // public + public void IsBlockedAddress_Should_ScreenNonRoutableTargets(string ip, bool blocked) => + WebhookUrlGuard.IsBlockedAddress(IPAddress.Parse(ip)).ShouldBe(blocked); +} From 36dcbdcef0be9ad7f464b79160079de631804e32 Mon Sep 17 00:00:00 2001 From: "Marcelo M. Maciel" <4993482+marcelo-maciel@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:43:18 -0300 Subject: [PATCH 2/2] test(webhooks): use an unresolvable .invalid target in the dispatch-failure test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dispatch-job test created a subscription pointing at https://127.0.0.1:1 to simulate an unreachable endpoint, but the new SSRF create guard correctly rejects loopback targets, so the create returned 400. Switch to a .invalid host, which never resolves (fast transient failure at DNS) and passes the create guard like any public hostname — same test intent, compatible with the egress guard. --- .../Integration.Tests/Tests/Webhooks/WebhookDispatchJobTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/Integration.Tests/Tests/Webhooks/WebhookDispatchJobTests.cs b/src/Tests/Integration.Tests/Tests/Webhooks/WebhookDispatchJobTests.cs index 25e35a7ab8..756e66c82f 100644 --- a/src/Tests/Integration.Tests/Tests/Webhooks/WebhookDispatchJobTests.cs +++ b/src/Tests/Integration.Tests/Tests/Webhooks/WebhookDispatchJobTests.cs @@ -68,7 +68,7 @@ public async Task DispatchAsync_Should_PersistDeliveryRow_And_ThrowOnTransientFa $"{TestConstants.WebhooksBasePath}/subscriptions", new { - url = "https://127.0.0.1:1/dispatch-test", // unreachable → transient failure + url = "https://webhook-dispatch.invalid/dispatch-test", // .invalid never resolves → transient failure (and passes the SSRF create guard, unlike a loopback/private target) events = new[] { uniqueEvent }, secret = "dispatch-secret" });