From f4dd6171dd9b37724d4bf1395d13ddf884530a3f Mon Sep 17 00:00:00 2001
From: Torben Hansen <derhansen@gmail.com>
Date: Tue, 14 Jun 2022 09:11:58 +0200
Subject: [PATCH] [SECURITY] Restrict export functionality to allowed users
The import functionality of the import/export module is already
restricted to admin users or users, who explicitly have access through
the user TSConfig setting "options.impexp.enableImportForNonAdminUser".
The export functionality has the following security drawbacks:
* Export for editors is not limited on field level
* The "Save to filename" functionality saves to a shared folder, which
other editors with different access rights may have access to.
Both issues are not easy to resolve and also the target audience for
the Import/Export functionality are mainly TYPO3 admins.
Therefore, now also the export functionality is restricted to TYPO3
admin users and to users, who explicitly have access through the new
user TSConfig setting "options.impexp.enableExportForNonAdminUser".
Additionally, the contents of the temporary "importexport" folder in
file storages is now only visible to users who have access to the
export functionality.
In general, it is recommended to only install the Import/Export
extension when the functionality is required.
Resolves: #94951
Releases: main, 11.5, 10.4
Change-Id: Iae020baf051aeec0613366687aa8ebcbf9b3d8b2
Security-Bulletin: TYPO3-CORE-SA-2022-001
Security-References: CVE-2022-31046
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/74897
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
.../BackendUserAuthentication.php | 22 +++++++
.../Resource/Filter/ImportExportFilter.php | 55 ++++++++++++++++
.../core/Classes/Resource/ResourceStorage.php | 29 ++++++--
.../Application/Impexp/UsersCest.php | 10 +--
.../BackendUserAuthenticationTest.php | 66 +++++++++++++++++++
.../Classes/ContextMenu/ItemProvider.php | 13 +---
.../Classes/Controller/ExportController.php | 8 +++
.../Classes/Controller/ImportController.php | 13 +---
.../Classes/Report/Status/SecurityStatus.php | 46 +++++++++++++
.../Private/Language/locallang_reports.xlf | 9 +++
10 files changed, 236 insertions(+), 35 deletions(-)
create mode 100644 typo3/sysext/core/Classes/Resource/Filter/ImportExportFilter.php
diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
index 5238df5e2698..570f3bbab02b 100644
--- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
+++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
@@ -2368,4 +2368,26 @@ TCAdefaults.sys_note.email = ' . $this->user['email'];
|| ($globalConfig === 3 && $isAdmin)
|| ($globalConfig === 4 && $this->isSystemMaintainer());
}
+
+ /**
+ * Returns if import functionality is available for current user
+ *
+ * @internal
+ */
+ public function isImportEnabled(): bool
+ {
+ return $this->isAdmin()
+ || ($this->getTSConfig()['options.']['impexp.']['enableImportForNonAdminUser'] ?? false);
+ }
+
+ /**
+ * Returns if export functionality is available for current user
+ *
+ * @internal
+ */
+ public function isExportEnabled(): bool
+ {
+ return $this->isAdmin()
+ || ($this->getTSConfig()['options.']['impexp.']['enableExportForNonAdminUser'] ?? false);
+ }
}
diff --git a/typo3/sysext/core/Classes/Resource/Filter/ImportExportFilter.php b/typo3/sysext/core/Classes/Resource/Filter/ImportExportFilter.php
new file mode 100644
index 000000000000..789f02ee7b46
--- /dev/null
+++ b/typo3/sysext/core/Classes/Resource/Filter/ImportExportFilter.php
@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Resource\Filter;
+
+use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
+use TYPO3\CMS\Core\Resource\Driver\DriverInterface;
+
+/**
+ * Utility methods for filtering filenames stored in `importexport` temporary folder.
+ * Albeit this filter is in the scope of `ext:impexp`, it is located in `ext:core` to
+ * apply filters on left-over fragments, even when `ext:impexp` is not installed.
+ *
+ * @internal
+ */
+class ImportExportFilter
+{
+ /**
+ * Filter method that checks if a directory or a file in such directory belongs to the temp directory of EXT:impexp
+ * and the user has "export" permissions.
+ */
+ public static function filterImportExportFilesAndFolders(string $itemName, string $itemIdentifier, string $parentIdentifier, array $additionalInformation, DriverInterface $driverInstance)
+ {
+ // + `_temp_` is hard-coded in `BackendUserAuthentication::getDefaultUploadTemporaryFolder()`
+ // + `importexport` is hard-coded in `ImportExport::createDefaultImportExportFolder()`
+ $importExportFolderSubPath = '/_temp_/importexport/';
+ if (str_ends_with($parentIdentifier, $importExportFolderSubPath) || str_contains($itemIdentifier, $importExportFolderSubPath)) {
+ $backendUser = self::getBackendUser();
+ if ($backendUser === null || !$backendUser->isExportEnabled()) {
+ return -1;
+ }
+ }
+
+ return true;
+ }
+
+ protected static function getBackendUser(): ?BackendUserAuthentication
+ {
+ return $GLOBALS['BE_USER'] ?? null;
+ }
+}
diff --git a/typo3/sysext/core/Classes/Resource/ResourceStorage.php b/typo3/sysext/core/Classes/Resource/ResourceStorage.php
index 94e1603b657f..752eb0d5c6f4 100644
--- a/typo3/sysext/core/Classes/Resource/ResourceStorage.php
+++ b/typo3/sysext/core/Classes/Resource/ResourceStorage.php
@@ -72,6 +72,7 @@ use TYPO3\CMS\Core\Resource\Exception\InvalidTargetFolderException;
use TYPO3\CMS\Core\Resource\Exception\ResourcePermissionsUnavailableException;
use TYPO3\CMS\Core\Resource\Exception\UploadException;
use TYPO3\CMS\Core\Resource\Exception\UploadSizeException;
+use TYPO3\CMS\Core\Resource\Filter\ImportExportFilter;
use TYPO3\CMS\Core\Resource\Index\FileIndexRepository;
use TYPO3\CMS\Core\Resource\Index\Indexer;
use TYPO3\CMS\Core\Resource\OnlineMedia\Helpers\OnlineMediaHelperRegistry;
@@ -1528,6 +1529,19 @@ class ResourceStorage implements ResourceStorageInterface
$this->fileAndFolderNameFilters = $GLOBALS['TYPO3_CONF_VARS']['SYS']['fal']['defaultFilterCallbacks'];
}
+ /**
+ * Returns a filter for files generated by EXT:impexp
+ *
+ * @return array<int, ImportExportFilter|string>
+ * @internal
+ */
+ public function getImportExportFilter(): array
+ {
+ $filter = GeneralUtility::makeInstance(ImportExportFilter::class);
+
+ return [$filter, 'filterImportExportFilesAndFolders'];
+ }
+
/**
* Returns the file and folder name filters used by this storage.
*
@@ -1535,7 +1549,7 @@ class ResourceStorage implements ResourceStorageInterface
*/
public function getFileAndFolderNameFilters()
{
- return $this->fileAndFolderNameFilters;
+ return array_merge($this->fileAndFolderNameFilters, [$this->getImportExportFilter()]);
}
/**
@@ -1600,7 +1614,7 @@ class ResourceStorage implements ResourceStorageInterface
$rows = $this->getFileIndexRepository()->findByFolder($folder);
- $filters = $useFilters == true ? $this->fileAndFolderNameFilters : [];
+ $filters = $useFilters == true ? $this->getFileAndFolderNameFilters() : [];
$fileIdentifiers = array_values($this->driver->getFilesInFolder($folder->getIdentifier(), $start, $maxNumberOfItems, $recursive, $filters, $sort, $sortRev));
$items = [];
@@ -1630,7 +1644,7 @@ class ResourceStorage implements ResourceStorageInterface
*/
public function getFileIdentifiersInFolder($folderIdentifier, $useFilters = true, $recursive = false)
{
- $filters = $useFilters == true ? $this->fileAndFolderNameFilters : [];
+ $filters = $useFilters == true ? $this->getFileAndFolderNameFilters() : [];
return $this->driver->getFilesInFolder($folderIdentifier, 0, 0, $recursive, $filters);
}
@@ -1644,7 +1658,7 @@ class ResourceStorage implements ResourceStorageInterface
public function countFilesInFolder(Folder $folder, $useFilters = true, $recursive = false)
{
$this->assureFolderReadPermission($folder);
- $filters = $useFilters ? $this->fileAndFolderNameFilters : [];
+ $filters = $useFilters ? $this->getFileAndFolderNameFilters() : [];
return $this->driver->countFilesInFolder($folder->getIdentifier(), $recursive, $filters);
}
@@ -1656,7 +1670,7 @@ class ResourceStorage implements ResourceStorageInterface
*/
public function getFolderIdentifiersInFolder($folderIdentifier, $useFilters = true, $recursive = false)
{
- $filters = $useFilters == true ? $this->fileAndFolderNameFilters : [];
+ $filters = $useFilters == true ? $this->getFileAndFolderNameFilters() : [];
return $this->driver->getFoldersInFolder($folderIdentifier, 0, 0, $recursive, $filters);
}
@@ -2418,7 +2432,7 @@ class ResourceStorage implements ResourceStorageInterface
*/
public function getFoldersInFolder(Folder $folder, $start = 0, $maxNumberOfItems = 0, $useFilters = true, $recursive = false, $sort = '', $sortRev = false)
{
- $filters = $useFilters == true ? $this->fileAndFolderNameFilters : [];
+ $filters = $useFilters == true ? $this->getFileAndFolderNameFilters() : [];
$folderIdentifiers = $this->driver->getFoldersInFolder($folder->getIdentifier(), $start, $maxNumberOfItems, $recursive, $filters, $sort, $sortRev);
@@ -2429,6 +2443,7 @@ class ResourceStorage implements ResourceStorageInterface
unset($folderIdentifiers[$processingIdentifier]);
}
}
+
$folders = [];
foreach ($folderIdentifiers as $folderIdentifier) {
$folders[$folderIdentifier] = $this->getFolder($folderIdentifier, true);
@@ -2446,7 +2461,7 @@ class ResourceStorage implements ResourceStorageInterface
public function countFoldersInFolder(Folder $folder, $useFilters = true, $recursive = false)
{
$this->assureFolderReadPermission($folder);
- $filters = $useFilters ? $this->fileAndFolderNameFilters : [];
+ $filters = $useFilters ? $this->getFileAndFolderNameFilters() : [];
return $this->driver->countFoldersInFolder($folder->getIdentifier(), $recursive, $filters);
}
diff --git a/typo3/sysext/core/Tests/Acceptance/Application/Impexp/UsersCest.php b/typo3/sysext/core/Tests/Acceptance/Application/Impexp/UsersCest.php
index 2c96c718bc99..43953ebb1167 100644
--- a/typo3/sysext/core/Tests/Acceptance/Application/Impexp/UsersCest.php
+++ b/typo3/sysext/core/Tests/Acceptance/Application/Impexp/UsersCest.php
@@ -52,7 +52,7 @@ class UsersCest extends AbstractCest
/**
* @throws \Exception
*/
- public function doNotShowImportInContextMenuForNonAdminUser(ApplicationTester $I, PageTree $pageTree): void
+ public function doNotShowImportAndExportInContextMenuForNonAdminUser(ApplicationTester $I, PageTree $pageTree): void
{
$selectedPageTitle = 'Root';
$selectedPageIcon = '//*[text()=\'' . $selectedPageTitle . '\']/../*[contains(@class, \'node-icon-container\')]';
@@ -65,7 +65,7 @@ class UsersCest extends AbstractCest
$I->click($selectedPageIcon);
$this->selectInContextMenu($I, [$this->contextMenuMore]);
$I->waitForElementVisible('#contentMenu1', 5);
- $I->seeElement($this->contextMenuExport);
+ $I->dontSeeElement($this->contextMenuExport);
$I->dontSeeElement($this->contextMenuImport);
$I->useExistingSession('admin');
@@ -74,19 +74,19 @@ class UsersCest extends AbstractCest
/**
* @throws \Exception
*/
- public function showImportInContextMenuForNonAdminUserIfFlagSet(ApplicationTester $I): void
+ public function showImportExportInContextMenuForNonAdminUserIfFlagSet(ApplicationTester $I): void
{
$selectedPageTitle = 'Root';
$selectedPageIcon = '//*[text()=\'' . $selectedPageTitle . '\']/../*[contains(@class, \'node-icon-container\')]';
- $this->setUserTsConfig($I, 2, 'options.impexp.enableImportForNonAdminUser = 1');
+ $this->setUserTsConfig($I, 2, "options.impexp.enableImportForNonAdminUser = 1\noptions.impexp.enableExportForNonAdminUser = 1");
$I->useExistingSession('editor');
$I->click($selectedPageIcon);
$this->selectInContextMenu($I, [$this->contextMenuMore]);
$I->waitForElementVisible('#contentMenu1', 5);
- $I->seeElement($this->contextMenuExport);
$I->seeElement($this->contextMenuImport);
+ $I->seeElement($this->contextMenuExport);
$I->useExistingSession('admin');
}
diff --git a/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php b/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
index 7e77ab07d3f2..981c67eddfbe 100644
--- a/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
+++ b/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
@@ -147,4 +147,70 @@ class BackendUserAuthenticationTest extends FunctionalTestCase
// which should fail since the user in the fixture has MFA activated but not yet passed.
$this->setUpBackendUser(4);
}
+
+ public function isImportEnabledDataProvider(): array
+ {
+ return [
+ 'admin user' => [
+ 1,
+ '',
+ true,
+ ],
+ 'editor user' => [
+ 2,
+ '',
+ false,
+ ],
+ 'editor user - enableImportForNonAdminUser = 1' => [
+ 2,
+ 'options.impexp.enableImportForNonAdminUser = 1',
+ true,
+ ],
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider isImportEnabledDataProvider
+ */
+ public function isImportEnabledReturnsExpectedValues(int $userId, string $tsConfig, bool $expected): void
+ {
+ $GLOBALS['TYPO3_CONF_VARS']['BE']['defaultUserTSconfig'] = $tsConfig;
+
+ $subject = $this->setUpBackendUser($userId);
+ self::assertEquals($expected, $subject->isImportEnabled());
+ }
+
+ public function isExportEnabledDataProvider(): array
+ {
+ return [
+ 'admin user' => [
+ 1,
+ '',
+ true,
+ ],
+ 'editor user' => [
+ 2,
+ '',
+ false,
+ ],
+ 'editor user - enableExportForNonAdminUser = 1' => [
+ 2,
+ 'options.impexp.enableExportForNonAdminUser = 1',
+ true,
+ ],
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider isExportEnabledDataProvider
+ */
+ public function isExportEnabledReturnsExpectedValues(int $userId, string $tsConfig, bool $expected): void
+ {
+ $GLOBALS['TYPO3_CONF_VARS']['BE']['defaultUserTSconfig'] = $tsConfig;
+
+ $subject = $this->setUpBackendUser($userId);
+ self::assertEquals($expected, $subject->isExportEnabled());
+ }
}
diff --git a/typo3/sysext/impexp/Classes/ContextMenu/ItemProvider.php b/typo3/sysext/impexp/Classes/ContextMenu/ItemProvider.php
index a253f0f85595..f1fa50a586ee 100644
--- a/typo3/sysext/impexp/Classes/ContextMenu/ItemProvider.php
+++ b/typo3/sysext/impexp/Classes/ContextMenu/ItemProvider.php
@@ -97,10 +97,10 @@ class ItemProvider extends AbstractProvider
$canRender = false;
switch ($itemName) {
case 'exportT3d':
- $canRender = true;
+ $canRender = $this->backendUser->isExportEnabled();
break;
case 'importT3d':
- $canRender = $this->table === 'pages' && $this->isImportEnabled();
+ $canRender = $this->table === 'pages' && $this->backendUser->isImportEnabled();
break;
}
return $canRender;
@@ -131,13 +131,4 @@ class ItemProvider extends AbstractProvider
return $attributes;
}
-
- /**
- * Check if import functionality is available for current user
- */
- protected function isImportEnabled(): bool
- {
- return $this->backendUser->isAdmin()
- || (bool)($this->backendUser->getTSConfig()['options.']['impexp.']['enableImportForNonAdminUser'] ?? false);
- }
}
diff --git a/typo3/sysext/impexp/Classes/Controller/ExportController.php b/typo3/sysext/impexp/Classes/Controller/ExportController.php
index 2536a113a0b5..adb9ae35ee29 100644
--- a/typo3/sysext/impexp/Classes/Controller/ExportController.php
+++ b/typo3/sysext/impexp/Classes/Controller/ExportController.php
@@ -96,6 +96,14 @@ class ExportController extends ImportExportController
*/
public function mainAction(ServerRequestInterface $request): ResponseInterface
{
+ if ($this->getBackendUser()->isExportEnabled() === false) {
+ throw new \RuntimeException(
+ 'Export module is disabled for non admin users and '
+ . 'userTsConfig options.impexp.enableExportForNonAdminUser is not enabled.',
+ 1636901978
+ );
+ }
+
$this->main($request);
// Input data
diff --git a/typo3/sysext/impexp/Classes/Controller/ImportController.php b/typo3/sysext/impexp/Classes/Controller/ImportController.php
index 0aa82d978bd1..182446baf77d 100644
--- a/typo3/sysext/impexp/Classes/Controller/ImportController.php
+++ b/typo3/sysext/impexp/Classes/Controller/ImportController.php
@@ -76,7 +76,7 @@ class ImportController extends ImportExportController
*/
public function mainAction(ServerRequestInterface $request): ResponseInterface
{
- if ($this->isImportEnabled() === false) {
+ if ($this->getBackendUser()->isImportEnabled() === false) {
throw new \RuntimeException(
'Import module is disabled for non admin users and '
. 'userTsConfig options.impexp.enableImportForNonAdminUser is not enabled.',
@@ -107,17 +107,6 @@ class ImportController extends ImportExportController
return new HtmlResponse($this->moduleTemplate->renderContent());
}
- /**
- * Check if import functionality is available for current user
- *
- * @return bool
- */
- protected function isImportEnabled(): bool
- {
- return $this->getBackendUser()->isAdmin()
- || (bool)($this->getBackendUser()->getTSConfig()['options.']['impexp.']['enableImportForNonAdminUser'] ?? false);
- }
-
protected function preprocessInputData(array $inData): array
{
if ($inData['new_import'] ?? false) {
diff --git a/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php b/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
index 654ab07da32f..2e0ff200afa2 100644
--- a/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
+++ b/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
@@ -55,6 +55,7 @@ class SecurityStatus implements RequestAwareStatusProviderInterface
'fileDenyPattern' => $this->getFileDenyPatternStatus(),
'htaccessUpload' => $this->getHtaccessUploadStatus(),
'exceptionHandler' => $this->getExceptionHandlerStatus(),
+ 'exportedFiles' => $this->getExportedFilesStatus(),
];
if ($request !== null) {
@@ -260,6 +261,51 @@ class SecurityStatus implements RequestAwareStatusProviderInterface
return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_exceptionHandler'), $value, $message, $severity);
}
+ protected function getExportedFilesStatus(): ReportStatus
+ {
+ $value = $this->getLanguageService()->getLL('status_ok');
+ $message = '';
+ $severity = ReportStatus::OK;
+
+ $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('sys_file');
+ $exportedFiles = $queryBuilder
+ ->select('storage', 'identifier')
+ ->from('sys_file')
+ ->where(
+ $queryBuilder->expr()->like(
+ 'identifier',
+ $queryBuilder->createNamedParameter('%/_temp_/importexport/%')
+ ),
+ $queryBuilder->expr()->or(
+ $queryBuilder->expr()->like(
+ 'identifier',
+ $queryBuilder->createNamedParameter('%.xml')
+ ),
+ $queryBuilder->expr()->like(
+ 'identifier',
+ $queryBuilder->createNamedParameter('%.t3d')
+ )
+ ),
+ )
+ ->executeQuery()
+ ->fetchAllAssociative();
+
+ if (count($exportedFiles) > 0) {
+ $files = [];
+ foreach ($exportedFiles as $exportedFile) {
+ $files[] = '<li>' . htmlspecialchars($exportedFile['storage'] . ':' . $exportedFile['identifier']) . '</li>';
+ }
+
+ $value = $this->getLanguageService()->getLL('status_insecure');
+ $severity = ReportStatus::WARNING;
+ $message = $this->getLanguageService()->getLL('status_exportedFiles_warningMessage');
+ $message .= '<ul>' . implode(PHP_EOL, $files) . '</ul>';
+ $message .= $this->getLanguageService()->getLL('status_exportedFiles_warningRecommendation');
+ }
+
+ return GeneralUtility::makeInstance(ReportStatus::class, $this->getLanguageService()->getLL('status_exportedFiles'), $value, $message, $severity);
+ }
+
/**
* @return LanguageService
*/
diff --git a/typo3/sysext/reports/Resources/Private/Language/locallang_reports.xlf b/typo3/sysext/reports/Resources/Private/Language/locallang_reports.xlf
index e8eebf0adf6f..6e5c86cad54a 100644
--- a/typo3/sysext/reports/Resources/Private/Language/locallang_reports.xlf
+++ b/typo3/sysext/reports/Resources/Private/Language/locallang_reports.xlf
@@ -156,12 +156,21 @@
<trans-unit id="status_exceptionHandler" resname="status_exceptionHandler">
<source>Exception Handler / Error Reporting</source>
</trans-unit>
+ <trans-unit id="status_exportedFiles" resname="status_exportedFiles">
+ <source>XML/T3D export files</source>
+ </trans-unit>
<trans-unit id="status_exceptionHandler_warningMessage" resname="status_exceptionHandler_warningMessage">
<source>Display Errors is set to 1 - errors will be displayed with the DebugExceptionHandler including stack traces.</source>
</trans-unit>
<trans-unit id="status_exceptionHandler_errorMessage" resname="status_exceptionHandler_errorMessage">
<source>Debug Exception Handler enabled in Production Context - will show full error messages including stack traces.</source>
</trans-unit>
+ <trans-unit id="status_exportedFiles_warningMessage" resname="status_exportedFiles_warningMessage">
+ <source>The following exported files where found:</source>
+ </trans-unit>
+ <trans-unit id="status_exportedFiles_warningRecommendation" resname="status_exportedFiles_warningRecommendation">
+ <source>It is recommended to delete exported files to avoid possible disclosure of exported data to backend users with lower/different access rights than user(s) who originally created the export(s).</source>
+ </trans-unit>
<trans-unit id="status_installToolPassword" resname="status_installToolPassword">
<source>Install Tool Password</source>
</trans-unit>
--
GitLab