[SECURITY] Prevent arbitrary access to privileged resources via t3://

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/+/82955 Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · Feb 13, 2024 · ae0dfc4 · ae0dfc4
1 parent 14d1013
commit ae0dfc4
Show file tree

Hide file tree

Showing 19 changed files with 110 additions and 36 deletions.
diff --git a/typo3/sysext/backend/Classes/Backend/Shortcut/ShortcutRepository.php b/typo3/sysext/backend/Classes/Backend/Shortcut/ShortcutRepository.php
@@ -405,7 +405,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;
  }

diff --git a/typo3/sysext/backend/Classes/Controller/LinkController.php b/typo3/sysext/backend/Classes/Controller/LinkController.php
@@ -56,7 +56,7 @@ public function resourceAction(ServerRequestInterface $request): ResponseInterfa
  if (!$resource instanceof File && !$resource instanceof Folder) {
  throw new \InvalidArgumentException('Resource must be a file or a folder', 1679039649);
  }
- if ($resource->getStorage()->getUid() === 0) {
+ if ($resource->getStorage()->isFallbackStorage()) {
  throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1679039650);
  }
  if ($resource instanceof File) {

diff --git a/typo3/sysext/backend/Classes/Controller/Resource/ResourceController.php b/typo3/sysext/backend/Classes/Controller/Resource/ResourceController.php
@@ -54,7 +54,7 @@ public function renameResourceAction(ServerRequestInterface $request): ResponseI
  if (!$origin instanceof File && !$origin instanceof Folder) {
  throw new \InvalidArgumentException('Resource must be a file or a folder', 1676979120);
  }
- if ($origin->getStorage()->getUid() === 0) {
+ if ($origin->getStorage()->isFallbackStorage()) {
  throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1676299579);
  }
  if (!$origin->checkActionPermission('rename')) {

diff --git a/typo3/sysext/backend/Classes/Form/Element/LinkElement.php b/typo3/sysext/backend/Classes/Form/Element/LinkElement.php
@@ -32,6 +32,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;
@@ -339,10 +340,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 = '';
@@ -376,7 +379,7 @@ protected function getLinkExplanation(string $itemValue): array
  break;
  case LinkService::TYPE_FILE:
  $file = $linkData['file'] ?? null;
- if ($file instanceof File) {
+ if ($file instanceof File && $file->checkActionPermission('read') && !$file->getStorage()->isFallbackStorage()) {
  $data = [
  'text' => $file->getPublicUrl(),
  'icon' => $this->iconFactory->getIconForFileExtension($file->getExtension(), IconSize::SMALL)->render(),
@@ -385,7 +388,7 @@ protected function getLinkExplanation(string $itemValue): array
  break;
  case LinkService::TYPE_FOLDER:
  $folder = $linkData['folder'] ?? null;
- if ($folder instanceof Folder) {
+ if ($folder instanceof Folder && $folder->checkActionPermission('read') && !$folder->getStorage()->isFallbackStorage()) {
  $data = [
  'text' => $folder->getPublicUrl(),
  'icon' => $this->iconFactory->getIcon('apps-filetree-folder-default', IconSize::SMALL)->render(),
@@ -395,7 +398,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 = [

diff --git a/typo3/sysext/backend/Classes/LinkHandler/PageLinkHandler.php b/typo3/sysext/backend/Classes/LinkHandler/PageLinkHandler.php
@@ -27,6 +27,7 @@
 use TYPO3\CMS\Core\Domain\Repository\PageRepository;
 use TYPO3\CMS\Core\Imaging\IconSize;
 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;
 
@@ -89,11 +90,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->sL('LLL:EXT:backend/Resources/Private/Language/locallang_browse_links.xlf:page') . ' ' . $idInfo;
+ }
+
+ $pageTitle = $pageRecord['title'] ?? '';
  return $lang->sL('LLL:EXT:backend/Resources/Private/Language/locallang_browse_links.xlf:page')
  . ($pageTitle ? ' \'' . GeneralUtility::fixed_lgd_cs($pageTitle, $titleLen) . '\'' : '')
- . ' (ID: ' . $id . (!empty($this->linkParts['url']['fragment']) ? ', #' . $this->linkParts['url']['fragment'] : '') . ')';
+ . ' (' . $idInfo . ')';
  }
 
  /**

diff --git a/typo3/sysext/core/Classes/LinkHandling/FileLinkHandler.php b/typo3/sysext/core/Classes/LinkHandling/FileLinkHandler.php
@@ -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;
 
 /**
@@ -71,6 +72,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;
  }

diff --git a/typo3/sysext/core/Classes/LinkHandling/LegacyLinkNotationConverter.php b/typo3/sysext/core/Classes/LinkHandling/LegacyLinkNotationConverter.php
@@ -220,7 +220,7 @@ protected function getFileOrFolderObjectFromMixedIdentifier(string $mixedIdentif
  }
  $fileOrFolderObject = $this->getResourceFactory()->retrieveFileOrFolderObject($fileIdentifier);
  // Links to a file/folder in the main TYPO3 directory should not be considered as file links, but an external link
- if ($fileOrFolderObject instanceof ResourceInterface && $fileOrFolderObject->getStorage()->getUid() === 0) {
+ if ($fileOrFolderObject instanceof ResourceInterface && $fileOrFolderObject->getStorage()->isFallbackStorage()) {
  return [
  'type' => LinkService::TYPE_URL,
  'url' => $mixedIdentifier,

diff --git a/typo3/sysext/core/Classes/Resource/ResourceStorage.php b/typo3/sysext/core/Classes/Resource/ResourceStorage.php
@@ -362,6 +362,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
  ********************************/
@@ -719,7 +730,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;
@@ -831,6 +842,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.
  *
@@ -897,7 +919,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
@@ -918,7 +940,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);
  }
  }
@@ -951,7 +973,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
@@ -1072,7 +1094,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
@@ -1115,7 +1137,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);
  }
  }
@@ -1736,6 +1758,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);
  }

