Skip to content

Commit

Permalink
[SECURITY] Prevent arbitrary access to privileged resources via t3://
Browse files Browse the repository at this point in the history 
Resolves: #93571
Releases: main, 13.0, 12.4, 11.5
Change-Id: I9622bfa47ef9637cecaff4a790f742445f598682
Security-Bulletin: TYPO3-CORE-SA-2024-005
Security-References: CVE-2024-25120
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82943
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
  • Loading branch information
@bnf @ohader
bnf authored and ohader committed Feb 13, 2024
1 parent fa12667 commit 2de87ff
Show file tree
Hide file tree
Showing 19 changed files with 79 additions and 33 deletions.
5 changes: 0 additions & 5 deletions Build/phpstan/phpstan-baseline.neon
View file
Expand Up @@ -110,11 +110,6 @@ parameters:
count: 1
path: ../../typo3/sysext/backend/Classes/Form/Element/ImageManipulationElement.php

-
message: "#^If condition is always true\\.$#"
count: 2
path: ../../typo3/sysext/backend/Classes/Form/Element/InputLinkElement.php

-
message: "#^Right side of && is always true\\.$#"
count: 1
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/backend/Classes/Backend/Shortcut/ShortcutRepository.php
View file
Expand Up @@ -509,7 +509,7 @@ protected function initShortcuts(): array
$combinedIdentifier = (string)($arguments['id'] ?? '');
if ($combinedIdentifier !== '') {
$storage = GeneralUtility::makeInstance(StorageRepository::class)->findByCombinedIdentifier($combinedIdentifier);
if ($storage === null || $storage->getUid() === 0) {
if ($storage === null || $storage->isFallbackStorage()) {
// Continue, if invalid storage or disallowed fallback storage
continue;
}
Expand Down
17 changes: 11 additions & 6 deletions typo3/sysext/backend/Classes/Form/Element/InputLinkElement.php
View file
Expand Up @@ -27,6 +27,7 @@
use TYPO3\CMS\Core\Resource\Exception\InvalidPathException;
use TYPO3\CMS\Core\Resource\File;
use TYPO3\CMS\Core\Resource\Folder;
use TYPO3\CMS\Core\Type\Bitmask\Permission;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Core\Utility\StringUtility;
Expand Down Expand Up @@ -355,10 +356,12 @@ protected function getLinkExplanation(string $itemValue): array
}
}

