[5.x] Harden remote URL validation

src/Imaging/GuzzleAdapter.php

@@ -6,6 +6,8 @@
 use GuzzleHttp\ClientInterface;
 use GuzzleHttp\Exception\BadResponseException;
 use GuzzleHttp\Exception\ClientException;
+use GuzzleHttp\Psr7\Uri;
+use GuzzleHttp\Psr7\UriResolver;
 use League\Flysystem\Config;
 use League\Flysystem\FileAttributes;
 use League\Flysystem\FilesystemAdapter;
@@ -14,6 +16,13 @@
 
 class GuzzleAdapter implements FilesystemAdapter
 {
+    /**
+     * The maximum number of redirects to follow.
+     *
+     * @var int
+     */
+    const MAX_REDIRECTS = 5;
+
     /**
      * Whether this endpoint supports head requests.
      *
@@ -149,18 +158,12 @@ public function visibility(string $path): FileAttributes
     protected function get($path)
     {
         try {
-            $response = $this->client->get($this->base.$path, $this->requestOptions());
-        } catch (InvalidRemoteUrlException $e) {
-            throw $e;
+            $response = $this->send('GET', $this->base.$path);
         } catch (BadResponseException $e) {
             return false;
         }
 
-        if ($response->getStatusCode() !== 200) {
-            return false;
-        }
-
-        return $response;
+        return $response->getStatusCode() === 200 ? $response : false;
     }
 
     /**
@@ -176,7 +179,7 @@ protected function head($path)
         }
 
         try {
-            $response = $this->client->head($this->base.$path, $this->requestOptions());
+            $response = $this->send('HEAD', $this->base.$path);
         } catch (ClientException $e) {
             if ($e->getResponse()->getStatusCode() === 405) {
                 $this->supportsHead = false;
@@ -185,27 +188,54 @@ protected function head($path)
             }
 
             return false;
-        } catch (InvalidRemoteUrlException $e) {
-            throw $e;
         } catch (BadResponseException $e) {
             return false;
         }
 
-        if ($response->getStatusCode() !== 200) {
-            return false;
+        return $response->getStatusCode() === 200 ? $response : false;
+    }
+
+    /**
+     * Send a request, pinning the connection to the validated IP so the host
+     * cannot be rebound to an internal address between validation and the
+     * actual fetch. Redirects are followed manually so each hop is validated
+     * and pinned too.
+     */
+    protected function send(string $method, string $url, int $redirects = 0)
+    {
+        // The connection is pinned to the validated IP via curl's CURLOPT_RESOLVE,
+        // which the stream handler has no equivalent for. Rather than silently fall
+        // back to an unpinned (rebindable) request, refuse to fetch without curl.
+        if (! $this->supportsConnectionPinning()) {
+            throw new \RuntimeException('The curl PHP extension is required to fetch remote images.');
+        }
+
+        $resolved = app(RemoteUrlValidator::class)->resolve($url);
+
+        $response = $this->client->request($method, $url, [
+            'allow_redirects' => false,
+            'curl' => [
+                CURLOPT_RESOLVE => [sprintf('%s:%d:%s', $resolved['host'], $resolved['port'], implode(',', $resolved['ips']))],
+            ],
+        ]);
+
+        $status = $response->getStatusCode();
+
+        if ($status >= 300 && $status < 400 && $response->hasHeader('Location')) {
+            if ($redirects >= self::MAX_REDIRECTS) {
+                throw new InvalidRemoteUrlException('Too many redirects.');
+            }
+
+            $location = UriResolver::resolve(new Uri($url), new Uri($response->getHeaderLine('Location')));
+
+            return $this->send($method, (string) $location, $redirects + 1);
         }
 
         return $response;
     }
 
-    protected function requestOptions()
+    protected function supportsConnectionPinning(): bool
     {
-        return [
-            'allow_redirects' => [
-                'on_redirect' => function ($request, $response, $uri) {
-                    app(RemoteUrlValidator::class)->validate((string) $uri);
-                },
-            ],
-        ];
+        return extension_loaded('curl');
     }
 }

src/Imaging/RemoteUrlValidator.php

@@ -15,6 +15,39 @@ public function __construct(?callable $resolver = null)
     }
 
     public function parse($url)
