From 0fe66dcc08b024be3ef09cfe2c4d8fb30e0a82cf Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Tue, 10 Aug 2021 20:16:52 +0200
Subject: [PATCH] [BUGFIX] Allow shortcut pages to pages of other sites
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TYPO3 has had a long history of linking to
other pages in other pagetrees. Most of the issues
were solved with the TYPO3 v9 Routing and Site Handling.

Through this feature, it is now possible to
easily create a page of type shortcut and link to
a page in a different page tree.

This was previously not possible because TYPO3 Core
by default generated URLs like "/index.php?id=123"
where people could simply add ?id=355 (where page 355
resides on a different page tree).

Since this change has been resolved, links to
other pages can be allowed and are automatically
redirected.

If we ever get rid of allowing to call pages with
an `?id=123` query parameter, the $pageNotFound=3 case
can be removed completely.

Resolves: #92859
Resolves: #92750
Releases: main, 11.5
Change-Id: I16ccc3e5b0ccb1419ccd2a8d78443616e7627a33
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/70432
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack <benni@typo3.org>
---
 .../TypoScriptFrontendController.php          |  40 +++++-
 .../ShortcutAndMountPointRedirect.php         |   7 +-
 .../SiteHandling/Fixtures/PlainScenario.yaml  |   1 +
 .../SiteHandling/Fixtures/SlugScenario.yaml   |   1 +
 .../SiteHandling/SiteRequestTest.php          | 130 ++++++++++++++++++
 .../SiteHandling/SlugLinkGeneratorTest.php    |   1 +
 .../SiteHandling/SlugSiteRequestTest.php      |  47 +++++++
 7 files changed, 220 insertions(+), 7 deletions(-)

diff --git a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
index 875a8023bc21..75baac07c3ad 100644
--- a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
+++ b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
@@ -44,7 +44,9 @@ use TYPO3\CMS\Core\Error\Http\AbstractServerErrorException;
 use TYPO3\CMS\Core\Error\Http\PageNotFoundException;
 use TYPO3\CMS\Core\Error\Http\ShortcutTargetPageNotFoundException;
 use TYPO3\CMS\Core\Exception\Page\RootLineException;
+use TYPO3\CMS\Core\Exception\SiteNotFoundException;
 use TYPO3\CMS\Core\Http\ApplicationType;
+use TYPO3\CMS\Core\Http\ImmediateResponseException;
 use TYPO3\CMS\Core\Http\NormalizedParams;
 use TYPO3\CMS\Core\Http\PropagateResponseException;
 use TYPO3\CMS\Core\Localization\LanguageService;
@@ -58,6 +60,7 @@ use TYPO3\CMS\Core\PageTitle\PageTitleProviderManager;
 use TYPO3\CMS\Core\Routing\PageArguments;
 use TYPO3\CMS\Core\Site\Entity\SiteInterface;
 use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
+use TYPO3\CMS\Core\Site\SiteFinder;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
 use TYPO3\CMS\Core\Type\Bitmask\PageTranslationVisibility;
 use TYPO3\CMS\Core\Type\Bitmask\Permission;
