Add SSRF protections to ChangePasswordUriSErvice

src/Icons/Models/IconUri.cs

@@ -39,7 +39,7 @@ public class IconUri
     }
 
     /// <summary>
-    /// 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.
     /// </summary>
     /// <param name="uriString"></param>
     /// <param name="ip"></param>

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;
     }
 
     /// <summary>
@@ -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;
         }
     }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// <returns>The HTTP response, or null if the URI fails SSRF validation.</returns>
+    private async Task<HttpResponseMessage?> SendSafeRequestAsync(Uri uri)
+    {
+        if (!_uriService.TryGetUri(uri, out var iconUri) || !iconUri!.IsValid)
+        {
+            return null;
+        }
+
+        return await SendWithRedirectsAsync(iconUri, 0);
+    }
+
+    private async Task<HttpResponseMessage?> 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);
+        }
+    }
 }

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,
         });
     }

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<ChangePasswordUriService>
 {
+    private static readonly IPAddress _publicIp = IPAddress.Parse("93.184.216.34");
+    private static readonly IPAddress _loopbackIp = IPAddress.Parse("127.0.0.1");
+
+    /// <summary>
+    /// A fake IUriService that resolves all URIs to the given IP address.
+    /// </summary>
+    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;
+        }
+    }
+
+    /// <summary>
+    /// A fake IUriService that succeeds for initial requests but returns an internal IP for redirects.
+    /// </summary>
+    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<IHttpClientFactory>();
         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<ChangePasswordUriSe
     [InlineData("https://example.com")]
     public async Task GetChangePasswordUri_WhenResourceThatShouldNotExistReturns200_ReturnsNull(string domain)
     {
-        var mockHttpFactory = Substitute.For<IHttpClientFactory>();
         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<IHttpClientFactory>();
+        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<ChangePasswordUriSe
     [InlineData("https://example.com")]
     public async Task GetChangePasswordUri_WhenChangePasswordUrlNotFound_ReturnsNull(string domain)
     {
-        var mockHttpFactory = Substitute.For<IHttpClientFactory>();
         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<IHttpClientFactory>();
+        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<ChangePasswordUriSe
     public async Task GetChangePasswordUri_WhenDomainIsNullOrEmpty_ReturnsNull(string domain)
     {
         var mockHttpFactory = Substitute.For<IHttpClientFactory>();
-        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<IHttpClientFactory>();
+        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<IHttpClientFactory>();
+        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<IHttpClientFactory>();
+        mockHttpFactory.CreateClient("ChangePasswordUri").Returns(mockedHandler.ToHttpClient());
+
+        var service = new ChangePasswordUriService(mockHttpFactory, uriService);
+
+        var result = await service.GetChangePasswordUri("https://attacker.com");
+
+        Assert.Null(result);
+    }
 }