+    {
+        $components = $this->validatedComponents($url);
+
+        return [
+            'path' => Str::after($components['path'], '/'),
+            'base' => $components['scheme'].'://'.$components['host'].$components['port_suffix'],
+            'query' => $components['query'],
+        ];
+    }
+
+    public function validate($url)
+    {
+        $this->parse($url);
+    }
+
+    /**
+     * Resolve and validate the URL host, returning the host, port, and the
+     * validated public IPs it resolves to. These IPs are intended to be pinned
+     * to the actual connection so the host cannot be rebound to an internal
+     * address between this check and the request being made.
+     */
+    public function resolve($url)
+    {
+        $components = $this->validatedComponents($url);
+
+        return [
+            'host' => $components['host'],
+            'port' => $components['port'],
+            'ips' => $components['ips'],
+        ];
+    }
+
+    protected function validatedComponents($url)
     {
         $parsed = parse_url($url);
 
@@ -48,22 +81,19 @@ public function parse($url)
             throw new InvalidRemoteUrlException('Invalid URL host.');
         }
 
-        $this->ensureHostResolvesToPublicIps($host);
-
-        $port = isset($parsed['port']) ? ':'.$parsed['port'] : '';
+        $ips = $this->ensureHostResolvesToPublicIps($host);
 
         return [
-            'path' => Str::after($parsed['path'] ?? '/', '/'),
-            'base' => $scheme.'://'.$host.$port,
+            'scheme' => $scheme,
+            'host' => $host,
+            'port' => $parsed['port'] ?? ($scheme === 'https' ? 443 : 80),
+            'port_suffix' => isset($parsed['port']) ? ':'.$parsed['port'] : '',
+            'path' => $parsed['path'] ?? '/',
             'query' => $parsed['query'] ?? null,
+            'ips' => $ips,
         ];
     }
 
-    public function validate($url)
-    {
-        $this->parse($url);
-    }
-
     protected function isValidHost($host)
     {
         return filter_var($host, FILTER_VALIDATE_IP)
@@ -75,7 +105,7 @@ protected function ensureHostResolvesToPublicIps($host)
         if (filter_var($host, FILTER_VALIDATE_IP)) {
             $this->assertPublicIp($host);
 
-            return;
+            return [$host];
         }
 
         $records = call_user_func($this->resolver, $host);
@@ -90,6 +120,8 @@ protected function ensureHostResolvesToPublicIps($host)
         foreach ($ips as $ip) {
             $this->assertPublicIp($ip);
         }
+
+        return $ips;
     }
 
     protected function assertPublicIp($ip)

tests/Imaging/GuzzleAdapterTest.php

@@ -3,9 +3,7 @@
 namespace Tests\Imaging;
 
 use GuzzleHttp\ClientInterface;
-use GuzzleHttp\Psr7\Request;
 use GuzzleHttp\Psr7\Response;
-use GuzzleHttp\Psr7\Uri;
 use Mockery;
 use PHPUnit\Framework\Attributes\Test;
 use Statamic\Exceptions\InvalidRemoteUrlException;
@@ -23,48 +21,88 @@ public function setUp(): void
             return new RemoteUrlValidator(function ($host) {
                 return match ($host) {
                     'example.com' => [['ip' => '93.184.216.34']],
+                    'cdn.example.com' => [['ip' => '93.184.216.35']],
                     default => [],
                 };
             });
         });
     }
 
     #[Test]