@@ -1415,21 +1418,29 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     protected function getPageAndRootlineWithDomain($rootPageId, ServerRequestInterface $request)
     {
         $this->getPageAndRootline($request);
+        $rootPageId = (int)$rootPageId;
         // Checks if the $domain-startpage is in the rootLine. This is necessary so that references to page-id's via ?id=123 from other sites are not possible.
         if (is_array($this->rootLine) && $this->rootLine !== []) {
             $idFound = false;
-            foreach ($this->rootLine as $key => $val) {
-                if ($val['uid'] == $rootPageId) {
+            foreach ($this->rootLine as $pageInRootLine) {
+                if ((int)$pageInRootLine['uid'] === $rootPageId) {
                     $idFound = true;
                     break;
                 }
             }
             if (!$idFound) {
                 // Page is 'not found' in case the id was outside the domain, code 3
-                $this->pageNotFound = 3;
-                $this->id = $rootPageId;
-                // re-get the page and rootline if the id was not found.
-                $this->getPageAndRootline($request);
+                // This can only happen if there was a shortcut. So $this->page is now the shortcut target
+                // But the original page is in $this->originalShortcutPage.
+                // This only happens if people actually call TYPO3 with index.php?id=123 where 123 is in a different
+                // page tree. This is not allowed.
+                $directlyRequestedId = (int)($request->getQueryParams()['id'] ?? 0);
+                if ($directlyRequestedId && (int)($this->originalShortcutPage['uid'] ?? 0) !== $directlyRequestedId) {
+                    $this->pageNotFound = 3;
+                    $this->id = $rootPageId;
+                    // re-get the page and rootline if the id was not found.
+                    $this->getPageAndRootline($request);
+                }
             }
         }
     }
@@ -2170,6 +2181,23 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     public function getRedirectUriForShortcut(ServerRequestInterface $request): ?string
     {
         if (!empty($this->originalShortcutPage) && $this->originalShortcutPage['doktype'] == PageRepository::DOKTYPE_SHORTCUT) {
+            // Check if the shortcut page is actually on the current site, if not, this is a "page not found"
+            // because the request was www.mydomain.com/?id=23 where page ID 23 (which is a shortcut) is on another domain/site.
+            if ((int)($request->getQueryParams()['id'] ?? 0) > 0) {
+                try {
+                    $site = GeneralUtility::makeInstance(SiteFinder::class)->getSiteByPageId($this->originalShortcutPage['l10n_parent'] ?: $this->originalShortcutPage['uid']);
+                } catch (SiteNotFoundException $e) {
+                    $site = null;
+                }
+                if ($site !== $this->site) {
+                    $response = GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
+                        $request,
+                        'ID was outside the domain',
+                        $this->getPageAccessFailureReasons(PageAccessFailureReasons::ACCESS_DENIED_HOST_PAGE_MISMATCH)
+                    );
+                    throw new ImmediateResponseException($response, 1638022483);
+                }
+            }
             return $this->getUriToCurrentPageForRedirect($request);
         }
 
diff --git a/typo3/sysext/frontend/Classes/Middleware/ShortcutAndMountPointRedirect.php b/typo3/sysext/frontend/Classes/Middleware/ShortcutAndMountPointRedirect.php
index 27256e0c37e7..98eb31ca213e 100644
--- a/typo3/sysext/frontend/Classes/Middleware/ShortcutAndMountPointRedirect.php
+++ b/typo3/sysext/frontend/Classes/Middleware/ShortcutAndMountPointRedirect.php
@@ -22,6 +22,7 @@ use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use TYPO3\CMS\Core\Domain\Repository\PageRepository;
+use TYPO3\CMS\Core\Http\ImmediateResponseException;
 use TYPO3\CMS\Core\Http\RedirectResponse;
 use TYPO3\CMS\Core\Routing\PageArguments;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -40,7 +41,11 @@ class ShortcutAndMountPointRedirect implements MiddlewareInterface
         $exposeInformation = $GLOBALS['TYPO3_CONF_VARS']['FE']['exposeRedirectInformation'] ?? false;
 
         // Check for shortcut page and mount point redirect
-        $redirectToUri = $this->getRedirectUri($request);
+        try {
+            $redirectToUri = $this->getRedirectUri($request);
+        } catch (ImmediateResponseException $e) {
+            return $e->getResponse();
+        }
         if ($redirectToUri !== null && $redirectToUri !== (string)$request->getUri()) {
             /** @var PageArguments $pageArguments */
             $pageArguments = $request->getAttribute('routing', null);
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/PlainScenario.yaml b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/PlainScenario.yaml
index 74b5eff73a61..a384e135ca1c 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/PlainScenario.yaml
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/PlainScenario.yaml
@@ -121,6 +121,7 @@ entities:
           children:
             - self: {id: 2021, title: 'FEGroups Restricted', visitorGroups: 30}
             - self: {id: 2022, title: 'FEGroups Restricted', hidden: 1, visitorGroups: 30}
+        - self: {id: 2030, title: 'Cross Site Shortcut', type: *pageShortcut, shortcut: 2100}
     - self: {id: 2000, title: 'ACME Blog', type: *pageShortcut, shortcut: 'first', root: true}
       children:
         - self: {id: 2100, title: 'Authors'}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
index 9e0c58b06fd9..0dc0e85dee08 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
@@ -132,6 +132,7 @@ entities:
           children:
             - self: {id: 2021, title: 'FEGroups Restricted', visitorGroups: 30, slug: '/sysfolder-restricted'}
             - self: {id: 2022, title: 'FEGroups Restricted', hidden: 1, visitorGroups: 30, slug: '/sysfolder-restricted-hidden'}
+        - self: {id: 2030, title: 'Cross Site Shortcut', slug: '/cross-site-shortcut', type: *pageShortcut, shortcut: 2100}
     - self: {id: 2000, title: 'ACME Blog', type: *pageShortcut, shortcut: 'first', root: true, slug: '/'}
       children:
         - self: {id: 2100, title: 'Authors', slug: '/authors'}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
index 80ee0d6e37d7..ab889b24dcff 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
@@ -1014,4 +1014,134 @@ class SiteRequestTest extends AbstractTestCase
         self::assertSame($expectedStatusCode, $response->getStatusCode());
         self::assertSame($expectedHeaders, $response->getHeaders());
     }
+
+    public function crossSiteShortcutsAreRedirectedDataProvider(): array
+    {
+        return [
+            'shortcut is redirected #1' => [
+                'https://website.local/index.php?id=2030',
+                307,
+                [
+                    'X-Redirect-By' => ['TYPO3 Shortcut/Mountpoint'],
+                    'location' => ['https://blog.local/authors'],
+                ],
+            ],
+            'shortcut is redirected #2' => [
+                'https://website.local/?id=2030',
+                307,
+                [
+                    'X-Redirect-By' => ['TYPO3 Shortcut/Mountpoint'],
+                    'location' => ['https://blog.local/authors'],
+                ],
+            ],
+            'shortcut is redirected #3' => [
+                'https://website.local/index.php?id=2030&type=0',
+                307,
+                [
+                    'X-Redirect-By' => ['TYPO3 Shortcut/Mountpoint'],
+                    'location' => ['https://blog.local/authors'],
+                ],
+            ],
+            'shortcut is redirected #4' => [
+                'https://website.local/?id=2030&type=0',
+                307,
+                [
+                    'X-Redirect-By' => ['TYPO3 Shortcut/Mountpoint'],
+                    'location' => ['https://blog.local/authors'],
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider crossSiteShortcutsAreRedirectedDataProvider
+     */
+    public function crossSiteShortcutsAreRedirected(string $uri, int $expectedStatusCode, array $expectedHeaders): void
+    {
+        $this->writeSiteConfiguration(
+            'website-local',
+            $this->buildSiteConfiguration(1000, 'https://website.local/')
+        );
+        $this->writeSiteConfiguration(
+            'blog-local',
+            $this->buildSiteConfiguration(2000, 'https://blog.local/')
+        );
+        $this->setUpFrontendRootPage(
+            2000,
+            [
+                'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+                'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+            ],
+            [
+                'title' => 'ACME Blog',
+            ]
+        );
+
+        $response = $this->executeFrontendSubRequest(
+            new InternalRequest($uri),
+            $this->internalRequestContext
+        );
+        self::assertSame($expectedStatusCode, $response->getStatusCode());
+        self::assertSame($expectedHeaders, $response->getHeaders());
+    }
+
+    public function crossSiteShortcutsWithWrongSiteHostSendsPageNotFoundWithoutHavingErrorHandlingDataProvider(): array
+    {
+        return [
+            'shortcut requested by id on wrong site #1' => [
+                'https://blog.local/index.php?id=2030',
+            ],
+            'shortcut requested by id on wrong site #2' => [
+                'https://blog.local/?id=2030',
+            ],
+            'shortcut requested by id on wrong site #3' => [
+                'https://blog.local/index.php?id=2030&type=0',
+            ],
+            'shortcut requested by id on wrong site #4' => [
+                'https://blog.local/?id=2030&type=0',
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider crossSiteShortcutsWithWrongSiteHostSendsPageNotFoundWithoutHavingErrorHandlingDataProvider
+     */
+    public function crossSiteShortcutsWithWrongSiteHostSendsPageNotFoundWithoutHavingErrorHandling(string $uri): void
+    {
+        $this->writeSiteConfiguration(
+            'website-local',
+            $this->buildSiteConfiguration(1000, 'https://website.local/'),
+            [],
+            $this->buildErrorHandlingConfiguration('PHP', [404])
+        );
+        $this->writeSiteConfiguration(
+            'blog-local',
+            $this->buildSiteConfiguration(2000, 'https://blog.local/'),
+            [],
+            $this->buildErrorHandlingConfiguration('PHP', [404])
+        );
+
+        $this->setUpFrontendRootPage(
+            2000,
+            [
+                'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+                'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+            ],
+            [
+                'title' => 'ACME Blog',
+            ]
+        );
+        $response = $this->executeFrontendSubRequest(
+            new InternalRequest($uri),
+            $this->internalRequestContext
+        );
+        $json = json_decode((string)$response->getBody(), true);
+        self::assertSame(404, $response->getStatusCode());
+        self::assertThat(
+            $json['message'] ?? null,
+            self::stringContains('ID was outside the domain')
+        );
+    }
 }
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
index b518b84c9aa8..d5842b9b2293 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
@@ -733,6 +733,7 @@ class SlugLinkGeneratorTest extends AbstractTestCase
                     ['title' => 'That page is forbidden to you', 'link' => '/403'],
                     ['title' => 'That page was not found', 'link' => '/404'],
                     ['title' => 'Our Blog', 'link' => 'https://blog.acme.com/authors'],
+                    ['title' => 'Cross Site Shortcut', 'link' => 'https://blog.acme.com/authors'],
                 ],
             ],
             'ACME Blog' => [
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
index 24c33dbb154f..1001b04e2b9e 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
@@ -1258,4 +1258,51 @@ class SlugSiteRequestTest extends AbstractTestCase
             $responseStructure->getScopePath('getpost/testing.value')
         );
     }
+
+    public function crossSiteShortcutsAreRedirectedDataProvider(): array
+    {
+        return [
+            'shortcut is redirected' => [
+                'https://website.local/cross-site-shortcut',
+                307,
+                [
+                    'X-Redirect-By' => ['TYPO3 Shortcut/Mountpoint'],
+                    'location' => ['https://blog.local/authors'],
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider crossSiteShortcutsAreRedirectedDataProvider
+     */
+    public function crossSiteShortcutsAreRedirected(string $uri, int $expectedStatusCode, array $expectedHeaders): void
+    {
+        $this->writeSiteConfiguration(
+            'website-local',
+            $this->buildSiteConfiguration(1000, 'https://website.local/')
+        );
+        $this->writeSiteConfiguration(
+            'blog-local',
+            $this->buildSiteConfiguration(2000, 'https://blog.local/')
+        );
+        $this->setUpFrontendRootPage(
+            2000,
+            [
+                'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+                'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+            ],
+            [
+                'title' => 'ACME Blog',
+            ]
+        );
+
+        $response = $this->executeFrontendSubRequest(
+            new InternalRequest($uri),
+            $this->internalRequestContext
+        );
+        self::assertSame($expectedStatusCode, $response->getStatusCode());
+        self::assertSame($expectedHeaders, $response->getHeaders());
+    }
 }
-- 
GitLab