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;