Skip to content

Commit 33f4d27

Browse files
bnfohader
bnf
authored and
ohader
committed
[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/+/82949 Reviewed-by: Oliver Hader <[email protected]> Tested-by: Oliver Hader <[email protected]>
1 parent df48637 commit 33f4d27

File tree

19 files changed

+110
-36
lines changed

19 files changed

+110
-36
lines changed

‎typo3/sysext/backend/Classes/Backend/Shortcut/ShortcutRepository.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ protected function initShortcuts(): array
405405
$combinedIdentifier = (string)($arguments['id'] ?? '');
406406
if ($combinedIdentifier !== '') {
407407
$storage = GeneralUtility::makeInstance(StorageRepository::class)->findByCombinedIdentifier($combinedIdentifier);
408-
if ($storage === null || $storage->getUid() === 0) {
408+
if ($storage === null || $storage->isFallbackStorage()) {
409409
// Continue, if invalid storage or disallowed fallback storage
410410
continue;
411411
}

‎typo3/sysext/backend/Classes/Controller/LinkController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function resourceAction(ServerRequestInterface $request): ResponseInterfa
5656
if (!$resource instanceof File && !$resource instanceof Folder) {
5757
throw new \InvalidArgumentException('Resource must be a file or a folder', 1679039649);
5858
}
59-
if ($resource->getStorage()->getUid() === 0) {
59+
if ($resource->getStorage()->isFallbackStorage()) {
6060
throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1679039650);
6161
}
6262
if ($resource instanceof File) {

‎typo3/sysext/backend/Classes/Controller/Resource/ResourceController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public function renameResourceAction(ServerRequestInterface $request): ResponseI
5454
if (!$origin instanceof File && !$origin instanceof Folder) {
5555
throw new \InvalidArgumentException('Resource must be a file or a folder', 1676979120);
5656
}
57-
if ($origin->getStorage()->getUid() === 0) {
57+
if ($origin->getStorage()->isFallbackStorage()) {
5858
throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1676299579);
5959
}
6060
if (!$origin->checkActionPermission('rename')) {

‎typo3/sysext/backend/Classes/Form/Element/LinkElement.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use TYPO3\CMS\Core\Resource\Exception\InvalidPathException;
3232
use TYPO3\CMS\Core\Resource\File;
3333
use TYPO3\CMS\Core\Resource\Folder;
34+
use TYPO3\CMS\Core\Type\Bitmask\Permission;
3435
use TYPO3\CMS\Core\Utility\GeneralUtility;
3536
use TYPO3\CMS\Core\Utility\MathUtility;
3637
use TYPO3\CMS\Core\Utility\StringUtility;
@@ -335,10 +336,12 @@ protected function getLinkExplanation(string $itemValue): array
335336
}
336337
}
337338

339+
$backendUser = $this->getBackendUser();
338340
// Resolve the actual link
339341
switch ($linkData['type']) {
340342
case LinkService::TYPE_PAGE:
341-
$pageRecord = BackendUtility::readPageAccess($linkData['pageuid'] ?? null, '1=1');
343+
$pagePermissionClause = $backendUser->getPagePermsClause(Permission::PAGE_SHOW);
344+
$pageRecord = BackendUtility::readPageAccess($linkData['pageuid'] ?? null, $pagePermissionClause);
342345
// Is this a real page
343346
if ($pageRecord['uid'] ?? 0) {
344347
$fragmentTitle = '';
@@ -372,7 +375,7 @@ protected function getLinkExplanation(string $itemValue): array
372375
break;
373376
case LinkService::TYPE_FILE:
374377
$file = $linkData['file'] ?? null;
375-
if ($file instanceof File) {
378+
if ($file instanceof File && $file->checkActionPermission('read') && !$file->getStorage()->isFallbackStorage()) {
376379
$data = [
377380
'text' => $file->getPublicUrl(),
378381
'icon' => $this->iconFactory->getIconForFileExtension($file->getExtension(), Icon::SIZE_SMALL)->render(),
@@ -381,7 +384,7 @@ protected function getLinkExplanation(string $itemValue): array
381384
break;
382385
case LinkService::TYPE_FOLDER:
383386
$folder = $linkData['folder'] ?? null;
384-
if ($folder instanceof Folder) {
387+
if ($folder instanceof Folder && $folder->checkActionPermission('read') && !$folder->getStorage()->isFallbackStorage()) {
385388
$data = [
386389
'text' => $folder->getPublicUrl(),
387390
'icon' => $this->iconFactory->getIcon('apps-filetree-folder-default', Icon::SIZE_SMALL)->render(),
@@ -391,7 +394,9 @@ protected function getLinkExplanation(string $itemValue): array
391394
case LinkService::TYPE_RECORD:
392395
$table = $this->data['pageTsConfig']['TCEMAIN.']['linkHandler.'][$linkData['identifier'] . '.']['configuration.']['table'] ?? '';
393396
$record = BackendUtility::getRecord($table, $linkData['uid']);
394-
if ($record) {
397+
$pagePermissionClause = $backendUser->getPagePermsClause(Permission::PAGE_SHOW);
398+
$hasPageAccess = BackendUtility::readPageAccess($record['pid'] ?? null, $pagePermissionClause) !== false;
399+
if ($record && $hasPageAccess && $backendUser->check('tables_select', $table)) {
395400
$recordTitle = BackendUtility::getRecordTitle($table, $record);
396401
$tableTitle = $this->getLanguageService()->sL($GLOBALS['TCA'][$table]['ctrl']['title']);
397402
$data = [

‎typo3/sysext/backend/Classes/LinkHandler/PageLinkHandler.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use TYPO3\CMS\Core\Domain\Repository\PageRepository;
2626
use TYPO3\CMS\Core\Imaging\Icon;
2727
use TYPO3\CMS\Core\LinkHandling\LinkService;
28+
use TYPO3\CMS\Core\Type\Bitmask\Permission;
2829
use TYPO3\CMS\Core\Utility\GeneralUtility;
2930
use TYPO3\CMS\Core\Utility\MathUtility;
3031

@@ -87,11 +88,19 @@ public function formatCurrentUrl()
8788
$titleLen = (int)$this->getBackendUser()->uc['titleLen'];
8889

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

92+
$idInfo = 'ID: ' . $id . (!empty($this->linkParts['url']['fragment']) ? ', #' . $this->linkParts['url']['fragment'] : '');
93+
94+
$permsClause = $this->getBackendUser()->getPagePermsClause(Permission::PAGE_SHOW);
95+
$pageRecord = BackendUtility::readPageAccess($id, $permsClause);
96+
if ($pageRecord === false) {
97+
return $lang->sL('LLL:EXT:backend/Resources/Private/Language/locallang_browse_links.xlf:page') . ' ' . $idInfo;
98+
}
99+
100+
$pageTitle = $pageRecord['title'] ?? '';
92101
return $lang->sL('LLL:EXT:backend/Resources/Private/Language/locallang_browse_links.xlf:page')
93102
. ($pageTitle ? ' \'' . GeneralUtility::fixed_lgd_cs($pageTitle, $titleLen) . '\'' : '')
94-
. ' (ID: ' . $id . (!empty($this->linkParts['url']['fragment']) ? ', #' . $this->linkParts['url']['fragment'] : '') . ')';
103+
. ' (' . $idInfo . ')';
95104
}
96105

97106
/**

‎typo3/sysext/core/Classes/LinkHandling/FileLinkHandler.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException;
2121
use TYPO3\CMS\Core\Resource\FileInterface;
2222
use TYPO3\CMS\Core\Resource\ResourceFactory;
23+
use TYPO3\CMS\Core\Resource\Security\FileNameValidator;
2324
use TYPO3\CMS\Core\Utility\GeneralUtility;
2425

2526
/**
@@ -71,6 +72,13 @@ public function resolveHandlerData(array $data): array
7172
{
7273
try {
7374
$file = $this->resolveFile($data);
75+
$fileNameValidator = GeneralUtility::makeInstance(FileNameValidator::class);
76+
if (
77+
!$fileNameValidator->isValid(basename($file->getIdentifier())) ||
78+
!$fileNameValidator->isValid($file->getName())
79+
) {
80+
$file = null;
81+
}
7482
} catch (FileDoesNotExistException $e) {
7583
$file = null;
7684
}

‎typo3/sysext/core/Classes/LinkHandling/LegacyLinkNotationConverter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ protected function getFileOrFolderObjectFromMixedIdentifier(string $mixedIdentif
220220
}
221221
$fileOrFolderObject = $this->getResourceFactory()->retrieveFileOrFolderObject($fileIdentifier);
222222
// Links to a file/folder in the main TYPO3 directory should not be considered as file links, but an external link
223-
if ($fileOrFolderObject instanceof ResourceInterface && $fileOrFolderObject->getStorage()->getUid() === 0) {
223+
if ($fileOrFolderObject instanceof ResourceInterface && $fileOrFolderObject->getStorage()->isFallbackStorage()) {
224224
return [
225225
'type' => LinkService::TYPE_URL,
226226
'url' => $mixedIdentifier,

‎typo3/sysext/core/Classes/Resource/ResourceStorage.php‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,17 @@ public function hasChildren()
354354
return true;
355355
}
356356

357+
/**
358+
* Returns true if this storage is a virtual storage that provides
359+
* access to all files in the project root.
360+
*
361+
* @internal
362+
*/
363+
public function isFallbackStorage(): bool
364+
{
365+
return $this->getUid() === 0;
366+
}
367+
357368
/*********************************
358369
* Capabilities
359370
********************************/
@@ -718,7 +729,7 @@ public function checkFileActionPermission($action, FileInterface $file)
718729
return false;
719730
}
720731
// Check 3: No action allowed on files for denied file extensions
721-
if (!$this->checkFileExtensionPermission($file->getName())) {
732+
if (!$this->checkValidFileExtension($file)) {
722733
return false;
723734
}
724735
$isReadCheck = false;
@@ -830,6 +841,17 @@ protected function checkFileExtensionPermission($fileName)
830841
return GeneralUtility::makeInstance(FileNameValidator::class)->isValid($fileName);
831842
}
832843

844+
/**
845+
* Check file extension of an existing file against the
846+
* current file deny pattern.
847+
*/
848+
protected function checkValidFileExtension(FileInterface $file): bool
849+
{
850+
$fileNameValidator = GeneralUtility::makeInstance(FileNameValidator::class);
851+
return $fileNameValidator->isValid($file->getName()) &&
852+
$fileNameValidator->isValid(basename($file->getIdentifier()));
853+
}
854+
833855
/**
834856
* Assures read permission for given folder.
835857
*
@@ -896,7 +918,7 @@ protected function assureFileReadPermission(FileInterface $file)
896918
1375955429
897919
);
898920
}
899-
if (!$this->checkFileExtensionPermission($file->getName())) {
921+
if (!$this->checkValidFileExtension($file)) {
900922
throw new IllegalFileExtensionException(
901923
'You are not allowed to use that file extension. File: "' . $file->getName() . '"',
902924
1375955430
@@ -917,7 +939,7 @@ protected function assureFileWritePermissions(FileInterface $file)
917939
if (!$this->checkFileActionPermission('write', $file)) {
918940
throw new InsufficientFileWritePermissionsException('Writing to file "' . $file->getIdentifier() . '" is not allowed.', 1330121088);
919941
}
920-
if (!$this->checkFileExtensionPermission($file->getName())) {
942+
if (!$this->checkValidFileExtension($file)) {
921943
throw new IllegalFileExtensionException('You are not allowed to edit a file with extension "' . $file->getExtension() . '"', 1366711933);
922944
}
923945
}
@@ -950,7 +972,7 @@ protected function assureFileReplacePermissions(FileInterface $file)
950972
protected function assureFileDeletePermissions(FileInterface $file)
951973
{
952974
// Check for disallowed file extensions
953-
if (!$this->checkFileExtensionPermission($file->getName())) {
975+
if (!$this->checkValidFileExtension($file)) {
954976
throw new IllegalFileExtensionException('You are not allowed to delete a file with extension "' . $file->getExtension() . '"', 1377778916);
955977
}
956978
// Check further permissions if file is not a processed file
@@ -1071,7 +1093,7 @@ protected function assureFileMovePermissions(FileInterface $file, Folder $target
10711093
protected function assureFileRenamePermissions(FileInterface $file, $targetFileName)
10721094
{
10731095
// Check if file extension is allowed
1074-
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkFileExtensionPermission($file->getName())) {
1096+
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkValidFileExtension($file)) {
10751097
throw new IllegalFileExtensionException('You are not allowed to rename a file with this extension. File given: "' . $file->getName() . '"', 1371466663);
10761098
}
10771099
// Check if user is allowed to rename
@@ -1114,7 +1136,7 @@ protected function assureFileCopyPermissions(FileInterface $file, Folder $target
11141136
throw new InsufficientFolderWritePermissionsException('You are not allowed to write to the target folder "' . $targetFolder->getIdentifier() . '"', 1319550435);
11151137
}
11161138
// Check for a valid file extension
1117-
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkFileExtensionPermission($file->getName())) {
1139+
if (!$this->checkFileExtensionPermission($targetFileName) || !$this->checkValidFileExtension($file)) {
11181140
throw new IllegalFileExtensionException('You are not allowed to copy a file of that type.', 1319553317);
11191141
}
11201142
}
@@ -1733,6 +1755,7 @@ public function streamFile(
17331755
string $alternativeFilename = null,
17341756
string $overrideMimeType = null
17351757
): ResponseInterface {
1758+
$this->assureFileReadPermission($file);
17361759
if (!$this->driver instanceof StreamableDriverInterface) {
17371760
return $this->getPseudoStream($file, $asDownload, $alternativeFilename, $overrideMimeType);
17381761
}

‎typo3/sysext/core/Classes/Resource/Security/StoragePermissionsAspect.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,10 @@ public function addUserPermissionsToStorage(AfterResourceStorageInitializationEv
4242
if (($GLOBALS['TYPO3_REQUEST'] ?? null) instanceof ServerRequestInterface
4343
&& ApplicationType::fromRequest($GLOBALS['TYPO3_REQUEST'])->isBackend()
4444
&& !$GLOBALS['BE_USER']->isAdmin()
45+
&& !$storage->isFallbackStorage()
4546
) {
4647
$storage->setEvaluatePermissions(true);
47-
if ($storage->getUid() > 0) {
48-
$storage->setUserPermissions($GLOBALS['BE_USER']->getFilePermissionsForStorage($storage));
49-
} else {
50-
$storage->setEvaluatePermissions(false);
51-
}
48+
$storage->setUserPermissions($GLOBALS['BE_USER']->getFilePermissionsForStorage($storage));
5249
$this->addFileMountsToStorage($storage);
5350
}
5451
}

‎typo3/sysext/core/Classes/Utility/File/ExtendedFileUtility.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ protected function getFileObject($identifier)
590590
if ($object === null) {
591591
throw new InvalidFileException('The item ' . $identifier . ' was not a file or directory', 1320122453);
592592
}
593-
if ($object->getStorage()->getUid() === 0) {
593+
if ($object->getStorage()->isFallbackStorage()) {
594594
throw new InsufficientFileAccessPermissionsException('You are not allowed to access files outside your storages', 1375889830);
595595
}
596596
return $object;

0 commit comments

Comments
 (0)