diff --git a/typo3/sysext/core/Classes/Resource/Security/StoragePermissionsAspect.php b/typo3/sysext/core/Classes/Resource/Security/StoragePermissionsAspect.php
@@ -47,13 +47,10 @@ public function addUserPermissionsToStorage(AfterResourceStorageInitializationEv
  if (($GLOBALS['TYPO3_REQUEST'] ?? null) instanceof ServerRequestInterface
  && ApplicationType::fromRequest($GLOBALS['TYPO3_REQUEST'])->isBackend()
  && !$this->getBackendUser()->isAdmin()
+ && !$storage->isFallbackStorage()
  ) {
  $storage->setEvaluatePermissions(true);
- if ($storage->getUid() > 0) {
- $storage->setUserPermissions($this->getFilePermissionsForStorage($storage));
- } else {
- $storage->setEvaluatePermissions(false);
- }
+ $storage->setUserPermissions($this->getFilePermissionsForStorage($storage));
  $this->addFileMountsToStorage($storage);
  }
  }

diff --git a/typo3/sysext/core/Classes/Utility/File/ExtendedFileUtility.php b/typo3/sysext/core/Classes/Utility/File/ExtendedFileUtility.php
@@ -572,7 +572,7 @@ protected function getFileObject($identifier)
  if ($object === null) {
  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;

diff --git a/typo3/sysext/core/Tests/Unit/DataHandling/SoftReference/TypoLinkSoftReferenceParserTest.php b/typo3/sysext/core/Tests/Unit/DataHandling/SoftReference/TypoLinkSoftReferenceParserTest.php
@@ -260,6 +260,9 @@ public function findRefReturnsParsedElementsWithFile(array $softrefConfiguration
  $storageObject->method('getUid')->willReturn(1);
  $fileObject = $this->createMock(File::class);
  $fileObject->expects(self::once())->method('getUid')->willReturn(42);
+ $fileObject->expects(self::any())->method('getName')->willReturn('download.jpg');
+ $fileObject->expects(self::any())->method('getIdentifier')->willReturn('fileadmin/download.jpg');
+
  $fileObject->expects(self::any())->method('getStorage')->willReturn($storageObject);
 
  $resourceFactory = $this->createMock(ResourceFactory::class);

diff --git a/.../sysext/core/Tests/Unit/DataHandling/SoftReference/TypoLinkTagSoftReferenceParserTest.php b/.../sysext/core/Tests/Unit/DataHandling/SoftReference/TypoLinkTagSoftReferenceParserTest.php
@@ -200,6 +200,8 @@ public function findRefReturnsParsedElementsWithFile(array $softrefConfiguration
  {
  $fileObject = $this->createMock(File::class);
  $fileObject->expects(self::once())->method('getUid')->willReturn(42);
+ $fileObject->expects(self::any())->method('getName')->willReturn('download.jpg');
+ $fileObject->expects(self::any())->method('getIdentifier')->willReturn('fileadmin/download.jpg');
 
  $resourceFactory = $this->createMock(ResourceFactory::class);
  $resourceFactory->method('getFileObject')->with('42')->willReturn($fileObject);

diff --git a/typo3/sysext/core/Tests/Unit/LinkHandling/FileLinkHandlerTest.php b/typo3/sysext/core/Tests/Unit/LinkHandling/FileLinkHandlerTest.php
@@ -98,7 +98,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;

diff --git a/typo3/sysext/filelist/Classes/Controller/File/CreateFileController.php b/typo3/sysext/filelist/Classes/Controller/File/CreateFileController.php
@@ -121,7 +121,7 @@ protected function initialize(ServerRequestInterface $request): void
  $message = $this->getLanguageService()->sL('LLL:EXT:filelist/Resources/Private/Language/locallang_mod_file_list.xlf:targetNoDir');
  throw new \RuntimeException($title . ': ' . $message, 1667565756);
  }
- if ($this->folderObject->getStorage()->getUid() === 0) {
+ if ($this->folderObject->getStorage()->isFallbackStorage()) {
  throw new InsufficientFolderAccessPermissionsException(
  'You are not allowed to access folders outside your storages',
  1667565757

diff --git a/typo3/sysext/filelist/Classes/Controller/File/EditFileController.php b/typo3/sysext/filelist/Classes/Controller/File/EditFileController.php
@@ -121,7 +121,7 @@ public function mainAction(ServerRequestInterface $request): ResponseInterface
  if (!$file instanceof FileInterface) {
  throw new InvalidFileException('Referenced target "' . $combinedIdentifier . '" could not be resolved to a valid file', 1294586841);
  }
- if ($file->getStorage()->getUid() === 0) {
+ if ($file->getStorage()->isFallbackStorage()) {
  throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1375889832);
  }
 

diff --git a/typo3/sysext/filelist/Classes/Controller/File/FileUploadController.php b/typo3/sysext/filelist/Classes/Controller/File/FileUploadController.php
@@ -60,7 +60,7 @@ public function mainAction(ServerRequestInterface $request): ResponseInterface
  $folder = $this->resourceFactory->retrieveFileOrFolderObject($targetFolderCombinedIdentifier);
 
  if (!$folder instanceof FolderInterface
- || $folder->getStorage()->getUid() === 0
+ || $folder->getStorage()->isFallbackStorage()
  ) {
  throw new InsufficientFolderAccessPermissionsException('You are not allowed to access folders outside your storages, or the folder couldn\'t be resolved', 1375889834);
  }

diff --git a/typo3/sysext/filelist/Classes/Controller/File/ReplaceFileController.php b/typo3/sysext/filelist/Classes/Controller/File/ReplaceFileController.php
@@ -106,7 +106,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

diff --git a/typo3/sysext/filelist/Classes/Controller/FileListController.php b/typo3/sysext/filelist/Classes/Controller/FileListController.php
@@ -126,7 +126,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