[SECURITY] Show only explicitly configured page tree information

Backend users were able see page tree items without having access: - in case no DB mounts were configured for a particular user and page permissions configured to allow "everybody" - in case DB mounts were pointing to pages, but actually not having any permission configured for these pages (user/group/everybody) It was not possible to manipulate any of the affected pages. Resolves: #104397 Releases: main, 13.3, 12.4, 11.5 Change-Id: I52079c8cef3d78946083403adb23a3e1a706c652 Security-Bulletin: TYPO3-CORE-SA-2024-012 Security-References: CVE-2024-47780 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/86495 Reviewed-by: Oliver Hader <oliver.hader@typo3.org> Tested-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3-CMS · Oct 8, 2024 · a7b3c92 · a7b3c92
1 parent 8276eac
commit a7b3c92
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 17 deletions.
diff --git a/Classes/Controller/Page/TreeController.php b/Classes/Controller/Page/TreeController.php
@@ -420,11 +420,11 @@ protected function pagesToFlatArray(array $page, int $entryPoint, int $depth = 0
             'allowEdit' => $this->userHasAccessToModifyPagesAndToDefaultLanguage && $backendUser->doesUserHaveAccess($page, Permission::PAGE_EDIT),
         ];
 
-        if (!empty($page['_children']) || $this->pageTreeRepository->hasChildren($pageId)) {
+        if ($depth >= $this->levelsToFetch && $this->pageTreeRepository->hasChildren($pageId)) {
+            $page = $this->pageTreeRepository->getTreeLevels($page, 1);
+        }
+        if (!empty($page['_children'])) {
             $item['hasChildren'] = true;
-            if ($depth >= $this->levelsToFetch) {
-                $page = $this->pageTreeRepository->getTreeLevels($page, 1);
-            }
         }
         if (!empty($prefix)) {
             $item['prefix'] = htmlspecialchars($prefix);

diff --git a/Classes/Tree/Repository/PageTreeRepository.php b/Classes/Tree/Repository/PageTreeRepository.php
@@ -168,29 +168,28 @@ protected function applyCallbackToChildren(array &$tree, callable $callback)
      *
      * @param array $pageTree The page record of the top level page you want to get the page tree of
      * @param int $depth Number of levels to fetch
-     * @param array $entryPointIds entryPointIds to include
+     * @param ?array $entryPointIds entryPointIds to include (null in case no entry-points were provided)
      * @return array An array with page records and their children
      */
-    public function getTreeLevels(array $pageTree, int $depth, array $entryPointIds = []): array
+    public function getTreeLevels(array $pageTree, int $depth, ?array $entryPointIds = null): array
     {
         $groupedAndSortedPagesByPid = [];
-
-        if (count($entryPointIds) > 0) {
+        // the method was called without any entry-point information
+        if ($entryPointIds === null) {
+            $parentPageIds = [$pageTree['uid']];
+            // the method was called with entry-point information, that is not empty
+        } elseif ($entryPointIds !== []) {
             $pageRecords = $this->getPageRecords($entryPointIds);
             $groupedAndSortedPagesByPid[$pageTree['uid']] = $pageRecords;
             $parentPageIds = $entryPointIds;
-        } else {
-            $parentPageIds = [$pageTree['uid']];
         }
-
         for ($i = 0; $i < $depth; $i++) {
+            // stop in case the initial or recursive query did not have any pages
             if (empty($parentPageIds)) {
                 break;
             }
             $pageRecords = $this->getChildPageRecords($parentPageIds);
-
             $groupedAndSortedPagesByPid = $this->groupAndSortPages($pageRecords, $groupedAndSortedPagesByPid);
-
             $parentPageIds = array_column($pageRecords, 'uid');
         }
         $this->addChildrenToPage($pageTree, $groupedAndSortedPagesByPid);

diff --git a/Tests/Functional/Controller/Page/Fixtures/PagesWithBEPermissions.yaml b/Tests/Functional/Controller/Page/Fixtures/PagesWithBEPermissions.yaml
@@ -4,8 +4,6 @@ __variables:
   - &pageMount 7
   - &pageFolder 254
   - &contentText 'text'
-  - &idAcmeRootPage 1000
-  - &idAcmeFirstPage 1100
 
 entitySettings:
   '*':
@@ -52,11 +50,13 @@ entities:
     - self: {id: 2, title: 'Franco-Canadian', code: 'fr'}
     - self: {id: 3, title: 'Spanish', code: 'es'}
   beGroup:
+    - self: {id: 7, title: 'editors without DB mounts', db_mountpoints: '', tables_select: 'pages,tt_content', tables_modify: 'pages,tt_content', page_types_select: '1,4,7,254', groupMods: 'web_layout,web_list'}
+    - self: {id: 8, title: 'editors with DB mounts', db_mountpoints: '9200', tables_select: 'pages,tt_content', tables_modify: 'pages,tt_content', page_types_select: '1,4,7,254', groupMods: 'web_layout,web_list'}
     - self: {id: 9, title: 'editors', db_mountpoints: '1000,2000,8110', tables_select: 'pages,tt_content', tables_modify: 'pages,tt_content', page_types_select: '1,4,7,254', groupMods: 'web_layout,web_list'}
   page:
-    - self: {id: *idAcmeRootPage, title: 'ACME Inc', type: *pageShortcut, shortcut: 'first', root: true, slug: '/'}
+    - self: {id: 1000, title: 'ACME Inc', type: *pageShortcut, shortcut: 'first', root: true, slug: '/'}
       children:
-        - self: {id: *idAcmeFirstPage, title: 'EN: Welcome', slug: '/welcome', subtitle: 'hello-and-welcome'}
+        - self: {id: 1100, title: 'EN: Welcome', slug: '/welcome', subtitle: 'hello-and-welcome'}
         - self: {id: 1200, title: 'EN: Features', slug: '/features'}
           children:
             - self: {id: 1210, title: 'EN: Frontend Editing', slug: '/features/frontend-editing', perms_userid: 9, perms_groupid: 1, description: "accessible for user, but not for group"}
@@ -160,3 +160,6 @@ entities:
                         - { action: 'move', type: 'toPage', target: 1700 }
             - self: {id: 8120, title: 'Asia', slug: '/divisions/asia', description: 'not visible as not mounted'}
             - self: {id: 8130, title: 'South America', slug: '/divisions/south-america'}
+    - self: {id: 9100, title: 'Page 9100', type: *pageStandard, root: true, slug: '/page-9100', perms_userid: 1, perms_groupid: 1, perms_everybody: 15}
+    - self: {id: 9200, title: 'Page 9200', type: *pageStandard, root: true, slug: '/page-9200', perms_userid: 1, perms_groupid: 1, perms_everybody: 0}
+    - self: {id: 9300, title: 'Page 9300', type: *pageStandard, root: true, slug: '/page-9300', perms_userid: 1, perms_groupid: 1, perms_everybody: 15}
diff --git a/Tests/Functional/Controller/Page/TreeControllerTest.php b/Tests/Functional/Controller/Page/TreeControllerTest.php
@@ -23,6 +23,8 @@
 use TYPO3\CMS\Core\Context\Context;
 use TYPO3\CMS\Core\Context\WorkspaceAspect;
 use TYPO3\CMS\Core\Core\Bootstrap;
+use TYPO3\CMS\Core\Http\ServerRequest;
+use TYPO3\CMS\Core\Http\Uri;
 use TYPO3\CMS\Core\Tests\Functional\SiteHandling\SiteBasedTestTrait;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\TestingFramework\Core\AccessibleObjectInterface;
@@ -291,6 +293,19 @@ public function getAllEntryPointPageTreesWithRootPageAsMountPoint(): void
                             ],
                         ],
                     ],
+                    [
+                        // 9100 is shown due to `perms_everybody=15`
+                        'uid' => 9100,
+                        'title' => 'Page 9100',
+                        '_children' => [],
+                    ],
+                    // 9200 is omitted due to `perms_everybody=0`
+                    [
+                        // 9300 is shown due to `perms_everybody=15`
+                        'uid' => 9300,
+                        'title' => 'Page 9300',
+                        '_children' => [],
+                    ],
                 ],
             ],
             [
@@ -723,6 +738,41 @@ public function getSubtreeForAccessiblePageInWorkspace(): void
         self::assertEquals($expected, $actual);
     }
 
