From 3aef4183ef2b00442f12f89961d27c300cc4c2fb Mon Sep 17 00:00:00 2001
From: Torben Hansen <derhansen@gmail.com>
Date: Tue, 14 Jun 2022 09:11:14 +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/+/74892
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 | 31 ++++++---
.../BackendUserAuthenticationTest.php | 66 +++++++++++++++++++
.../Classes/ContextMenu/ItemProvider.php | 13 +---
.../Classes/Controller/ExportController.php | 8 +++
.../Classes/Controller/ImportController.php | 4 +-
.../Classes/Report/Status/SecurityStatus.php | 48 +++++++++++++-
.../Private/Language/locallang_reports.xlf | 9 +++
9 files changed, 233 insertions(+), 23 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 05cf595b4cfa..2bca0908bccd 100644
--- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
+++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
@@ -2716,4 +2716,26 @@ TCAdefaults.sys_note.email = ' . $this->user['email'];
);
}
}
+
+ /**
+ * 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 640ee2d08b50..6190e0cbc974 100644
--- a/typo3/sysext/core/Classes/Resource/ResourceStorage.php
+++ b/typo3/sysext/core/Classes/Resource/ResourceStorage.php
@@ -70,6 +70,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;
@@ -822,7 +823,7 @@ class ResourceStorage implements ResourceStorageInterface
public function checkFileAndFolderNameFilters(ResourceInterface $fileOrFolder)
{
trigger_error(__METHOD__ . ' is deprecated. This method will be removed without substitution, use the ResourceStorage API instead', \E_USER_DEPRECATED);
- foreach ($this->fileAndFolderNameFilters as $filter) {
+ foreach ($this->getFileAndFolderNameFilters() as $filter) {
if (is_callable($filter)) {
$result = call_user_func($filter, $fileOrFolder->getName(), $fileOrFolder->getIdentifier(), $fileOrFolder->getParentFolder()->getIdentifier(), [], $this->driver);
// We have to use -1 as the „don't include“ return value, as call_user_func() will return FALSE
@@ -1517,6 +1518,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.
*
@@ -1524,7 +1538,7 @@ class ResourceStorage implements ResourceStorageInterface
*/
public function getFileAndFolderNameFilters()
{
- return $this->fileAndFolderNameFilters;
+ return array_merge($this->fileAndFolderNameFilters, [$this->getImportExportFilter()]);
}
/**
@@ -1589,7 +1603,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 = [];
@@ -1619,7 +1633,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);
}
@@ -1633,7 +1647,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);
}
@@ -1645,7 +1659,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);
}
@@ -2410,7 +2424,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);
@@ -2421,6 +2435,7 @@ class ResourceStorage implements ResourceStorageInterface
unset($folderIdentifiers[$processingIdentifier]);
}
}
+
$folders = [];
foreach ($folderIdentifiers as $folderIdentifier) {
$folders[$folderIdentifier] = $this->getFolder($folderIdentifier, true);
@@ -2438,7 +2453,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/Functional/Authentication/BackendUserAuthenticationTest.php b/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
index 1756bd689723..0798494dd647 100644
--- a/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
+++ b/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
@@ -128,4 +128,70 @@ class BackendUserAuthenticationTest extends FunctionalTestCase
$folder = $this->subject->getDefaultUploadFolder();
self::assertEquals('/' . $path . '/', $folder->getIdentifier());
}
+
+ 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 5cc9b42484c7..ba639cc85af5 100644
--- a/typo3/sysext/impexp/Classes/ContextMenu/ItemProvider.php
+++ b/typo3/sysext/impexp/Classes/ContextMenu/ItemProvider.php
@@ -94,10 +94,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;
@@ -113,13 +113,4 @@ class ItemProvider extends AbstractProvider
{
return ['data-callback-module' => 'TYPO3/CMS/Impexp/ContextMenuActions'];
}
-
- /**
- * Check if import functionality is available for current user
- */
- protected function isImportEnabled(): bool
- {
- return $this->backendUser->isAdmin()
- || !$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 cdd20f967e92..df013cb37575 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->lang->includeLLFile('EXT:impexp/Resources/Private/Language/locallang.xlf');
$this->pageinfo = BackendUtility::readPageAccess($this->id, $this->perms_clause);
diff --git a/typo3/sysext/impexp/Classes/Controller/ImportController.php b/typo3/sysext/impexp/Classes/Controller/ImportController.php
index 55a6c8240e17..baf1d5b1531d 100644
--- a/typo3/sysext/impexp/Classes/Controller/ImportController.php
+++ b/typo3/sysext/impexp/Classes/Controller/ImportController.php
@@ -108,9 +108,7 @@ class ImportController extends ImportExportController
$this->standaloneView->assign('id', $this->id);
$this->standaloneView->assign('inData', $inData);
- $backendUser = $this->getBackendUser();
- $isEnabledForNonAdmin = (bool)($backendUser->getTSConfig()['options.']['impexp.']['enableImportForNonAdminUser'] ?? false);
- if (!$backendUser->isAdmin() && !$isEnabledForNonAdmin) {
+ if ($this->getBackendUser()->isImportEnabled() === false) {
throw new \RuntimeException(
'Import module is disabled for non admin users and '
. 'userTsConfig options.impexp.enableImportForNonAdminUser is not enabled.',
diff --git a/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php b/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
index 2968fd5973fd..d2bd02faf258 100644
--- a/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
+++ b/typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
@@ -53,7 +53,8 @@ class SecurityStatus implements RequestAwareStatusProviderInterface
'adminUserAccount' => $this->getAdminAccountStatus(),
'fileDenyPattern' => $this->getFileDenyPatternStatus(),
'htaccessUpload' => $this->getHtaccessUploadStatus(),
- 'exceptionHandler' => $this->getExceptionHandlerStatus()
+ 'exceptionHandler' => $this->getExceptionHandlerStatus(),
+ 'exportedFiles' => $this->getExportedFilesStatus(),
];
if ($request !== null) {
@@ -261,6 +262,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()->orX(
+ $queryBuilder->expr()->like(
+ 'identifier',
+ $queryBuilder->createNamedParameter('%.xml')
+ ),
+ $queryBuilder->expr()->like(
+ 'identifier',
+ $queryBuilder->createNamedParameter('%.t3d')
+ )
+ )
+ )
+ ->execute()
+ ->fetchAll();
+
+ 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 9941ea565eba..806c1cbf8346 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