-    public function it_allows_redirects_when_every_hop_is_public()
+    public function it_pins_the_connection_to_the_validated_ip()
     {
         $client = Mockery::mock(ClientInterface::class);
-        $client->shouldReceive('get')->once()->andReturnUsing(function ($url, $options) {
-            $this->assertEquals('https://example.com/foo.jpg', $url);
-            $this->assertArrayHasKey('allow_redirects', $options);
-            $this->assertArrayHasKey('on_redirect', $options['allow_redirects']);
+        $client->shouldReceive('request')->once()->andReturnUsing(function ($method, $url, $options) {
+            $this->assertSame('GET', $method);
+            $this->assertSame('https://example.com/foo.jpg', $url);
+            $this->assertFalse($options['allow_redirects']);
+            $this->assertSame(['example.com:443:93.184.216.34'], $options['curl'][CURLOPT_RESOLVE]);
 
-            $options['allow_redirects']['on_redirect'](
-                new Request('GET', 'https://example.com/foo.jpg'),
-                new Response(302, ['Location' => 'https://example.com/redirected/foo.jpg']),
-                new Uri('https://example.com/redirected/foo.jpg')
-            );
+            return new Response(200, [], 'image-bytes');
+        });
+
+        $adapter = new GuzzleAdapter('https://example.com', $client);
+
+        $this->assertSame('image-bytes', $adapter->read('foo.jpg'));
+    }
+
+    #[Test]
+    public function it_resolves_the_host_once_and_pins_that_ip()
+    {
+        // DNS rebinding works by returning a public IP for the validation lookup,
+        // then a private one for the connection's lookup. The fix resolves the host
+        // a single time and pins that IP to the connection via CURLOPT_RESOLVE, so
+        // the rebound answer below (127.0.0.1) is never reached. We assert both: the
+        // pin is the public IP, and the resolver is consulted exactly once.
+        $lookups = 0;
+        $resolver = function () use (&$lookups) {
+            return [['ip' => ++$lookups === 1 ? '93.184.216.34' : '127.0.0.1']];
+        };
+
+        $this->app->bind(RemoteUrlValidator::class, fn () => new RemoteUrlValidator($resolver));
+
+        $client = Mockery::mock(ClientInterface::class);
+        $client->shouldReceive('request')->once()->andReturnUsing(function ($method, $url, $options) {
+            $this->assertSame(['example.com:443:93.184.216.34'], $options['curl'][CURLOPT_RESOLVE]);
 
             return new Response(200, [], 'image-bytes');
         });
 
         $adapter = new GuzzleAdapter('https://example.com', $client);
 
+        $this->assertSame('image-bytes', $adapter->read('foo.jpg'));
+        $this->assertSame(1, $lookups, 'The host should be resolved once; the connection must reuse that result, not re-resolve.');
+    }
+
+    #[Test]
+    public function it_follows_redirects_and_pins_every_hop()
+    {
+        $client = Mockery::mock(ClientInterface::class);
+
+        $client->shouldReceive('request')->once()
+            ->with('GET', 'https://example.com/foo.jpg', Mockery::on(function ($options) {
+                return $options['curl'][CURLOPT_RESOLVE] === ['example.com:443:93.184.216.34'];
+            }))
+            ->andReturn(new Response(302, ['Location' => 'https://cdn.example.com/foo.jpg']));
+
+        $client->shouldReceive('request')->once()
+            ->with('GET', 'https://cdn.example.com/foo.jpg', Mockery::on(function ($options) {
+                return $options['curl'][CURLOPT_RESOLVE] === ['cdn.example.com:443:93.184.216.35'];
+            }))
+            ->andReturn(new Response(200, [], 'image-bytes'));
+
+        $adapter = new GuzzleAdapter('https://example.com', $client);
+
         $this->assertSame('image-bytes', $adapter->read('foo.jpg'));
     }
 
     #[Test]
     public function it_blocks_redirects_to_non_public_destinations()
     {
         $client = Mockery::mock(ClientInterface::class);
-        $client->shouldReceive('get')->once()->andReturnUsing(function ($url, $options) {
-            $options['allow_redirects']['on_redirect'](
-                new Request('GET', 'https://example.com/foo.jpg'),
-                new Response(302, ['Location' => 'http://169.254.169.254/latest/meta-data/']),
-                new Uri('http://169.254.169.254/latest/meta-data/')
-            );
-
-            return new Response(200, [], 'should-not-return');
-        });
+        $client->shouldReceive('request')->once()->andReturn(
+            new Response(302, ['Location' => 'http://169.254.169.254/latest/meta-data/'])
+        );
 
         $adapter = new GuzzleAdapter('https://example.com', $client);
 
@@ -73,4 +111,41 @@ public function it_blocks_redirects_to_non_public_destinations()
 
         $adapter->read('foo.jpg');
     }
+
+    #[Test]
+    public function it_refuses_to_fetch_when_curl_is_unavailable()
+    {
+        // Without curl the connection can't be pinned to the validated IP, so rather
+        // than silently fall back to a rebindable request we refuse to fetch at all.
+        $client = Mockery::mock(ClientInterface::class);
+        $client->shouldNotReceive('request');
+
+        $adapter = new class('https://example.com', $client) extends GuzzleAdapter
+        {
+            protected function supportsConnectionPinning(): bool
+            {
+                return false;
+            }
+        };
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('curl PHP extension is required');
+
+        $adapter->read('foo.jpg');
+    }
+
+    #[Test]
+    public function it_stops_following_redirects_after_the_limit()
+    {
+        $client = Mockery::mock(ClientInterface::class);
+        $client->shouldReceive('request')
+            ->andReturn(new Response(302, ['Location' => 'https://example.com/foo.jpg']));
+
+        $adapter = new GuzzleAdapter('https://example.com', $client);
+
+        $this->expectException(InvalidRemoteUrlException::class);
+        $this->expectExceptionMessage('Too many redirects.');
+
+        $adapter->read('foo.jpg');
+    }
 }