prevent SSRF by blocking redirects from public websites to local addresses

commafeed-server/src/main/java/com/commafeed/backend/HttpClientFactory.java

@@ -1,8 +1,8 @@
 package com.commafeed.backend;
 
 import java.net.InetAddress;
+import java.net.URI;
 import java.net.UnknownHostException;
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -18,13 +18,19 @@ import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.client5.http.entity.DeflateInputStream;
 import org.apache.hc.client5.http.entity.InputStreamFactory;
 import org.apache.hc.client5.http.entity.compress.ContentCoding;
+import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
 import org.apache.hc.client5.http.io.HttpClientConnectionManager;
+import org.apache.hc.client5.http.protocol.RedirectStrategy;
 import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
 import org.brotli.dec.BrotliInputStream;
@@ -41,6 +47,8 @@ import nl.altindag.ssl.apache5.util.Apache5SslUtils;
 @RequiredArgsConstructor
 public class HttpClientFactory {
 
+	private static final DnsResolver DNS_RESOLVER = SystemDefaultDnsResolver.INSTANCE;
+
 	private final CommaFeedConfiguration config;
 	private final CommaFeedVersion version;
 
@@ -49,11 +57,10 @@ public class HttpClientFactory {
 		String userAgent = config.httpClient()
 				.userAgent()
 				.orElseGet(() -> String.format("CommaFeed/%s (https://github.com/Athou/commafeed)", version.getVersion()));
-		return newClient(connectionManager, userAgent, config.httpClient().idleConnectionsEvictionInterval());
+		return newClient(config, connectionManager, userAgent);
 	}
 
-	private CloseableHttpClient newClient(HttpClientConnectionManager connectionManager, String userAgent,
+	private CloseableHttpClient newClient(CommaFeedConfiguration config, HttpClientConnectionManager connectionManager, String userAgent) {
-			Duration idleConnectionsEvictionInterval) {
 		List<Header> headers = new ArrayList<>();
 		headers.add(new BasicHeader(HttpHeaders.ACCEPT_LANGUAGE, "en"));
 		headers.add(new BasicHeader(HttpHeaders.PRAGMA, "No-cache"));
@@ -64,6 +71,10 @@ public class HttpClientFactory {
 		contentDecoderMap.put(ContentCoding.DEFLATE.token(), DeflateInputStream::new);
 		contentDecoderMap.put(ContentCoding.BROTLI.token(), BrotliInputStream::new);
 
+		RedirectStrategy redirectStrategy = config.httpClient().blockLocalAddresses()
+				? new BlockLocalAddressesRedirectStrategy(DNS_RESOLVER)
+				: new DefaultRedirectStrategy();
+
 		return HttpClientBuilder.create()
 				.useSystemProperties()
 				.disableAutomaticRetries()
@@ -72,16 +83,16 @@ public class HttpClientFactory {
 				.setDefaultHeaders(headers)
 				.setConnectionManager(connectionManager)
 				.evictExpiredConnections()
-				.evictIdleConnections(TimeValue.of(idleConnectionsEvictionInterval))
+				.evictIdleConnections(TimeValue.of(config.httpClient().idleConnectionsEvictionInterval()))
 				.setContentDecoderRegistry(new LinkedHashMap<>(contentDecoderMap))
+				.setRedirectStrategy(redirectStrategy)
 				.build();
 	}
 
 	private PoolingHttpClientConnectionManager newConnectionManager(CommaFeedConfiguration config, int poolSize) {
 		SSLFactory sslFactory = SSLFactory.builder().withUnsafeTrustMaterial().withUnsafeHostnameVerifier().build();
-		DnsResolver dnsResolver = config.httpClient().blockLocalAddresses()
+		DnsResolver dnsResolver = config.httpClient().blockLocalAddresses() ? new BlockLocalAddressesDnsResolver(DNS_RESOLVER)
-				? new BlockLocalAddressesDnsResolver(SystemDefaultDnsResolver.INSTANCE)
+				: DNS_RESOLVER;
-				: SystemDefaultDnsResolver.INSTANCE;
 
 		return PoolingHttpClientConnectionManagerBuilder.create()
 				.setTlsSocketStrategy(Apache5SslUtils.toTlsSocketStrategy(sslFactory))
@@ -98,13 +109,18 @@ public class HttpClientFactory {
 
 	}
 
+	private static boolean isLocalAddress(InetAddress address) {
+		return address.isSiteLocalAddress() || address.isAnyLocalAddress() || address.isLinkLocalAddress() || address.isLoopbackAddress()
+				|| address.isMulticastAddress();
+	}
+
 	private record BlockLocalAddressesDnsResolver(DnsResolver delegate) implements DnsResolver {
 		@Override
 		public InetAddress[] resolve(String host) throws UnknownHostException {
 			InetAddress[] addresses = delegate.resolve(host);
 			for (InetAddress addr : addresses) {
 				if (isLocalAddress(addr)) {
-					throw new UnknownHostException("Access to address blocked: " + addr.getHostAddress());
+					throw new UnknownHostException("Access to local address blocked: " + addr.getHostAddress());
 				}
 			}
 			return addresses;
@@ -114,10 +130,36 @@ public class HttpClientFactory {
 		public String resolveCanonicalHostname(String host) throws UnknownHostException {
 			return delegate.resolveCanonicalHostname(host);
 		}
+	}
 
-		private boolean isLocalAddress(InetAddress address) {
+	@RequiredArgsConstructor
-			return address.isSiteLocalAddress() || address.isAnyLocalAddress() || address.isLinkLocalAddress()
+	private static class BlockLocalAddressesRedirectStrategy extends DefaultRedirectStrategy {
-					|| address.isLoopbackAddress() || address.isMulticastAddress();
+
+		private final DnsResolver delegate;
+
+		@Override
+		public URI getLocationURI(HttpRequest request, HttpResponse response, HttpContext context) throws HttpException {
+			URI redirectUri = super.getLocationURI(request, response, context);
+
+			String host = redirectUri.getHost();
+			if (host == null) {
+				throw new HttpException("Redirect URI does not have a host: " + redirectUri);
+			}
+
+			InetAddress[] addresses;
+			try {
+				addresses = delegate.resolve(host);
+			} catch (UnknownHostException e) {
+				throw new HttpException("Unknown host: " + host);
+			}
+
+			for (InetAddress addr : addresses) {
+				if (isLocalAddress(addr)) {
+					throw new HttpException("Access to local address blocked: " + addr.getHostAddress());
+				}
+			}
+
+			return redirectUri;
 		}
 	}