What the developer wrote
Authored by Nicolas Dorier
Small refactoring
What changed, and why it matters
This commit tweaks how BTCPay Server blocks internal/private network addresses when making outbound connections (an SSRF protection feature). Previously, if any resolved address was private, the whole request was rejected. Now, the code skips private addresses and tries only public ones. If no public address exists, it still rejects the request. The change is described by the author as a small refactoring, but it alters the security behavior slightly: a hostname that resolves to both public and private addresses will now connect to the public address instead of failing outright. The tests were updated only to match the new error message.
Review the SSRF protection change as a potential security-relevant behavior modification, not merely a refactor. Verify that skipping local addresses does not introduce DNS rebinding or mixed-resolution bypasses, and confirm IsLocalNetwork covers all intended private/reserved ranges. Consider adding tests for mixed public/private DNS resolution scenarios.
Security signals we found
Modifies SSRF protection logic
Changes error message for blocked local addresses
Behavioral change in address filtering: skip-local vs reject-if-any-local
No explicit security rationale in commit message
No CVE or advisory references present
Evidence from the diff
SSRFProtectionExtensions.cs previously resolved a hostname to all IP addresses and threw if the list was empty or any address matched IsLocalNetwork. The new logic iterates addresses, skips local ones, and attempts to connect to each remaining address. If all addresses are local, it throws a new message: “The endpoint does not resolve a public network address”. The unit tests were updated to expect that new message. This is a behavioral change, not a pure refactor: mixed public/private resolution now succeeds if a public address is reachable, whereas before it would fail. It is unclear whether this weakens or strengthens SSRF protection without additional context on IsLocalNetwork coverage and DNS rebinding scenarios.
Changed components
BTCPayServer/SSRFProtectionExtensions.csBTCPayServer.Tests/LightningTests.csInspect captured patch +9 / −9
### BTCPayServer.Tests/LightningTests.cs
@@ -772,18 +772,18 @@ public async Task CanSetLightningServer()
.CreateClient(LightningClientFactoryService.SafeNamedClient);
var localUri = new UriBuilder(tester.PayTester.ServerUriWithIP) { Host = "localhost" }.Uri;
var exception = await Assert.ThrowsAsync<HttpRequestException>(() => safeLightningHttpClient.GetAsync(localUri));
- Assert.Contains("does not resolve exclusively to public addresses", exception.Message);
+ Assert.Contains("does not resolve a public network address", exception.Message);
var lnurlHttpClient = tester.PayTester.GetService<IHttpClientFactory>()
.CreateClient(LightningLikePayoutHandler.LightningLikePayoutHandlerClearnetNamedClient);
exception = await Assert.ThrowsAsync<HttpRequestException>(() => lnurlHttpClient.GetAsync(localUri));
- Assert.Contains("does not resolve exclusively to public addresses", exception.Message);
+ Assert.Contains("does not resolve a public network address", exception.Message);
var guardedLightningClient = tester.PayTester.GetService<LightningClientFactoryService>().Create(
$"type=phoenixd;server={localUri};password=secret",
tester.PayTester.Networks.GetNetwork<BTCPayNetwork>("BTC"),
allowUnsafe: false);
exception = await Assert.ThrowsAsync<HttpRequestException>(() => guardedLightningClient.GetInfo());
- Assert.Contains("does not resolve exclusively to public addresses", exception.Message);
+ Assert.Contains("does not resolve a public network address", exception.Message);
var onionHandler = tester.PayTester.GetService<IHttpMessageHandlerFactory>()
.CreateHandler(LightningClientFactoryService.OnionNamedClient);
while (onionHandler is DelegatingHandler delegatingHandler)
### BTCPayServer/SSRFProtectionExtensions.cs
@@ -31,15 +31,12 @@ static async ValueTask<Stream> Connect(SocketsHttpConnectionContext context,
{
var addresses = await Dns.GetHostAddressesAsync(context.DnsEndPoint.Host,
AddressFamily.Unspecified, cancellationToken);
- if (addresses.Length is 0 ||
- addresses.Any(a => BTCPayServer.Extensions.IsLocalNetwork(a.ToString())))
- {
- throw new HttpRequestException("The endpoint does not resolve exclusively to public addresses");
- }
Exception lastException = null;
foreach (var address in addresses)
{
+ if (Extensions.IsLocalNetwork(address.ToString()))
+ continue;
var socket = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
try
{
@@ -58,6 +55,9 @@ static async ValueTask<Stream> Connect(SocketsHttpConnectionContext context,
}
}
- throw new HttpRequestException("Could not connect to the endpoint", lastException);
+ if (lastException is null)
+ throw new HttpRequestException("The endpoint does not resolve a public network address");
+ else
+ throw new HttpRequestException("Could not connect to the endpoint", lastException);
}
}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.