From 90ea7d6bb4c74ca169ee1dde3ca38b08bd0966ed Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Sun, 2 Aug 2026 19:23:16 +0200 Subject: [PATCH] Resolve favicon extraction target DNS host once per connection (#2310) --- .../Utilities/FaviconExtractorTests.cs | 15 +++ .../AliasVault.FaviconExtractor.csproj | 4 + .../FaviconExtractor.cs | 115 +++++++++++------- 3 files changed, 93 insertions(+), 41 deletions(-) diff --git a/apps/server/Tests/AliasVault.UnitTests/Utilities/FaviconExtractorTests.cs b/apps/server/Tests/AliasVault.UnitTests/Utilities/FaviconExtractorTests.cs index 64a1bb383..0d951b158 100644 --- a/apps/server/Tests/AliasVault.UnitTests/Utilities/FaviconExtractorTests.cs +++ b/apps/server/Tests/AliasVault.UnitTests/Utilities/FaviconExtractorTests.cs @@ -7,6 +7,8 @@ namespace AliasVault.UnitTests.Utilities; +using System.Net; + /// /// Tests for the AliasVault.FaviconExtractor class. /// @@ -91,4 +93,17 @@ public class FaviconExtractorTests Assert.That(faviconBytes, Is.Null, $"Should block non-standard port URL: {url}"); } } + + /// + /// Check that a mixed set of public and private IP addresses is rejected as only fully public sets are allowed. + /// + [Test] + public void RejectsAddressSetMixingPublicAndPrivate() + { + Assert.Multiple(() => + { + Assert.That(FaviconExtractor.FaviconExtractor.AreIpAddressesPublic([IPAddress.Parse("8.8.8.8"), IPAddress.Parse("10.0.0.1")]), Is.False, "A mixed set must be rejected"); + Assert.That(FaviconExtractor.FaviconExtractor.AreIpAddressesPublic([IPAddress.Parse("8.8.8.8"), IPAddress.Parse("1.1.1.1")]), Is.True, "An all-public set must be allowed"); + }); + } } diff --git a/apps/server/Utilities/AliasVault.FaviconExtractor/AliasVault.FaviconExtractor.csproj b/apps/server/Utilities/AliasVault.FaviconExtractor/AliasVault.FaviconExtractor.csproj index 95cb6fedf..3a66883cf 100644 --- a/apps/server/Utilities/AliasVault.FaviconExtractor/AliasVault.FaviconExtractor.csproj +++ b/apps/server/Utilities/AliasVault.FaviconExtractor/AliasVault.FaviconExtractor.csproj @@ -17,6 +17,10 @@ bin\Release\net10.0\FaviconExtractor.xml + + + + diff --git a/apps/server/Utilities/AliasVault.FaviconExtractor/FaviconExtractor.cs b/apps/server/Utilities/AliasVault.FaviconExtractor/FaviconExtractor.cs index faf77acf6..3e51ab8f5 100644 --- a/apps/server/Utilities/AliasVault.FaviconExtractor/FaviconExtractor.cs +++ b/apps/server/Utilities/AliasVault.FaviconExtractor/FaviconExtractor.cs @@ -9,10 +9,13 @@ namespace AliasVault.FaviconExtractor; using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Net; using System.Net.Http; +using System.Net.Sockets; using System.Text; +using System.Threading; using System.Threading.Tasks; using HtmlAgilityPack; using SkiaSharp; @@ -88,22 +91,30 @@ public static class FaviconExtractor url = NormalizeUrl(url); Uri uri = new(url); - if (!IsValidUri(uri)) + if (!IsAllowedSchemeAndPort(uri)) { return null; } using HttpClient client = CreateHttpClient(); - // Attempt the operation up to two times to handle common cookiewall redirects or transient issues. - for (int attempt = 0; attempt < 2; attempt++) + try { - var result = await TryGetFaviconAsync(client, uri); - if (result != null) + // Attempt the operation up to two times to handle common cookiewall redirects or transient issues. + for (int attempt = 0; attempt < 2; attempt++) { - return result; + var result = await TryGetFaviconAsync(client, uri); + if (result != null) + { + return result; + } } } + catch (HttpRequestException) + { + // Abort on any request exception. + return null; + } // Return null if the favicon extraction failed. return null; @@ -205,6 +216,51 @@ public static class FaviconExtractor return ImageFormatSignature.Unknown; } + /// + /// Checks whether a set of IP addresses are all publicly routable. + /// + /// The addresses returned by one DNS resolution. + /// True if all addresses are publicly routable, false otherwise. + internal static bool AreIpAddressesPublic(IPAddress[] addresses) + { + foreach (var address in addresses) + { + if (!IPAddressValidator.IsPublicIPAddress(address)) + { + return false; + } + } + + return true; + } + + /// + /// Opens the TCP connection for an outgoing request and resolve a hostname only once. + /// + /// Connection context supplied by the HTTP stack. + /// Token to cancel the connection attempt. + /// A stream over the established connection. + private static async ValueTask ConnectToValidatedAddressAsync(SocketsHttpConnectionContext context, CancellationToken cancellationToken) + { + var addresses = await Dns.GetHostAddressesAsync(context.DnsEndPoint.Host, cancellationToken); + if (!AreIpAddressesPublic(addresses)) + { + throw new HttpRequestException("Blocked connection to a non-public address."); + } + + var socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true }; + try + { + await socket.ConnectAsync(addresses, context.DnsEndPoint.Port, cancellationToken); + return new NetworkStream(socket, ownsSocket: true); + } + catch + { + socket.Dispose(); + throw; + } + } + private static async Task SafeGetFaviconAsync(string url) { try @@ -320,7 +376,7 @@ public static class FaviconExtractor try { // Validate the favicon URL before fetching - if (!Uri.TryCreate(url, UriKind.Absolute, out var faviconUri) || !IsValidUri(faviconUri)) + if (!Uri.TryCreate(url, UriKind.Absolute, out var faviconUri) || !IsAllowedSchemeAndPort(faviconUri)) { return null; } @@ -374,12 +430,13 @@ public static class FaviconExtractor /// The HTTP client. private static HttpClient CreateHttpClient() { - var handler = new HttpClientHandler + var handler = new SocketsHttpHandler { AllowAutoRedirect = false, // Handle redirects manually UseCookies = true, // Enable cookie handling for session management CookieContainer = new System.Net.CookieContainer(), AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate | System.Net.DecompressionMethods.Brotli, + ConnectCallback = ConnectToValidatedAddressAsync, }; var client = new HttpClient(handler) @@ -447,37 +504,13 @@ public static class FaviconExtractor } /// - /// Checks if the URI is valid and not pointing to internal/private IPs. + /// Checks the URI scheme and port. /// /// The URI to check. - /// True if the URI is valid and safe, false otherwise. - private static bool IsValidUri(Uri uri) + /// True if the scheme and port are both allowed, false otherwise. + private static bool IsAllowedSchemeAndPort(Uri uri) { - // Check scheme and port - if (!_allowedSchemes.Contains(uri.Scheme) || !uri.IsDefaultPort) - { - return false; - } - - // Resolve hostname to IP and validate - try - { - var addresses = Dns.GetHostAddresses(uri.Host); - foreach (var address in addresses) - { - if (!IPAddressValidator.IsPublicIPAddress(address)) - { - return false; - } - } - } - catch - { - // If DNS resolution fails, block the request - return false; - } - - return true; + return _allowedSchemes.Contains(uri.Scheme) && uri.IsDefaultPort; } /// @@ -498,12 +531,12 @@ public static class FaviconExtractor var request = new HttpRequestMessage(HttpMethod.Get, currentUri); if (redirectCount == 0) { - // First request - add Google referer to appear like navigation + // First request: add Google referer to appear like navigation request.Headers.Add("Referer", "https://www.google.com/"); } else { - // Subsequent redirects - use original URL as referer + // Subsequent redirects: use original URL as referer request.Headers.Add("Referer", uri.ToString()); } @@ -523,10 +556,10 @@ public static class FaviconExtractor location = new Uri(currentUri, location); } - // Validate the redirect target - if (!IsValidUri(location)) + // Validate the target URL scheme and port. + if (!IsAllowedSchemeAndPort(location)) { - return null; // Block redirect to internal IPs + return null; } currentUri = location;