CVE-2024-28235 (PHP, published 2024-04-09T16:15Z)
Description: value: Contao is an open source content management system. Starting in version 4.9.0 and prior to versions 4.13.40 and 5.3.4, when checking for broken links on protected pages, Contao sends the cookie header to external urls as well, the passed  options for the http client are used for all requests. Contao versions 4.13.40 and 5.3.4 have a patch for this issue. As a workaround, disable crawling protected pages.
Severity: nan (CVSS v3 base score: )
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/contao/contao
Files changed:
core-bundle/src/Crawl/Escargot/Factory.php
core-bundle/src/DependencyInjection/ContaoCoreExtension.php
core-bundle/src/Resources/config/services.yml
core-bundle/src/Resources/contao/classes/Crawl.php
core-bundle/tests/Crawl/Escargot/FactoryTest.php
core-bundle/tests/DependencyInjection/ContaoCoreExtensionTest.php
Commit message: Merge pull request from GHSA-9jh5-qf84-x6pr

* Ensure confidential headers are not leaked to external URIs when crawling

* CS

---------

Co-authored-by: Leo Feyer <1192057+leofeyer@users.noreply.github.com>

Vulnerable code:
    public function __construct(Connection $connection, ContaoFramework $framework, array $additionalUris = [], array $defaultHttpClientOptions = [])
        return HttpClient::create(
            array_merge_recursive(
                [
                    'headers' => [
                        'accept' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
                        'user-agent' => self::USER_AGENT,
                    ],
                    'max_duration' => 10, // Ignore requests that take longer than 10 seconds
                array_merge_recursive($this->getDefaultHttpClientOptions(), $options)
            )
        $factory->setArgument(2, $config['crawl']['additional_uris']);
        $factory->setArgument(3, $config['crawl']['default_http_client_options']);

				// Closing the session is necessary here as otherwise we run into our own session lock
				// TODO: we need a way to authenticate with a token instead of our own cookie
				$session->save();
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework());
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework());
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework());
        $this->assertSame(['https://example.com'], $definition->getArgument(2));
        $this->assertSame(['proxy' => 'http://localhost:7080', 'headers' => ['Foo' => 'Bar']], $definition->getArgument(3));

