Commit 7447a3d1 authored by Torben Hansen's avatar Torben Hansen Committed by Oliver Hader
Browse files

[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/+/74902


Tested-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
parent 7879a3de
......@@ -2289,4 +2289,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);
}
}
<?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;
}
}
......@@ -74,6 +74,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;
......@@ -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);
}
......@@ -2417,7 +2431,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);
......@@ -2428,6 +2442,7 @@ class ResourceStorage implements ResourceStorageInterface
unset($folderIdentifiers[$processingIdentifier]);
}
}
$folders = [];
foreach ($folderIdentifiers as $folderIdentifier) {
$folders[$folderIdentifier] = $this->getFolder($folderIdentifier, true);
......@@ -2445,7 +2460,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);
}
......
......@@ -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');
}
......
......@@ -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());
}
}
......@@ -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);
}
}
......@@ -81,6 +81,14 @@ class ExportController
public function handleRequest(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
);
}
$backendUser = $this->getBackendUser();
$queryParams = $request->getQueryParams();
$parsedBody = $request->getParsedBody();
......
......@@ -59,7 +59,7 @@ class ImportController
public function handleRequest(ServerRequestInterface $request): ResponseInterface
{
if (!$this->isImportEnabled()) {
if (!$this->getBackendUser()->isImportEnabled()) {
throw new \RuntimeException(
'Import module is disabled for non admin users and userTsConfig options.impexp.enableImportForNonAdminUser is not enabled.',
1464435459
......@@ -142,15 +142,6 @@ class ImportController
$buttonBar->addButton($viewButton);
}
/**
* Check if import functionality is available for current user.
*/
protected function isImportEnabled(): bool
{
$backendUser = $this->getBackendUser();
return $backendUser->isAdmin() || ($backendUser->getTSConfig()['options.']['impexp.']['enableImportForNonAdminUser'] ?? false);
}
protected function handleFileUpload(ServerRequestInterface $request): ?File
{
$parsedBody = $request->getParsedBody() ?? [];
......
......@@ -55,6 +55,7 @@ class SecurityStatus implements RequestAwareStatusProviderInterface
'fileDenyPattern' => $this->getFileDenyPatternStatus(),
'htaccessUpload' => $this->getHtaccessUploadStatus(),
'exceptionHandler' => $this->getExceptionHandlerStatus(),
'exportedFiles' => $this->getExportedFilesStatus(),
];
if ($request !== null) {
......@@ -265,6 +266,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);
}
protected function getLanguageService(): LanguageService
{
return $GLOBALS['LANG'];
......
......@@ -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_installTool" resname="status_installTool">
<source>Install Tool</source>
</trans-unit>
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment