What the developer wrote
Authored by Nicolas Dorier
Refactor SSRF
What changed, and why it matters
This commit is a code cleanup that changes how BTCPay Server blocks outgoing requests to internal/local network addresses (SSRF protection). It moves the protection logic from a shared service into an extension method used when configuring HTTP clients. The change appears intended to be behavior-preserving, but it removes some unit tests that previously verified the protection works for Bitpay and Webhook clients. Because the patch is only a refactor and no vulnerability is introduced in the diff, this is not a clear security issue on its own, though removing tests reduces future assurance.
Treat as a maintenance refactor rather than a vulnerability. Review whether the removed SSRF tests are covered by integration tests or reintroduce equivalent tests against the new UseSSRFProtection extension to prevent regressions. Verify that all clearnet HTTP clients that should be protected still call UseSSRFProtection.
Security signals we found
SSRF protection logic refactored but preserved
Unit tests verifying SSRF rejection for Bitpay and Webhooks removed
Manual unprotected-handler test path removed in LightningTests
Protection still gated by DisableSSRFProtection option
Evidence from the diff
The refactor converts SSRFProtection from a singleton service into a static extension method (UseSSRFProtection) that configures SocketsHttpHandler.ConnectCallback to resolve DNS and reject addresses matching IsLocalNetwork unless BTCPayServerOptions.DisableSSRFProtection is true. The callback logic itself remains functionally identical. The commit removes two direct unit tests (BitpayIPNClientRejectsLocalEndpoints and WebhookClientRejectsLocalEndpoints) and trims a test in LightningTests that used a manually constructed unprotected handler. The production code now applies UseSSRFProtection to LightningClientFactoryService.SafeNamedClient, LightningLikePayoutHandler.LightningLikePayoutHandlerClearnetNamedClient, BitpayIPNSender.NamedClient, and WebhookSender.ClearnetNamedClient. No new attack surface is opened by the diff, but the removed tests lower observability of regressions.
Changed components
BTCPayServer/SSRFProtectionExtensions.csBTCPayServer/Hosting/BTCPayServerServices.csBTCPayServer/Plugins/Bitpay/BitpayPlugin.csBTCPayServer/Plugins/Webhooks/WebhooksPlugin.csBTCPayServer.Tests/BitpayTests.csBTCPayServer.Tests/LightningTests.csBTCPayServer.Tests/WebhooksTests.csInspect captured patch +26 / −69
### BTCPayServer.Tests/BitpayTests.cs
@@ -33,22 +33,6 @@ namespace BTCPayServer.Tests;
[Collection(nameof(NonParallelizableCollectionDefinition))]
public class BitpayTests(ITestOutputHelper log) : UnitTestBase(log)
{
- [Fact]
- public async Task BitpayIPNClientRejectsLocalEndpoints()
- {
- var services = new ServiceCollection();
- services.AddSingleton(new BTCPayServerOptions());
- services.AddSingleton<SSRFProtection>();
- new BitpayPlugin().Execute(services);
- await using var serviceProvider = services.BuildServiceProvider();
- var client = serviceProvider.GetRequiredService<IHttpClientFactory>()
- .CreateClient(BitpayIPNSender.NamedClient);
-
- var exception = await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync("http://localhost"));
-
- Assert.Contains("does not resolve exclusively to public addresses", exception.Message);
- }
-
[Fact]
[Trait("Integration", "Integration")]
public async Task CanUseServerInitiatedPairingCode()
### BTCPayServer.Tests/LightningTests.cs
@@ -777,15 +777,7 @@ public async Task CanSetLightningServer()
.CreateClient(LightningLikePayoutHandler.LightningLikePayoutHandlerClearnetNamedClient);
exception = await Assert.ThrowsAsync<HttpRequestException>(() => lnurlHttpClient.GetAsync(localUri));
Assert.Contains("does not resolve exclusively to public addresses", exception.Message);
- using var unprotectedHandler = new SocketsHttpHandler
- {
- AllowAutoRedirect = false,
- UseProxy = false,
- ConnectCallback = new SSRFProtection(new BTCPayServerOptions { DisableSSRFProtection = true }).Connect
- };
- using var unprotectedClient = new HttpClient(unprotectedHandler);
- using var unprotectedResponse = await unprotectedClient.GetAsync(localUri);
- Assert.Equal(HttpStatusCode.Redirect, unprotectedResponse.StatusCode);
+
var guardedLightningClient = tester.PayTester.GetService<LightningClientFactoryService>().Create(
$"type=phoenixd;server={localUri};password=secret",
tester.PayTester.Networks.GetNetwork<BTCPayNetwork>("BTC"),
### BTCPayServer.Tests/WebhooksTests.cs
@@ -26,22 +26,6 @@ namespace BTCPayServer.Tests;
public class WebhooksTests(ITestOutputHelper log) : UnitTestBase(log)
{
- [Fact]
- public async Task WebhookClientRejectsLocalEndpoints()
- {
- var services = new ServiceCollection();
- services.AddSingleton(new BTCPayServerOptions());
- services.AddSingleton<SSRFProtection>();
- new WebhooksPlugin().Execute(services);
- await using var serviceProvider = services.BuildServiceProvider();
- var client = serviceProvider.GetRequiredService<IHttpClientFactory>()
- .CreateClient(WebhookSender.ClearnetNamedClient);
-
- var exception = await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync("http://localhost"));
-
- Assert.Contains("does not resolve exclusively to public addresses", exception.Message);
- }
-
[Fact]
[Trait("Playwright", "Playwright-2")]
public async Task CanUseWebhooks()
### BTCPayServer/Hosting/BTCPayServerServices.cs
@@ -132,16 +132,12 @@ public static IServiceCollection AddBTCPayServer(this IServiceCollection service
services.AddSingleton<Func<HttpClient, ILightningConnectionStringHandler>>(client =>
new LndHubConnectionStringHandler(client));
services.TryAddSingleton<LightningClientFactoryService>();
- services.TryAddSingleton<SSRFProtection>();
services.AddHttpClient(LightningClientFactoryService.NamedClient)
.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { AllowAutoRedirect = false });
services.AddHttpClient(LightningClientFactoryService.SafeNamedClient)
- .ConfigurePrimaryHttpMessageHandler(sp => new SocketsHttpHandler
- {
- AllowAutoRedirect = false,
- UseProxy = false,
- ConnectCallback = sp.GetRequiredService<SSRFProtection>().Connect
- });
+ .UseSSRFProtection()
+ .ConfigurePrimaryHttpMessageHandler((handler, _) =>
+ ((SocketsHttpHandler)handler).AllowAutoRedirect = false);
services.AddHttpClient(LightningClientFactoryService.OnionNamedClient)
.ConfigurePrimaryHttpMessageHandler(sp =>
{
@@ -404,11 +400,7 @@ public static IServiceCollection AddBTCPayServer(this IServiceCollection service
services.AddHttpClient(LightningLikePayoutHandler.LightningLikePayoutHandlerOnionNamedClient)
.ConfigurePrimaryHttpMessageHandler<Socks5HttpClientHandler>();
services.AddHttpClient(LightningLikePayoutHandler.LightningLikePayoutHandlerClearnetNamedClient)
- .ConfigurePrimaryHttpMessageHandler(sp => new SocketsHttpHandler
- {
- UseProxy = false,
- ConnectCallback = sp.GetRequiredService<SSRFProtection>().Connect
- });
+ .UseSSRFProtection();
services.AddSingleton<HostedServices.PullPaymentHostedService>();
services.AddSingleton<IHostedService, HostedServices.PullPaymentHostedService>(o => o.GetRequiredService<PullPaymentHostedService>());
### BTCPayServer/Plugins/Bitpay/BitpayPlugin.cs
@@ -1,5 +1,4 @@
#nullable enable
-using System.Net.Http;
using BTCPayServer.Abstractions.Constants;
using BTCPayServer.Abstractions.Models;
using BTCPayServer.Client;
@@ -37,11 +36,7 @@ public override void Execute(IServiceCollection services)
{
client.DefaultRequestHeaders.UserAgent.Add(userAgent);
})
- .ConfigurePrimaryHttpMessageHandler(sp => new SocketsHttpHandler
- {
- UseProxy = false,
- ConnectCallback = sp.GetRequiredService<SSRFProtection>().Connect
- });
+ .UseSSRFProtection();
services.AddSingleton<MatcherPolicy, BitpayEndpointSelectorPolicy>();
services.TryAddSingleton<TokenRepository>();
### BTCPayServer/Plugins/Webhooks/WebhooksPlugin.cs
@@ -1,7 +1,6 @@
#nullable enable
using System;
using System.Collections.Generic;
-using System.Net.Http;
using BTCPayServer.Abstractions.Models;
using BTCPayServer.Client;
using BTCPayServer.Client.Models;
@@ -40,11 +39,7 @@ public override void Execute(IServiceCollection services)
services.AddHttpClient(WebhookSender.OnionNamedClient)
.ConfigurePrimaryHttpMessageHandler<Socks5HttpClientHandler>();
services.AddHttpClient(WebhookSender.ClearnetNamedClient)
- .ConfigurePrimaryHttpMessageHandler(sp => new SocketsHttpHandler
- {
- UseProxy = false,
- ConnectCallback = sp.GetRequiredService<SSRFProtection>().Connect
- });
+ .UseSSRFProtection();
var userAgent = BTCPayServerEnvironment.GetUserAgentHeaderValue();
foreach (var clientName in WebhookSender.AllClients)
{
### BTCPayServer/SSRFProtectionExtensions.cs
@@ -7,18 +7,32 @@
using System.Threading;
using System.Threading.Tasks;
using BTCPayServer.Configuration;
+using Microsoft.Extensions.DependencyInjection;
-namespace BTCPayServer.Services;
+namespace BTCPayServer;
-public class SSRFProtection(BTCPayServerOptions options)
+public static class SSRFProtectionExtensions
{
- public async ValueTask<Stream> Connect(SocketsHttpConnectionContext context,
+ public static IHttpClientBuilder UseSSRFProtection(this IHttpClientBuilder builder)
+ => builder.ConfigurePrimaryHttpMessageHandler((h, sp) =>
+ {
+ var handler = (SocketsHttpHandler)h;
+ var opt = sp.GetRequiredService<BTCPayServerOptions>();
+ if (!opt.DisableSSRFProtection)
+ {
+ handler.UseProxy = false;
+ handler.ConnectCallback = Connect;
+ }
+ });
+
+
+ static async ValueTask<Stream> Connect(SocketsHttpConnectionContext context,
CancellationToken cancellationToken)
{
var addresses = await Dns.GetHostAddressesAsync(context.DnsEndPoint.Host,
AddressFamily.Unspecified, cancellationToken);
if (addresses.Length is 0 ||
- !options.DisableSSRFProtection && addresses.Any(a => BTCPayServer.Extensions.IsLocalNetwork(a.ToString())))
+ addresses.Any(a => BTCPayServer.Extensions.IsLocalNetwork(a.ToString())))
{
throw new HttpRequestException("The endpoint does not resolve exclusively to public addresses");
}
### btcpayserver.sln.DotSettings
@@ -15,6 +15,7 @@
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=PSBT/@EntryIndexedValue">PSBT</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=SMTP/@EntryIndexedValue">SMTP</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=SSH/@EntryIndexedValue">SSH</s:String>
+ <s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=SSRF/@EntryIndexedValue">SSRF</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=TX/@EntryIndexedValue">TX</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=UI/@EntryIndexedValue">UI</s:String>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Bitpay/@EntryIndexedValue">True</s:Boolean>Why this scored 45/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.