From ac7b9bb13915d2b5e8607405cf41834ef73c3d10 Mon Sep 17 00:00:00 2001 From: Christian Beeznest Date: Sun, 18 Oct 2026 18:37:44 -0500 Subject: [PATCH] Security: Prevent DNS rebinding in link checker --- .../Controller/Api/CheckCLinkAction.php | 121 +++++++++++++----- .../Controller/CheckCLinkActionTest.php | 112 ++++++++++++++++ 2 files changed, 204 insertions(+), 29 deletions(-) create mode 100644 tests/CoreBundle/Controller/CheckCLinkActionTest.php diff --git a/src/CoreBundle/Controller/Api/CheckCLinkAction.php b/src/CoreBundle/Controller/Api/CheckCLinkAction.php index dd156b31332..1fab235c263 100644 --- a/src/CoreBundle/Controller/Api/CheckCLinkAction.php +++ b/src/CoreBundle/Controller/Api/CheckCLinkAction.php @@ -15,10 +15,11 @@ use const CURLINFO_HTTP_CODE; use const CURLOPT_FOLLOWLOCATION; use const CURLOPT_HEADER; +use const CURLOPT_NOPROXY; use const CURLOPT_PROTOCOLS; use const CURLOPT_PROXY; -use const CURLOPT_PROXYPORT; use const CURLOPT_REDIR_PROTOCOLS; +use const CURLOPT_RESOLVE; use const CURLOPT_RETURNTRANSFER; use const CURLOPT_TIMEOUT; use const CURLOPT_URL; @@ -54,39 +55,36 @@ private function checkUrl(string $url, SettingsManager $settingsManager): bool return false; } - $normalizedUrl = $this->normalizeAndValidateUrl($url); - if (null === $normalizedUrl) { + $target = $this->normalizeAndValidateUrl($url); + if (null === $target) { return false; } - $defaults = [ - CURLOPT_URL => $normalizedUrl, - CURLOPT_FOLLOWLOCATION => false, // Avoid SSRF via redirects - CURLOPT_HEADER => 0, - CURLOPT_RETURNTRANSFER => true, - CURLOPT_TIMEOUT => 4, - CURLOPT_PROTOCOLS => CURLPROTO_HTTP | CURLPROTO_HTTPS, - ]; - - if (\defined('CURLOPT_REDIR_PROTOCOLS')) { - $defaults[CURLOPT_REDIR_PROTOCOLS] = CURLPROTO_HTTP | CURLPROTO_HTTPS; - } - - // Optional proxy settings + // A proxy can resolve the hostname itself, bypassing CURLOPT_RESOLVE. + // Fail closed rather than sending user-controlled URLs through a proxy. $proxySettings = $settingsManager->getSetting('security.proxy_settings', true); - if (\is_array($proxySettings) && isset($proxySettings['curl_setopt_array']) && \is_array($proxySettings['curl_setopt_array'])) { - $opts = $proxySettings['curl_setopt_array']; + if (\is_array($proxySettings) + && isset($proxySettings['curl_setopt_array']) + && \is_array($proxySettings['curl_setopt_array']) + && !empty($proxySettings['curl_setopt_array']['CURLOPT_PROXY']) + ) { + return false; + } - if (isset($opts['CURLOPT_PROXY'])) { - $defaults[CURLOPT_PROXY] = $opts['CURLOPT_PROXY']; - } - if (isset($opts['CURLOPT_PROXYPORT'])) { - $defaults[CURLOPT_PROXYPORT] = $opts['CURLOPT_PROXYPORT']; - } + if (!\defined('CURLOPT_RESOLVE')) { + return false; } $ch = curl_init(); - curl_setopt_array($ch, $defaults); + if (false === $ch) { + return false; + } + + if (!curl_setopt_array($ch, $this->createCurlOptions($target))) { + curl_close($ch); + + return false; + } $result = curl_exec($ch); $httpCode = (int) curl_getinfo($ch, CURLINFO_HTTP_CODE); @@ -101,10 +99,50 @@ private function checkUrl(string $url, SettingsManager $settingsManager): bool return $httpCode >= 200 && $httpCode < 400; } - private function normalizeAndValidateUrl(string $url): ?string + /** + * Keep the original URL for Host, SNI and certificate verification while + * forcing the connection to the IP already checked by the SSRF filter. + * + * @param array{url: string, host: string, port: int, ip: string, literal: bool} $target + * + * @return array + */ + private function createCurlOptions(array $target): array + { + $options = [ + CURLOPT_URL => $target['url'], + CURLOPT_FOLLOWLOCATION => false, + CURLOPT_HEADER => 0, + CURLOPT_RETURNTRANSFER => true, + CURLOPT_TIMEOUT => 4, + CURLOPT_PROTOCOLS => CURLPROTO_HTTP | CURLPROTO_HTTPS, + CURLOPT_PROXY => '', // Also disable proxy environment variables. + CURLOPT_NOPROXY => '*', + ]; + + if (\defined('CURLOPT_REDIR_PROTOCOLS')) { + $options[CURLOPT_REDIR_PROTOCOLS] = CURLPROTO_HTTP | CURLPROTO_HTTPS; + } + + if (!$target['literal']) { + $ip = $target['ip']; + if (str_contains($ip, ':')) { + $ip = '['.$ip.']'; + } + + $options[CURLOPT_RESOLVE] = [$target['host'].':'.$target['port'].':'.$ip]; + } + + return $options; + } + + /** + * @return array{url: string, host: string, port: int, ip: string, literal: bool}|null + */ + private function normalizeAndValidateUrl(string $url): ?array { $url = trim($url); - if ('' === $url) { + if ('' === $url || str_contains($url, '\\') || preg_match('/[\x00-\x1F\x7F]/', $url)) { return null; } @@ -123,6 +161,19 @@ private function normalizeAndValidateUrl(string $url): ?string return null; } + // parse_url() retains brackets around IPv6 literals. + if (str_starts_with($host, '[') && str_ends_with($host, ']')) { + $host = substr($host, 1, -1); + if (false === filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + return null; + } + } + + $port = $parts['port'] ?? ('https' === $scheme ? 443 : 80); + if (!\is_int($port) || $port < 1 || $port > 65535) { + return null; + } + // Block credentials in URL: http://user:pass@host/ if (isset($parts['user']) || isset($parts['pass'])) { return null; @@ -133,6 +184,12 @@ private function normalizeAndValidateUrl(string $url): ?string return null; } + // Reject non-canonical hostnames that PHP and cURL may parse differently. + $isIpLiteral = false !== filter_var($host, FILTER_VALIDATE_IP); + if (!$isIpLiteral && !preg_match('/\A[a-z0-9.-]+\z/i', $host)) { + return null; + } + // Resolve and block private/reserved ranges to mitigate SSRF. $ips = $this->resolveHostIps($host); if ([] === $ips) { @@ -145,7 +202,13 @@ private function normalizeAndValidateUrl(string $url): ?string } } - return $url; + return [ + 'url' => $url, + 'host' => $host, + 'port' => $port, + 'ip' => $ips[0], + 'literal' => $isIpLiteral, + ]; } /** diff --git a/tests/CoreBundle/Controller/CheckCLinkActionTest.php b/tests/CoreBundle/Controller/CheckCLinkActionTest.php new file mode 100644 index 00000000000..3b380173eb3 --- /dev/null +++ b/tests/CoreBundle/Controller/CheckCLinkActionTest.php @@ -0,0 +1,112 @@ +validate($url), $url); + } + } + + public function testAcceptsPublicIpLiteralsWithoutAnotherDnsLookup(): void + { + self::assertSame( + ['url' => 'https://8.8.8.8:8443/test', 'host' => '8.8.8.8', 'port' => 8443, 'ip' => '8.8.8.8', 'literal' => true], + $this->validate('https://8.8.8.8:8443/test'), + ); + + self::assertSame( + ['url' => 'https://[2606:4700:4700::1111]/', 'host' => '2606:4700:4700::1111', 'port' => 443, 'ip' => '2606:4700:4700::1111', 'literal' => true], + $this->validate('https://[2606:4700:4700::1111]/'), + ); + } + + /** + * The original hostname must be retained for Host/SNI and TLS verification, + * but cURL must connect to the checked IP, never resolve the name again. + */ + public function testPinsHostnameAndPortAndDisablesProxies(): void + { + if (!\extension_loaded('curl')) { + self::markTestSkipped('Requires ext-curl.'); + } + + $options = $this->curlOptions([ + 'url' => 'https://example.com:8443/path', + 'host' => 'example.com', + 'port' => 8443, + 'ip' => '8.8.8.8', + 'literal' => false, + ]); + + self::assertSame('https://example.com:8443/path', $options[CURLOPT_URL]); + self::assertSame(['example.com:8443:8.8.8.8'], $options[CURLOPT_RESOLVE]); + self::assertFalse($options[CURLOPT_FOLLOWLOCATION]); + self::assertSame('', $options[CURLOPT_PROXY]); + self::assertSame('*', $options[CURLOPT_NOPROXY]); + } + + public function testPinsIpv6DnsAnswersWithCurlBracketSyntax(): void + { + if (!\extension_loaded('curl')) { + self::markTestSkipped('Requires ext-curl.'); + } + + $options = $this->curlOptions([ + 'url' => 'https://example.com/', + 'host' => 'example.com', + 'port' => 443, + 'ip' => '2606:4700:4700::1111', + 'literal' => false, + ]); + + self::assertSame(['example.com:443:[2606:4700:4700::1111]'], $options[CURLOPT_RESOLVE]); + } + + private function validate(string $url): ?array + { + $method = new ReflectionMethod(CheckCLinkAction::class, 'normalizeAndValidateUrl'); + + return $method->invoke(new CheckCLinkAction(), $url); + } + + private function curlOptions(array $target): array + { + $method = new ReflectionMethod(CheckCLinkAction::class, 'createCurlOptions'); + + return $method->invoke(new CheckCLinkAction(), $target); + } +}