diff --git a/src/Icons/Models/IconUri.cs b/src/Icons/Models/IconUri.cs
index 143bc26f72..e436d40066 100644
--- a/src/Icons/Models/IconUri.cs
+++ b/src/Icons/Models/IconUri.cs
@@ -39,7 +39,7 @@ public class IconUri
}
///
- /// Represents an ip-validated Uri for use in grabbing an icon.
+ /// Represents a URI validated against SSRF: resolved to a public IP and bound to it.
///
///
///
diff --git a/src/Icons/Services/ChangePasswordUriService.cs b/src/Icons/Services/ChangePasswordUriService.cs
index 6f2b73efff..63dc1ddb5b 100644
--- a/src/Icons/Services/ChangePasswordUriService.cs
+++ b/src/Icons/Services/ChangePasswordUriService.cs
@@ -1,12 +1,27 @@
-namespace Bit.Icons.Services;
+using System.Net;
+using Bit.Icons.Models;
+
+namespace Bit.Icons.Services;
public class ChangePasswordUriService : IChangePasswordUriService
{
+ private const int _maxRedirects = 2;
+
+ private static readonly HttpStatusCode[] _redirectStatusCodes =
+ [
+ HttpStatusCode.Redirect,
+ HttpStatusCode.MovedPermanently,
+ HttpStatusCode.RedirectKeepVerb,
+ HttpStatusCode.SeeOther
+ ];
+
private readonly HttpClient _httpClient;
+ private readonly IUriService _uriService;
- public ChangePasswordUriService(IHttpClientFactory httpClientFactory)
+ public ChangePasswordUriService(IHttpClientFactory httpClientFactory, IUriService uriService)
{
_httpClient = httpClientFactory.CreateClient("ChangePasswordUri");
+ _uriService = uriService;
}
///
@@ -24,7 +39,6 @@ public class ChangePasswordUriService : IChangePasswordUriService
var hasReliableStatusCode = await HasReliableHttpStatusCode(domain);
var wellKnownChangePasswordUrl = await GetWellKnownChangePasswordUrl(domain);
-
if (hasReliableStatusCode && wellKnownChangePasswordUrl != null)
{
return wellKnownChangePasswordUrl;
@@ -49,10 +63,16 @@ public class ChangePasswordUriService : IChangePasswordUriService
Path = "/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200"
};
- var request = new HttpRequestMessage(HttpMethod.Get, url.ToString());
+ var response = await SendSafeRequestAsync(url.Uri);
+ if (response == null)
+ {
+ return false;
+ }
- var response = await _httpClient.SendAsync(request);
- return !response.IsSuccessStatusCode;
+ using (response)
+ {
+ return !response.IsSuccessStatusCode;
+ }
}
catch
{
@@ -76,14 +96,72 @@ public class ChangePasswordUriService : IChangePasswordUriService
Path = "/.well-known/change-password"
};
- var request = new HttpRequestMessage(HttpMethod.Get, url.ToString());
+ var response = await SendSafeRequestAsync(url.Uri);
+ if (response == null)
+ {
+ return null;
+ }
- var response = await _httpClient.SendAsync(request);
- return response.IsSuccessStatusCode ? url.ToString() : null;
+ using (response)
+ {
+ return response.IsSuccessStatusCode ? url.ToString() : null;
+ }
}
catch
{
return null;
}
}
+
+ ///
+ /// Sends an HTTP GET request with SSRF protections: validates the target IP is not internal,
+ /// binds the request to the resolved IP to prevent DNS rebinding, and manually follows redirects
+ /// with validation at each hop.
+ ///
+ /// The HTTP response, or null if the URI fails SSRF validation.
+ private async Task SendSafeRequestAsync(Uri uri)
+ {
+ if (!_uriService.TryGetUri(uri, out var iconUri) || !iconUri!.IsValid)
+ {
+ return null;
+ }
+
+ return await SendWithRedirectsAsync(iconUri, 0);
+ }
+
+ private async Task SendWithRedirectsAsync(IconUri iconUri, int redirectCount)
+ {
+ HttpResponseMessage response;
+ try
+ {
+ using var message = new HttpRequestMessage(HttpMethod.Get, iconUri.InnerUri);
+ message.Headers.Host = iconUri.Host;
+ response = await _httpClient.SendAsync(message);
+ }
+ catch
+ {
+ return null;
+ }
+
+ if (response.IsSuccessStatusCode || !_redirectStatusCodes.Contains(response.StatusCode))
+ {
+ return response;
+ }
+
+ // Handle redirect with SSRF validation
+ using (response)
+ {
+ if (redirectCount >= _maxRedirects || response.Headers.Location == null)
+ {
+ return null;
+ }
+
+ if (!_uriService.TryGetRedirect(response, iconUri, out var redirectIconUri) || !redirectIconUri!.IsValid)
+ {
+ return null;
+ }
+
+ return await SendWithRedirectsAsync(redirectIconUri, redirectCount + 1);
+ }
+ }
}
diff --git a/src/Icons/Util/ServiceCollectionExtension.cs b/src/Icons/Util/ServiceCollectionExtension.cs
index 3bd3537198..9d774a139e 100644
--- a/src/Icons/Util/ServiceCollectionExtension.cs
+++ b/src/Icons/Util/ServiceCollectionExtension.cs
@@ -29,8 +29,8 @@ public static class ServiceCollectionExtension
AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate,
});
- // The CreatePasswordUri handler wants similar headers as Icons to portray coming from a browser but
- // needs to follow redirects to get the final URL.
+ // The ChangePasswordUri handler wants similar headers as Icons to portray coming from a browser.
+ // Redirects are handled manually with SSRF validation at each hop.
services.AddHttpClient("ChangePasswordUri", client =>
{
client.Timeout = TimeSpan.FromSeconds(20);
@@ -44,6 +44,7 @@ public static class ServiceCollectionExtension
client.DefaultRequestHeaders.Add("Pragma", "no-cache");
}).ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler
{
+ AllowAutoRedirect = false,
AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate,
});
}
diff --git a/test/Icons.Test/Services/ChangePasswordUriServiceTests.cs b/test/Icons.Test/Services/ChangePasswordUriServiceTests.cs
index 53b883733b..f4f4bf0092 100644
--- a/test/Icons.Test/Services/ChangePasswordUriServiceTests.cs
+++ b/test/Icons.Test/Services/ChangePasswordUriServiceTests.cs
@@ -1,4 +1,5 @@
using System.Net;
+using Bit.Icons.Models;
using Bit.Icons.Services;
using Bit.Test.Common.MockedHttpClient;
using NSubstitute;
@@ -8,32 +9,123 @@ namespace Bit.Icons.Test.Services;
public class ChangePasswordUriServiceTests : ServiceTestBase
{
+ private static readonly IPAddress _publicIp = IPAddress.Parse("93.184.216.34");
+ private static readonly IPAddress _loopbackIp = IPAddress.Parse("127.0.0.1");
+
+ ///
+ /// A fake IUriService that resolves all URIs to the given IP address.
+ ///
+ private class FakeUriService : IUriService
+ {
+ private readonly IPAddress _ip;
+ private readonly bool _shouldSucceed;
+
+ public FakeUriService(IPAddress ip, bool shouldSucceed = true)
+ {
+ _ip = ip;
+ _shouldSucceed = shouldSucceed;
+ }
+
+ public bool TryGetUri(string stringUri, out IconUri? iconUri)
+ {
+ if (!_shouldSucceed || !Uri.TryCreate(stringUri, UriKind.Absolute, out var uri))
+ {
+ iconUri = null;
+ return false;
+ }
+ iconUri = new IconUri(uri, _ip);
+ return true;
+ }
+
+ public bool TryGetUri(Uri uri, out IconUri? iconUri)
+ {
+ if (!_shouldSucceed)
+ {
+ iconUri = null;
+ return false;
+ }
+ iconUri = new IconUri(uri, _ip);
+ return true;
+ }
+
+ public bool TryGetRedirect(HttpResponseMessage response, IconUri originalUri, out IconUri? iconUri)
+ {
+ iconUri = null;
+ return false;
+ }
+ }
+
+ ///
+ /// A fake IUriService that succeeds for initial requests but returns an internal IP for redirects.
+ ///
+ private class FakeUriServiceWithInternalRedirect : IUriService
+ {
+ private readonly IPAddress _initialIp;
+ private readonly IPAddress _redirectIp;
+
+ public FakeUriServiceWithInternalRedirect(IPAddress initialIp, IPAddress redirectIp)
+ {
+ _initialIp = initialIp;
+ _redirectIp = redirectIp;
+ }
+
+ public bool TryGetUri(string stringUri, out IconUri? iconUri)
+ {
+ if (!Uri.TryCreate(stringUri, UriKind.Absolute, out var uri))
+ {
+ iconUri = null;
+ return false;
+ }
+ iconUri = new IconUri(uri, _initialIp);
+ return true;
+ }
+
+ public bool TryGetUri(Uri uri, out IconUri? iconUri)
+ {
+ iconUri = new IconUri(uri, _initialIp);
+ return true;
+ }
+
+ public bool TryGetRedirect(HttpResponseMessage response, IconUri originalUri, out IconUri? iconUri)
+ {
+ if (response.Headers.Location == null)
+ {
+ iconUri = null;
+ return false;
+ }
+
+ var redirectUri = response.Headers.Location.IsAbsoluteUri
+ ? response.Headers.Location
+ : new Uri(originalUri.InnerUri, response.Headers.Location);
+
+ iconUri = new IconUri(redirectUri, _redirectIp);
+ return true;
+ }
+ }
+
[Theory]
[InlineData("https://example.com", "https://example.com:443/.well-known/change-password")]
public async Task GetChangePasswordUri_WhenBothChecksPass_ReturnsWellKnownUrl(string domain, string expectedUrl)
{
// Arrange
var mockedHandler = new MockedHttpMessageHandler();
+ var uriService = new FakeUriService(_publicIp);
- var nonExistentUrl = $"{domain}/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200";
- var changePasswordUrl = $"{domain}/.well-known/change-password";
-
- // Mock the response for the resource-that-should-not-exist request (returns 404)
+ // Match requests by path since the host will be the resolved IP
mockedHandler
- .When(nonExistentUrl)
+ .When(r => r.RequestUri!.AbsolutePath.Contains("resource-that-should-not-exist"))
.RespondWith(HttpStatusCode.NotFound)
.WithContent(new StringContent("Not found"));
- // Mock the response for the change-password request (returns 200)
mockedHandler
- .When(changePasswordUrl)
+ .When(r => r.RequestUri!.AbsolutePath == "/.well-known/change-password")
.RespondWith(HttpStatusCode.OK)
.WithContent(new StringContent("Ok"));
var mockHttpFactory = Substitute.For();
mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
- var service = new ChangePasswordUriService(mockHttpFactory);
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
var result = await service.GetChangePasswordUri(domain);
@@ -44,23 +136,23 @@ public class ChangePasswordUriServiceTests : ServiceTestBase();
var mockedHandler = new MockedHttpMessageHandler();
+ var uriService = new FakeUriService(_publicIp);
mockedHandler
- .When(HttpMethod.Get, $"{domain}/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200")
+ .When(r => r.RequestUri!.AbsolutePath.Contains("resource-that-should-not-exist"))
.RespondWith(HttpStatusCode.OK)
.WithContent(new StringContent("Ok"));
mockedHandler
- .When(HttpMethod.Get, $"{domain}/.well-known/change-password")
+ .When(r => r.RequestUri!.AbsolutePath == "/.well-known/change-password")
.RespondWith(HttpStatusCode.OK)
.WithContent(new StringContent("Ok"));
- var httpClient = mockedHandler.ToHttpClient();
- mockHttpFactory.CreateClient("ChangePasswordUri").Returns(httpClient);
+ var mockHttpFactory = Substitute.For();
+ mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
- var service = new ChangePasswordUriService(mockHttpFactory);
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
var result = await service.GetChangePasswordUri(domain);
@@ -71,23 +163,23 @@ public class ChangePasswordUriServiceTests : ServiceTestBase();
var mockedHandler = new MockedHttpMessageHandler();
+ var uriService = new FakeUriService(_publicIp);
mockedHandler
- .When(HttpMethod.Get, $"{domain}/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200")
+ .When(r => r.RequestUri!.AbsolutePath.Contains("resource-that-should-not-exist"))
.RespondWith(HttpStatusCode.NotFound)
.WithContent(new StringContent("Not found"));
mockedHandler
- .When(HttpMethod.Get, $"{domain}/.well-known/change-password")
+ .When(r => r.RequestUri!.AbsolutePath == "/.well-known/change-password")
.RespondWith(HttpStatusCode.NotFound)
.WithContent(new StringContent("Not found"));
- var httpClient = mockedHandler.ToHttpClient();
- mockHttpFactory.CreateClient("ChangePasswordUri").Returns(httpClient);
+ var mockHttpFactory = Substitute.For();
+ mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
- var service = new ChangePasswordUriService(mockHttpFactory);
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
var result = await service.GetChangePasswordUri(domain);
@@ -99,10 +191,78 @@ public class ChangePasswordUriServiceTests : ServiceTestBase();
- var service = new ChangePasswordUriService(mockHttpFactory);
+ var uriService = new FakeUriService(_publicIp);
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
var result = await service.GetChangePasswordUri(domain);
Assert.Null(result);
}
+
+ [Fact]
+ public async Task GetChangePasswordUri_WhenDomainResolvesToInternalIp_ReturnsNull()
+ {
+ // UriService returns an IconUri with a loopback IP, which makes IsValid return false
+ var uriService = new FakeUriService(_loopbackIp);
+
+ var mockedHandler = new MockedHttpMessageHandler();
+ var mockHttpFactory = Substitute.For();
+ mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
+
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
+
+ var result = await service.GetChangePasswordUri("https://evil.com");
+
+ // No HTTP requests should have been made
+ Assert.Empty(mockedHandler.CapturedRequests);
+ Assert.Null(result);
+ }
+
+ [Fact]
+ public async Task GetChangePasswordUri_WhenDnsResolutionFails_ReturnsNull()
+ {
+ var uriService = new FakeUriService(_publicIp, shouldSucceed: false);
+
+ var mockedHandler = new MockedHttpMessageHandler();
+ var mockHttpFactory = Substitute.For();
+ mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
+
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
+
+ var result = await service.GetChangePasswordUri("https://nonexistent.invalid");
+
+ Assert.Empty(mockedHandler.CapturedRequests);
+ Assert.Null(result);
+ }
+
+ [Fact]
+ public async Task GetChangePasswordUri_WhenRedirectTargetsInternalIp_ReturnsNull()
+ {
+ // Initial URI resolves to a public IP, but redirects resolve to a loopback IP
+ var uriService = new FakeUriServiceWithInternalRedirect(_publicIp, _loopbackIp);
+
+ var mockedHandler = new MockedHttpMessageHandler();
+
+ // Both endpoints redirect (simulating attacker redirect to localhost)
+ mockedHandler
+ .When(r => r.RequestUri!.AbsolutePath.Contains("resource-that-should-not-exist"))
+ .RespondWith(HttpStatusCode.Redirect)
+ .WithHeader("Location", "http://localhost:5000/some-path")
+ .WithContent(new StringContent(""));
+
+ mockedHandler
+ .When(r => r.RequestUri!.AbsolutePath == "/.well-known/change-password")
+ .RespondWith(HttpStatusCode.Redirect)
+ .WithHeader("Location", "http://localhost:5000/version")
+ .WithContent(new StringContent(""));
+
+ var mockHttpFactory = Substitute.For();
+ mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
+
+ var service = new ChangePasswordUriService(mockHttpFactory, uriService);
+
+ var result = await service.GetChangePasswordUri("https://attacker.com");
+
+ Assert.Null(result);
+ }
}