$backendUser = $this->getBackendUser();
// Resolve the actual link
switch ($linkData['type']) {
case LinkService::TYPE_PAGE:
$pageRecord = BackendUtility::readPageAccess($linkData['pageuid'] ?? null, '1=1');
$pagePermissionClause = $backendUser->getPagePermsClause(Permission::PAGE_SHOW);
$pageRecord = BackendUtility::readPageAccess($linkData['pageuid'] ?? null, $pagePermissionClause);
// Is this a real page
if ($pageRecord['uid'] ?? 0) {
$fragmentTitle = '';
Expand Down Expand Up @@ -391,19 +394,19 @@ protected function getLinkExplanation(string $itemValue): array
];
break;
case LinkService::TYPE_FILE:
/** @var File $file */
/** @var File|null $file */
$file = $linkData['file'];
if ($file) {
if ($file && $file->checkActionPermission('read') && !$file->getStorage()->isFallbackStorage()) {
$data = [
'text' => $file->getPublicUrl(),
'icon' => $this->iconFactory->getIconForFileExtension($file->getExtension(), Icon::SIZE_SMALL)->render(),
];
}
break;
case LinkService::TYPE_FOLDER:
/** @var Folder $folder */
/** @var Folder|null $folder */
$folder = $linkData['folder'];
if ($folder) {
if ($folder && $folder->checkActionPermission('read') && !$folder->getStorage()->isFallbackStorage()) {
$data = [
'text' => $folder->getPublicUrl(),
'icon' => $this->iconFactory->getIcon('apps-filetree-folder-default', Icon::SIZE_SMALL)->render(),
Expand All @@ -413,7 +416,9 @@ protected function getLinkExplanation(string $itemValue): array
case LinkService::TYPE_RECORD:
$table = $this->data['pageTsConfig']['TCEMAIN.']['linkHandler.'][$linkData['identifier'] . '.']['configuration.']['table'] ?? '';
$record = BackendUtility::getRecord($table, $linkData['uid']);
if ($record) {
$pagePermissionClause = $backendUser->getPagePermsClause(Permission::PAGE_SHOW);
$hasPageAccess = BackendUtility::readPageAccess($record['pid'] ?? null, $pagePermissionClause) !== false;
if ($record && $hasPageAccess && $backendUser->check('tables_select', $table)) {
$recordTitle = BackendUtility::getRecordTitle($table, $record);
$tableTitle = $this->getLanguageService()->sL($GLOBALS['TCA'][$table]['ctrl']['title']);
$data = [
Expand Down
8 changes: 8 additions & 0 deletions typo3/sysext/core/Classes/LinkHandling/FileLinkHandler.php
View file
Expand Up @@ -20,6 +20,7 @@
use TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException;
use TYPO3\CMS\Core\Resource\FileInterface;
use TYPO3\CMS\Core\Resource\ResourceFactory;
use TYPO3\CMS\Core\Resource\Security\FileNameValidator;
use TYPO3\CMS\Core\Utility\GeneralUtility;

/**
Expand Down Expand Up @@ -77,6 +78,13 @@ public function resolveHandlerData(array $data): array
{
try {
$file = $this->resolveFile($data);
$fileNameValidator = GeneralUtility::makeInstance(FileNameValidator::class);
if (
!$fileNameValidator->isValid(basename($file->getIdentifier())) ||
!$fileNameValidator->isValid($file->getName())
) {
$file = null;
}
} catch (FileDoesNotExistException $e) {
$file = null;
}
Expand Down
35 changes: 29 additions & 6 deletions typo3/sysext/core/Classes/Resource/ResourceStorage.php
View file
Expand Up @@ -356,6 +356,17 @@ public function hasChildren()
return true;
}

/**
* Returns true if this storage is a virtual storage that provides
* access to all files in the project root.
*
* @internal
*/
public function isFallbackStorage(): bool
{
return $this->getUid() === 0;
}

/*********************************
* Capabilities
********************************/
Expand Down Expand Up @@ -727,7 +738,7 @@ public function checkFileActionPermission($action, FileInterface $file)
return false;
}
// Check 3: No action allowed on files for denied file extensions
if (!$this->checkFileExtensionPermission($file->getName())) {
if (!$this->checkValidFileExtension($file)) {
return false;
}
$isReadCheck = false;
Expand Down Expand Up @@ -839,6 +850,17 @@ protected function checkFileExtensionPermission($fileName)
return GeneralUtility::makeInstance(FileNameValidator::class)->isValid($fileName);
}

/**
* Check file extension of an existing file against the
* current file deny pattern.
*/
protected function checkValidFileExtension(FileInterface $file): bool
{
$fileNameValidator = GeneralUtility::makeInstance(FileNameValidator::class);
return $fileNameValidator->isValid($file->getName()) &&
$fileNameValidator->isValid(basename($file->getIdentifier()));
}

/**
* Assures read permission for given folder.
*
Expand Down Expand Up @@ -906,7 +928,7 @@ protected function assureFileReadPermission(FileInterface $file)
1375955429
);
}
if (!$this->checkFileExtensionPermission($file->getName())) {
if (!$this->checkValidFileExtension($file)) {
throw new IllegalFileExtensionException(
'You are not allowed to use that file extension. File: "' . $file->getName() . '"',
1375955430
Expand All @@ -928,7 +950,7 @@ protected function assureFileWritePermissions(FileInterface $file)
if (!$this->checkFileActionPermission('write', $file)) {
throw new InsufficientFileWritePermissionsException('Writing to file "' . $file->getIdentifier() . '" is not allowed.', 1330121088);
}
if (!$this->checkFileExtensionPermission($file->getName())) {
if (!$this->checkValidFileExtension($file)) {
throw new IllegalFileExtensionException('You are not allowed to edit a file with extension "' . $file->getExtension() . '"', 1366711933);
}
}
Expand Down Expand Up @@ -963,7 +985,7 @@ protected function assureFileReplacePermissions(FileInterface $file)
protected function assureFileDeletePermissions(FileInterface $file)
{
// Check for disallowed file extensions
if (!$this->checkFileExtensionPermission($file->getName())) {
if (!$this->checkValidFileExtension($file)) {
throw new IllegalFileExtensionException('You are not allowed to delete a file with extension "' . $file->getExtension() . '"', 1377778916);
}
// Check further permissions if file is not a processed file
Expand Down Expand Up @@ -1082,7 +1104,7 @@ protected function assureFileMovePermissions(FileInterface $file, Folder $target
protected function assureFileRenamePermissions(FileInterface $file, $targetFileName)
{
// Check if file extension is allowed
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkFileExtensionPermission($file->getName())) {
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkValidFileExtension($file)) {
throw new IllegalFileExtensionException('You are not allowed to rename a file with this extension. File given: "' . $file->getName() . '"', 1371466663);
}
// Check if user is allowed to rename
Expand Down Expand Up @@ -1127,7 +1149,7 @@ protected function assureFileCopyPermissions(FileInterface $file, Folder $target
throw new InsufficientFolderWritePermissionsException('You are not allowed to write to the target folder "' . $targetFolder->getIdentifier() . '"', 1319550435);
}
// Check for a valid file extension
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkFileExtensionPermission($file->getName())) {
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkValidFileExtension($file)) {
throw new IllegalFileExtensionException('You are not allowed to copy a file of that type.', 1319553317);
}
}
Expand Down Expand Up @@ -1779,6 +1801,7 @@ public function streamFile(
string $alternativeFilename = null,
string $overrideMimeType = null
): ResponseInterface {
$this->assureFileReadPermission($file);
if (!$this->driver instanceof StreamableDriverInterface) {
return $this->getPseudoStream($file, $asDownload, $alternativeFilename, $overrideMimeType);
}
Expand Down
7 changes: 2 additions & 5 deletions typo3/sysext/core/Classes/Resource/Security/StoragePermissionsAspect.php
View file
Expand Up @@ -42,13 +42,10 @@ public function addUserPermissionsToStorage(AfterResourceStorageInitializationEv
if (($GLOBALS['TYPO3_REQUEST'] ?? null) instanceof ServerRequestInterface
&& ApplicationType::fromRequest($GLOBALS['TYPO3_REQUEST'])->isBackend()
&& !$GLOBALS['BE_USER']->isAdmin()
&& !$storage->isFallbackStorage()
) {
$storage->setEvaluatePermissions(true);
if ($storage->getUid() > 0) {
$storage->setUserPermissions($GLOBALS['BE_USER']->getFilePermissionsForStorage($storage));
} else {
$storage->setEvaluatePermissions(false);
}
$storage->setUserPermissions($GLOBALS['BE_USER']->getFilePermissionsForStorage($storage));
$this->addFileMountsToStorage($storage);
}
}
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/core/Classes/Utility/File/ExtendedFileUtility.php
View file
Expand Up @@ -627,7 +627,7 @@ protected function getFileObject($identifier)
if (!is_object($object)) {
throw new InvalidFileException('The item ' . $identifier . ' was not a file or directory!!', 1320122453);
}
if ($object->getStorage()->getUid() === 0) {
if ($object->getStorage()->isFallbackStorage()) {
throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1375889830);
}
return $object;
Expand Down
2 changes: 2 additions & 0 deletions typo3/sysext/core/Tests/Unit/DataHandling/SoftReference/TypoLinkSoftReferenceParserTest.php
View file
Expand Up @@ -236,6 +236,8 @@ public function findRefReturnsParsedElementsWithFile(array $softrefConfiguration
{
$fileObject = $this->prophesize(File::class);
$fileObject->getUid()->willReturn(42)->shouldBeCalledTimes(1);
$fileObject->getName()->willReturn('download.jpg');
$fileObject->getIdentifier()->willReturn('fileadmin/download.jpg');

$resourceFactory = $this->prophesize(ResourceFactory::class);
$resourceFactory->getFileObject('42')->willReturn($fileObject->reveal());
Expand Down
2 changes: 2 additions & 0 deletions .../sysext/core/Tests/Unit/DataHandling/SoftReference/TypoLinkTagSoftReferenceParserTest.php
View file
Expand Up @@ -180,6 +180,8 @@ public function findRefReturnsParsedElementsWithFile(array $softrefConfiguration
{
$fileObject = $this->prophesize(File::class);
$fileObject->getUid()->willReturn(42)->shouldBeCalledTimes(1);
$fileObject->getName()->willReturn('download.jpg');
$fileObject->getIdentifier()->willReturn('fileadmin/download.jpg');

$resourceFactory = $this->prophesize(ResourceFactory::class);
$resourceFactory->getFileObject('42')->willReturn($fileObject->reveal());
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/core/Tests/Unit/LinkHandling/FileLinkHandlerTest.php
View file
Expand Up @@ -106,7 +106,7 @@ public function resolveFileReferencesToSplitParameters(array $input, array $expe
->getMock();

// fake methods to return proper objects
$fileObject = new File(['identifier' => $expected['file'], 'name' => 'foobar.txt'], $storage);
$fileObject = new File(['identifier' => 'fileadmin/deep/down.jpg', 'name' => 'down.jpg'], $storage);
$factory->method('getFileObject')->with($expected['file'])->willReturn($fileObject);
$factory->method('getFileObjectFromCombinedIdentifier')->with($expected['file'])->willReturn($fileObject);
$expected['file'] = $fileObject;
Expand Down
2 changes: 2 additions & 0 deletions typo3/sysext/core/Tests/UnitDeprecated/Database/SoftReferenceIndexTest.php
View file
Expand Up @@ -353,6 +353,8 @@ public function findRefReturnsParsedElementsWithFileDataProvider(): array
public function findRefReturnsParsedElementsWithFile(array $softrefConfiguration, array $expectedElement): void
{
$fileObject = $this->prophesize(File::class);
$fileObject->getName()->willReturn('download.jpg');
$fileObject->getIdentifier()->willReturn('fileadmin/download.jpg');
$fileObject->getUid()->willReturn(42)->shouldBeCalledTimes(count($softrefConfiguration));

$resourceFactory = $this->prophesize(ResourceFactory::class);
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/filelist/Classes/Controller/File/CreateFolderController.php
View file
Expand Up @@ -145,7 +145,7 @@ protected function init(ServerRequestInterface $request): void
$message = $this->getLanguageService()->sL('LLL:EXT:filelist/Resources/Private/Language/locallang_mod_file_list.xlf:targetNoDir');
throw new \RuntimeException($title . ': ' . $message, 1294586845);
}
if ($this->folderObject->getStorage()->getUid() === 0) {
if ($this->folderObject->getStorage()->isFallbackStorage()) {
throw new InsufficientFolderAccessPermissionsException(
'You are not allowed to access folders outside your storages',
1375889838
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/filelist/Classes/Controller/File/EditFileController.php
View file
Expand Up @@ -146,7 +146,7 @@ protected function init(ServerRequestInterface $request): void
$message = $this->getLanguageService()->sL('LLL:EXT:filelist/Resources/Private/Language/locallang_mod_file_list.xlf:targetNoDir');
throw new \RuntimeException($title . ': ' . $message, 1294586841);
}
if ($this->fileObject->getStorage()->getUid() === 0) {
if ($this->fileObject->getStorage()->isFallbackStorage()) {
throw new InsufficientFileAccessPermissionsException(
'You are not allowed to access files outside your storages',
1375889832
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/filelist/Classes/Controller/File/FileUploadController.php
View file
Expand Up @@ -132,7 +132,7 @@ protected function init(ServerRequestInterface $request): void
if ($this->target) {
$this->folderObject = $this->resourceFactory->retrieveFileOrFolderObject($this->target);
}
if ($this->folderObject->getStorage()->getUid() === 0) {
if ($this->folderObject->getStorage()->isFallbackStorage()) {
throw new InsufficientFolderAccessPermissionsException(
'You are not allowed to access folders outside your storages',
1375889834
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/filelist/Classes/Controller/File/RenameFileController.php
View file
Expand Up @@ -128,7 +128,7 @@ protected function init(ServerRequestInterface $request): void
$message = $this->getLanguageService()->sL('LLL:EXT:filelist/Resources/Private/Language/locallang_mod_file_list.xlf:targetNoDir');
throw new \RuntimeException($title . ': ' . $message, 1294586844);
}
if ($this->fileOrFolderObject->getStorage()->getUid() === 0) {
if ($this->fileOrFolderObject->getStorage()->isFallbackStorage()) {
throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1375889840);
}

Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/filelist/Classes/Controller/File/ReplaceFileController.php
View file
Expand Up @@ -128,7 +128,7 @@ protected function init(ServerRequestInterface $request): void
$message = $lang->sL('LLL:EXT:filelist/Resources/Private/Language/locallang_mod_file_list.xlf:targetNoDir');
throw new \RuntimeException($title . ': ' . $message, 1436895930);
}
if ($this->fileOrFolderObject->getStorage()->getUid() === 0) {
if ($this->fileOrFolderObject->getStorage()->isFallbackStorage()) {
throw new InsufficientFileAccessPermissionsException(
'You are not allowed to access files outside your storages',
1436895931
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/filelist/Classes/Controller/FileListController.php
View file
Expand Up @@ -127,7 +127,7 @@ public function handleRequest(ServerRequestInterface $request): ResponseInterfac
}
$this->folderObject = $storage->getFolder($identifier);
// Disallow access to fallback storage 0
if ($storage->getUid() === 0) {
if ($storage->isFallbackStorage()) {
throw new InsufficientFolderAccessPermissionsException(
'You are not allowed to access files outside your storages',
1434539815
Expand Down
3 changes: 3 additions & 0 deletions typo3/sysext/recordlist/Classes/LinkHandler/FileLinkHandler.php
View file
Expand Up @@ -123,6 +123,9 @@ public function render(ServerRequestInterface $request)

// Create upload/create folder forms, if a path is given
$selectedFolder = $this->getSelectedFolder($this->expandFolder);
if ($selectedFolder->getStorage()->isFallbackStorage()) {
$selectedFolder = null;
}

// Build the file upload and folder creation form
if ($selectedFolder) {
Expand Down
13 changes: 11 additions & 2 deletions typo3/sysext/recordlist/Classes/LinkHandler/PageLinkHandler.php
View file
Expand Up @@ -24,6 +24,7 @@
use TYPO3\CMS\Core\Domain\Repository\PageRepository;
use TYPO3\CMS\Core\Imaging\Icon;
use TYPO3\CMS\Core\LinkHandling\LinkService;
use TYPO3\CMS\Core\Type\Bitmask\Permission;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Recordlist\Tree\View\LinkParameterProviderInterface;
Expand Down Expand Up @@ -86,11 +87,19 @@ public function formatCurrentUrl()
$titleLen = (int)$this->getBackendUser()->uc['titleLen'];

$id = (int)$this->linkParts['url']['pageuid'];
$pageTitle = BackendUtility::getRecordWSOL('pages', $id, 'title')['title'] ?? '';

$idInfo = 'ID: ' . $id . (!empty($this->linkParts['url']['fragment']) ? ', #' . $this->linkParts['url']['fragment'] : '');

$permsClause = $this->getBackendUser()->getPagePermsClause(Permission::PAGE_SHOW);
$pageRecord = BackendUtility::readPageAccess($id, $permsClause);
if ($pageRecord === false) {
return $lang->getLL('page') . ' ' . $idInfo;
}

$pageTitle = $pageRecord['title'] ?? '';
return $lang->getLL('page')
. ($pageTitle ? ' \'' . GeneralUtility::fixed_lgd_cs($pageTitle, $titleLen) . '\'' : '')
. ' (ID: ' . $id . (!empty($this->linkParts['url']['fragment']) ? ', #' . $this->linkParts['url']['fragment'] : '') . ')';
. ' (' . $idInfo . ')';
}

/**
Expand Down

0 comments on commit 2de87ff

Please sign in to comment.