+    public static function fetchDataActionConsidersPermissionsDataProvider(): \Generator
+    {
+        yield 'admin user can see all root pages' => [
+            'backendUser' => 1,
+            'expectation' => ['0', '1000', '2000', '7000', '8000', '9100', '9200', '9300'],
+        ];
+        yield 'editor with DB mounts can only see accessible pages' => [
+            'backendUser' => 9,
+            'expectation' => ['0', '1000', '8110'],
+        ];
+        yield 'editor with DB mounts cannot see inaccessible pages' => [
+            'backendUser' => 8,
+            'expectation' => ['0'],
+        ];
+        yield 'editor without DB mounts cannot see any pages' => [
+            'backendUser' => 7,
+            'expectation' => ['0'],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider fetchDataActionConsidersPermissionsDataProvider
+     */
+    public function fetchDataActionConsidersPermissions(int $backendUser, array $expectation): void
+    {
+        $this->backendUser = $this->setUpBackendUser($backendUser);
+        $request = (new ServerRequest(new Uri('https://example.com')))->withQueryParams(['depth' => 1]);
+        $response = (new TreeController())->fetchDataAction($request);
+        $data = json_decode((string)$response->getBody(), true);
+        $items = array_filter($data, static fn(array $page): bool => $page['depth'] <= 1);
+        $items = array_map(static fn(array $page): string => $page['identifier'], $items);
+        self::assertSame($expectation, array_values($items));
+    }
+
     /**
      * @param int $workspaceId
      */