Fix regression with HTTPS proxies due to wrong TLS SNI resolution (#9341)

Fixes #9314
More details: https://github.com/FreshRSS/FreshRSS/issues/9314#issuecomment-5750644306

Short version is that I forgot to consider CURLOPT_RESOLVE, since I was focused on making sure that socks4a and socks5h behaved correctly.

SimplePie fork PR: https://github.com/FreshRSS/simplepie/pull/91

(there would be problems only with some HTTPS proxies, such as the one described in the issue)

app/Models/SimplePieFetch.php

@@ -45,8 +45,8 @@ final class FreshRSS_SimplePieFetch extends \SimplePie\File
 	}
 
 	#[\Override]
-	protected function get_curl_resolve_info(string $url, bool $for_proxy = false): array|string|null|false {
-		return FreshRSS_http_Util::getCurlResolveInfo($url, $for_proxy);
+	protected function get_curl_resolve_info(string $url): array|null|false {
+		return FreshRSS_http_Util::getCurlResolveInfo($url);
 	}
 
 	#[\Override]

app/Utils/httpUtil.php

@@ -308,17 +308,12 @@ final class FreshRSS_http_Util {
 	/**
 	 * Returns a value for CURLOPT_RESOLVE as an array, null if no allowed IPs were found, false if the domain failed to resolve.
 	 *
-	 * Can also be used for checking if the CURLOPT_PROXY value is allowed, by providing a proxy URL with the `for_proxy` parameter set to `true`.
-	 * In that case, a string value will be returned with the hostname resolved to an IP if allowed.
-	 *
-	 * @return array<string>|string|null|false
+	 * @return array<string>|null|false
 	 */
-	public static function getCurlResolveInfo(string $url, bool $for_proxy = false): array|string|null|false {
-		// Parse the original URL first so that credentials keep their original case (only the host is case-insensitive).
-		$parsedOriginal = parse_url($url);
+	public static function getCurlResolveInfo(string $url): array|null|false {
 		$url = strtolower($url);
 		$parsed = parse_url($url);
-		if ($parsed === false || $parsedOriginal === false) {
+		if ($parsed === false) {
 			return false;
 		}
 		$host = $parsed['host'] ?? null;
@@ -326,12 +321,6 @@ final class FreshRSS_http_Util {
 		if ($host === null || $scheme === null) {
 			return false;
 		}
-		$credentials = '';
-		$user = $parsedOriginal['user'] ?? null;
-		$pass = $parsedOriginal['pass'] ?? null;
-		if (is_string($user) && is_string($pass)) {
-			$credentials = "$user:$pass@";
-		}
 		if (str_starts_with($host, '[') && str_ends_with($host, ']')) {
 			if (strlen($host) === 2) {
 				return false;
@@ -357,12 +346,6 @@ final class FreshRSS_http_Util {
 			default => 0,
 		};
 		if (in_array('*', $internal_host_allowlist, true)) {
-			if ($for_proxy) {
-				if (filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false) {
-					return $credentials . "[$host]:$port";
-				}
-				return $credentials . "$host:$port";
-			}
 			return [];	// Disables SSRF checks entirely (unsafe)
 		}
 
@@ -424,20 +407,10 @@ final class FreshRSS_http_Util {
 
 		if (count($ips_ok) > 0) {
 			if (count($records) > 0 || isset(self::$resolve_ok[$host])) {
-				if ($for_proxy) {
-					// $ips_ok[0] is already bracketed when it is an IPv6 address
-					return $credentials . "$ips_ok[0]:$port";
-				}
 				$resolve_str .= implode(',', $ips_ok);
 				return [$resolve_str];
 			}
 			if (filter_var($host, FILTER_VALIDATE_IP) !== false) {
-				if ($for_proxy) {
-					if (filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false) {
-						return $credentials . "[$host]:$port";
-					}
-					return $credentials . "$host:$port";
-				}
 				// No resolve overrides since the URL only contained an IP, not a domain
 				return [];
 			}
@@ -574,7 +547,7 @@ final class FreshRSS_http_Util {
 					return ['body' => '', 'effective_url' => '', 'redirect_count' => 0, 'fail' => true, 'status' => -500, 'error' => ''];
 				}
 				$proxy_url = "$proxy_scheme://$proxy"; // CURLOPT_PROXY ($proxy) is formatted as user:pass@hostname:port, with the part before @ being optional
-				$resolve = self::getCurlResolveInfo($proxy_url, for_proxy: true);
+				$resolve = self::getCurlResolveInfo($proxy_url);
 				if ($resolve === null) {
 					Minz_Log::warning('Failed to fetch this URL, because the proxy’s IP is not in the allowlist [' .
 						\SimplePie\Misc::url_remove_credentials($url) . '] [' .
@@ -583,12 +556,11 @@ final class FreshRSS_http_Util {
 				} elseif ($resolve === false) {
 					return ['body' => '', 'effective_url' => '', 'redirect_count' => 0, 'fail' => true, 'status' => -500, 'error' => ''];
 				}
-				// Translate from a hostname:port value to ip:port, in order to avoid DNS rebinding
-				$curl_options[CURLOPT_PROXY] = $resolve;
-				if (defined('CURLOPT_PROXY_SSL_VERIFYHOST')) {
-					// Skip verifying the hostname (a bit unsafe, but needed since
-					// there is no CURLOPT_RESOLVE equivalent for proxy hostnames)
-					$curl_options[CURLOPT_PROXY_SSL_VERIFYHOST] = 0;
+				if (!empty($resolve)) {
+					// Only for the proxy domain, socks4a and socks5h DNS queries will be passed through the proxy.
+					// For the other proxy protocols, note that IPs for internal domains can be leaked,
+					// since the domain is resolved outside of the proxy.
+					$curl_options[CURLOPT_RESOLVE] = $resolve;
 				}
 			}
 			// TODO: Implement HTTP 1.1 conditional GET If-Modified-Since

lib/composer.json

@@ -18,7 +18,7 @@
 		"marienfressinaud/lib_opml": "dev-main#f0e850b6394af90b898daf0e65fcc7363457b844",
 		"phpgt/cssxpath": "v1.5.0",
 		"phpmailer/phpmailer": "7.1.1",
-		"simplepie/simplepie": "dev-freshrss#ec17fd8af7dc4dc60895b0542d551ea6895e1dbb"
+		"simplepie/simplepie": "dev-freshrss#caa6737811d75301720d07b30d9b53a4ab0b969d"
 	},
 	"config": {
 		"sort-packages": true,

lib/simplepie/simplepie/src/File.php

@@ -160,7 +160,7 @@ class File implements Response
                     }
                     $proxy = preg_replace('#^.*://#i', '', $proxy); // Strip any scheme already present in CURLOPT_PROXY
                     $proxy_url = "$proxy_scheme://$proxy"; // CURLOPT_PROXY ($proxy) is formatted as user:pass@hostname:port, with the part before @ being optional
-                    $resolve = $this->get_curl_resolve_info($proxy_url, true);
+                    $resolve = $this->get_curl_resolve_info($proxy_url);
                     if ($resolve === null) {
                         $this->error = 'Failed to fetch this URL, because the proxy’s IP is not in the allowlist [' .
                             \SimplePie\Misc::url_remove_credentials($url) . '] [' .
@@ -172,10 +172,11 @@ class File implements Response
                         $this->success = false;
                         return;
                     }
-                    $curl_options[CURLOPT_PROXY] = $resolve; // Translate from a hostname:port value to ip:port, in order to prevent DNS rebinding
-                    if (defined('CURLOPT_PROXY_SSL_VERIFYHOST')) {
-                        // Available as of PHP 7.3.0 and cURL 7.52.0
-                        $curl_options[CURLOPT_PROXY_SSL_VERIFYHOST] = 0; // Skip verifying the hostname (a bit unsafe, but needed since there is no CURLOPT_RESOLVE equivalent for proxy hostnames)
+                    if (!empty($resolve)) {
+                        // Only for the proxy domain, socks4a and socks5h DNS queries will be passed through the proxy.
+                        // For the other proxy protocols, note that IPs for internal domains can be leaked,
+                        // since the domain is resolved outside of the proxy.
+                        $curl_options[CURLOPT_RESOLVE] = $resolve;
                     }
                 }
                 $this->method = \SimplePie\SimplePie::FILE_SOURCE_REMOTE | \SimplePie\SimplePie::FILE_SOURCE_CURL;
@@ -495,19 +496,12 @@ class File implements Response
     }
 
     /**
-     * Event to allow inheriting classes to control fetching certain URLs.
+     * Event to allow inheriting classes to control fetching certain website and proxy URLs.
      * @param string $url
-     * @return array<string>|string|null|false Returns a value for CURLOPT_RESOLVE as an array, null if no allowed IPs were found, false if the domain failed to resolve. Can also be used for checking if the CURLOPT_PROXY value is allowed, by providing a proxy URL with the `for_proxy` parameter set to `true`. In that case, a string value will be returned with the hostname resolved to an IP if allowed.
+     * @return array<string>|null|false Returns a value for CURLOPT_RESOLVE as an array, null if no allowed IPs were found, false if the domain failed to resolve.
      */
-    protected function get_curl_resolve_info(string $url, bool $for_proxy = false)
+    protected function get_curl_resolve_info(string $url)
     {
-        if ($for_proxy) {
-            $pos = strpos($url, '://');
-            if ($pos === false) {
-                return false;
-            }
-            return substr($url, $pos + 3);
-        }
         return [];
     }