Security: Prevent DNS rebinding in link checker - refs GHSA-ggx3-h674-xqq3

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<int, mixed>
+     */
+    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,
+        ];
     }
 
     /**

tests/CoreBundle/Controller/CheckCLinkActionTest.php

@@ -0,0 +1,112 @@
+<?php
+
+declare(strict_types=1);
+
+/* For licensing terms, see /license.txt */
+
+namespace Chamilo\Tests\CoreBundle\Controller;
+
+use Chamilo\CoreBundle\Controller\Api\CheckCLinkAction;
+use PHPUnit\Framework\TestCase;
+use ReflectionMethod;
+
+use const CURLOPT_FOLLOWLOCATION;
+use const CURLOPT_NOPROXY;
+use const CURLOPT_PROXY;
+use const CURLOPT_RESOLVE;
+use const CURLOPT_URL;
+
+final class CheckCLinkActionTest extends TestCase
+{
+    /**
+     * DNS rebinding must not allow internal targets through the first validation step.
+     */
+    public function testRejectsPrivateAndInvalidTargets(): void
+    {
+        foreach ([
+            'http://127.0.0.1/',
+            'http://10.1.2.3/',
+            'http://169.254.169.254/latest/meta-data/',
+            'http://100.64.1.1/',
+            'http://[::1]/',
+            'http://[::ffff:127.0.0.1]/',
+            'http://localhost/',
+            'https://user:password@example.com/',
+            'file:///etc/passwd',
+            'http://8.8.8.8:0/',
+            'http://exämple.com/',
+            'http://example.com\\\@127.0.0.1/',
+        ] as $url) {
+            self::assertNull($this->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);
+    }
+}