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/config/services.yaml
core-bundle/contao/classes/Crawl.php
core-bundle/src/Crawl/Escargot/Factory.php
core-bundle/src/DependencyInjection/ContaoCoreExtension.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:

				// 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();
     * @param array<string> $additionalUris
        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(3, $config['crawl']['additional_uris']);
        $factory->setArgument(4, $config['crawl']['default_http_client_options']);
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), $this->createMock(ContentUrlGenerator::class));
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), $this->createMock(ContentUrlGenerator::class));
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), $this->createMock(ContentUrlGenerator::class));
        $this->assertSame(['https://example.com'], $definition->getArgument(3));
        $this->assertSame(['proxy' => 'http://localhost:7080', 'headers' => ['Foo' => 'Bar']], $definition->getArgument(4));

Fixed code:
            - '@request_stack'
				// TODO: we need a way to authenticate with a token instead of our own cookie
use Psr\Http\Message\UriInterface;
use Symfony\Component\HttpClient\ScopingHttpClient;
use Symfony\Component\HttpFoundation\RequestStack;
     * @var \Closure(array<string, mixed>): HttpClientInterface|null
     */
    private readonly \Closure|null $httpClientFactory;

    /**
     * @param array<string>                                            $additionalUris
     * @param \Closure(array<string, mixed>): HttpClientInterface|null $httpClientFactory
        private readonly RequestStack $requestStack,
        \Closure|null $httpClientFactory = null,
        $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
        if ($this->requestStack->getMainRequest()?->hasSession()) {
            $this->requestStack->getMainRequest()->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(4, $config['crawl']['additional_uris']);
        $factory->setArgument(5, $config['crawl']['default_http_client_options']);
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(), $this->createMock(ContentUrlGenerator::class), new RequestStack());
            new RequestStack(),
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), $this->createMock(ContentUrlGenerator::class), new RequestStack());
        $factory = new Factory($this->createMock(Connection::class), $this->mockContaoFramework(), $this->createMock(ContentUrlGenerator::class), 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->mockClassWithProperties(PageModel::class, ['dns' => 'contao.org']);
        $rootPage2 = $this->mockClassWithProperties(PageModel::class, ['dns' => 'contao.de']);

        $urlGenerator = $this->createMock(ContentUrlGenerator::class);
        $urlGenerator
            ->method('generate')
            ->willReturnCallback(static fn (PageModel $rootPage) => 'https://'.$rootPage->dns)
        ;

        $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]),
            $urlGenerator,
            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(4));
        $this->assertSame(['proxy' => 'http://localhost:7080', 'headers' => ['Foo' => 'Bar']], $definition->getArgument(5));