Fixed code:
use Psr\Http\Message\UriInterface;
use Symfony\Component\HttpClient\ScopingHttpClient;
use Symfony\Component\HttpFoundation\RequestStack;
    private RequestStack $requestStack;
    /**
     * @var \Closure(array<string, mixed>): HttpClientInterface|null
     */
    private ?\Closure $httpClientFactory;

    public function __construct(Connection $connection, ContaoFramework $framework, RequestStack $requestStack, array $additionalUris = [], array $defaultHttpClientOptions = [], \Closure $httpClientFactory = null)
        $this->requestStack = $requestStack;
        $this->httpClientFactory = $httpClientFactory ?? static fn (array $defaultOptions) => HttpClient::create($defaultOptions);
        $options = array_merge_recursive(
            [
                'headers' => [
                    'accept' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
                    'user-agent' => self::USER_AGENT,
                'max_duration' => 10, // Ignore requests that take longer than 10 seconds
            ],
            array_merge_recursive($this->getDefaultHttpClientOptions(), $options)

        // Make sure confidential headers force a scoped client so external domains do not leak data
        $cleanOptions = $this->cleanOptionsFromConfidentialData($options);

        if ($options === $cleanOptions) {
            return ($this->httpClientFactory)($options);
        }

        $scopedOptionsByRegex = [];

        // All options including the confidential headers for our root page collection
        foreach ($this->getRootPageUriCollection()->all() as $rootPageUri) {
            $scopedOptionsByRegex[preg_quote($this->getOriginFromUri($rootPageUri))] = $options;
        }

        // Closing the session is necessary here as otherwise we might run into our own session lock
        $request = $this->requestStack->getMainRequest();

        if ($request && $request->hasSession()) {
            $request->getSession()->save();
        }

        return new ScopingHttpClient(($this->httpClientFactory)($cleanOptions), $scopedOptionsByRegex);
    }

    private function getOriginFromUri(UriInterface $uri): string
    {
        $origin = $uri->getScheme().'://'.$uri->getHost();

        if ($uri->getPort()) {
            $origin .= ':'.$uri->getPort();
        }

        return $origin.'/';
    }

    private function cleanOptionsFromConfidentialData(array $options): array
    {
        $cleanOptions = [];

        foreach ($options as $k => $v) {
            if ('headers' === $k) {
                foreach ($v as $header => $value) {
                    if (\in_array(strtolower($header), ['authorization', 'cookie'], true)) {
                        continue;
                    }

                    $cleanOptions['headers'][$header] = $value;
                }

                continue;
            }

            if ('basic_auth' === $k || 'bearer_auth' === $k) {
                continue;
            }

            $cleanOptions[$k] = $v;
        }

        return $cleanOptions;
        $factory->setArgument(3, $config['crawl']['additional_uris']);
        $factory->setArgument(4, $config['crawl']['default_http_client_options']);
            - '@request_stack'
				// TODO: we need a way to authenticate with a token instead of our own cookie
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\HttpClient\Response\MockResponse;
use Symfony\Component\HttpClient\ScopingHttpClient;
use Symfony\Component\HttpFoundation\RequestStack;
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), new RequestStack());
            new RequestStack(),
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), new RequestStack());
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), new RequestStack());

    public function testScopesConfidentialHeadersAutomatically(): void
    {
        $expectedRequests = [
            function (string $method, string $url, array $options): MockResponse {
                $this->assertSame('GET', $method);
                $this->assertSame('https://contao.org/robots.txt', $url);
                $this->assertContains('Cookie: Confidential', $options['headers']);
                $this->assertContains('Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=', $options['headers']);

                return new MockResponse();
            },
            function (string $method, string $url, array $options): MockResponse {
                $this->assertSame('GET', $method);
                $this->assertSame('https://contao.de/robots.txt', $url);
                $this->assertContains('Cookie: Confidential', $options['headers']);
                $this->assertContains('Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=', $options['headers']);

                return new MockResponse();
            },
            function (string $method, string $url, array $options): MockResponse {
                $this->assertSame('GET', $method);
                $this->assertSame('https://www.foreign-domain.com/robots.txt', $url);
                $this->assertNotContains('Cookie: Confidential', $options['headers']);
                $this->assertNotContains('Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=', $options['headers']);

                return new MockResponse();
            },
        ];

        $mockClient = new MockHttpClient($expectedRequests);
        $clientFactory = static fn (array $defaultOptions) => $mockClient;

        $rootPage1 = $this->createMock(PageModel::class);
        $rootPage1
            ->method('getAbsoluteUrl')
            ->willReturn('https://contao.org')
        ;

        $rootPage2 = $this->createMock(PageModel::class);
        $rootPage2
            ->method('getAbsoluteUrl')
            ->willReturn('https://contao.de')
        ;

        $pageModelAdapter = $this->mockAdapter(['findPublishedRootPages']);
        $pageModelAdapter
            ->method('findPublishedRootPages')
            ->willReturn([$rootPage1, $rootPage2])
        ;

        $subscriber1 = $this->createMock(EscargotSubscriberInterface::class);
        $subscriber1
            ->method('getName')
            ->willReturn('subscriber-1')
        ;

        $factory = new Factory(
            $this->createMock(Connection::class),
            $this->mockContaoFramework([PageModel::class => $pageModelAdapter]),
            new RequestStack(),
            ['https://www.foreign-domain.com'],
            [
                'headers' => [
                    'Cookie' => 'Confidential',
                ],
                'auth_basic' => 'username:password',
            ],
            $clientFactory
        );

        $factory->addSubscriber($subscriber1);

        $escargot = $factory->create($factory->getCrawlUriCollection(), new InMemoryQueue(), ['subscriber-1']);
        $escargot->crawl();

        $this->assertSame(3, $mockClient->getRequestsCount());
        $this->assertInstanceOf(ScopingHttpClient::class, $escargot->getClient());
    }
        $this->assertSame(['https://example.com'], $definition->getArgument(3));
        $this->assertSame(['proxy' => 'http://localhost:7080', 'headers' => ['Foo' => 'Bar']], $definition->getArgument(4));