Skip to content

Commit

Permalink
[BUGFIX] Properly check shortcut permissions in ShortcutRepository
Browse files Browse the repository at this point in the history
When fetching available shortcuts for a user, also permissions
are checked by the ShortcutRepository. However previously a lot
of use-cases were missed and the implemented checks were more
or less faulty, especially when it comes to non-admin users.

Therefore, three main topics are now handled properly:

* Evaluation of record edit permissions for shortcuts,
  targeting the record_edit route
* Evaluation of page access permissions for every shortcut
  not targeting the file list
* Proper distinction between shortcuts for file list and
  the ones for other modules, since both use the "id"
  argument, while for the file list, this is a string
  (combined identifier), and for the rest, this is an
  integer (the requested page id or the records' pid)

Resolves: #89530
Resolves: #93516
Releases: master, 10.4
Change-Id: Ib18eaf506886627360c58857f0160d008e130368
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69765
Tested-by: core-ci <typo3@b13.com>
Tested-by: Peter Kraume <peter.kraume@gmx.de>
Tested-by: Jochen <rothjochen@gmail.com>
Tested-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Peter Kraume <peter.kraume@gmx.de>
Reviewed-by: Jochen <rothjochen@gmail.com>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
  • Loading branch information
o-ba committed Jul 12, 2021
1 parent e705b30 commit a06a256
Showing 1 changed file with 70 additions and 21 deletions.
Expand Up @@ -28,11 +28,12 @@
use TYPO3\CMS\Core\Imaging\Icon;
use TYPO3\CMS\Core\Imaging\IconFactory;
use TYPO3\CMS\Core\Localization\LanguageService;
use TYPO3\CMS\Core\Resource\Exception\FolderDoesNotExistException;
use TYPO3\CMS\Core\Resource\Exception\InsufficientFolderAccessPermissionsException;
use TYPO3\CMS\Core\Resource\Exception\ResourceDoesNotExistException;
use TYPO3\CMS\Core\Resource\ResourceFactory;
use TYPO3\CMS\Core\Type\Bitmask\Permission;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;

/**
* Repository for backend shortcuts
Expand Down Expand Up @@ -232,11 +233,13 @@ public function addShortcut(string $url, string $module, string $parentModule =
}
}

// Check if given id is a combined identifier
if (!empty($queryParameters['id']) && preg_match('/^[\d]+:/', $queryParameters['id'])) {
$moduleName = $module ?: $parentModule;
$id = $this->extractIdFromShortcutUrl($moduleName, $url);
if ($moduleName === 'file_FilelistList' && $id !== '') {
// If filelist module, check if the id is a valid module identifier
try {
$resourceFactory = GeneralUtility::makeInstance(ResourceFactory::class);
$resource = $resourceFactory->getObjectFromCombinedIdentifier($queryParameters['id']);
$resource = $resourceFactory->getObjectFromCombinedIdentifier($id);
$title = trim(sprintf(
'%s (%s)',
$titlePrefix,
Expand All @@ -246,7 +249,7 @@ public function addShortcut(string $url, string $module, string $parentModule =
}
} else {
// Lookup the title of this page and use it as default description
$pageId = $pageId ?: $recordId ?: $this->extractPageIdFromShortcutUrl($url);
$pageId = $pageId ?: $recordId ?: (int)$id;
$page = $pageId ? BackendUtility::getRecord('pages', $pageId) : null;

if (!empty($page)) {
Expand Down Expand Up @@ -484,6 +487,7 @@ protected function initShortcuts(): array
->execute();

while ($row = $result->fetch()) {
$pageId = 0;
$shortcut = ['raw' => $row];

[$row['module_name'], $row['M_module_name']] = explode('|', $row['module_name']);
Expand Down Expand Up @@ -519,22 +523,53 @@ protected function initShortcuts(): array
continue;
}

$pageId = $this->extractPageIdFromShortcutUrl($row['url']);

if (!$backendUser->isAdmin()) {
if (MathUtility::canBeInterpretedAsInteger($pageId)) {
if ($moduleName === 'file_FilelistList') {
$combinedIdentifier = $this->extractIdFromShortcutUrl($moduleName, $row['url'] ?? '');
if ($combinedIdentifier !== '') {
try {
$resourceFactory = GeneralUtility::makeInstance(ResourceFactory::class);
$storage = $resourceFactory->getStorageObjectFromCombinedIdentifier($combinedIdentifier);
if ($storage === null || $storage->getUid() === 0) {
// Continue, if invalid storage or disallowed fallback storage
continue;
}
$folderIdentifier = substr($combinedIdentifier, strpos($combinedIdentifier, ':') + 1);
// By using $storage->getFolder() we implicitly check whether the folder
// still exists and the user has necessary permissions to access it.
$storage->getFolder($folderIdentifier);
} catch (InsufficientFolderAccessPermissionsException $e) {
// Continue, since current user does not have access to the folder
continue;
} catch (FolderDoesNotExistException $e) {
// Folder does not longer exists. However, the shortcut
// is still displayed, allowing the user to remove it.
}
}
} else {
if ($moduleName === 'xMOD_alt_doc.php' && isset($shortcut['table'], $shortcut['recordid'])) {
// Check if user is allowed to edit the requested record
if (!$backendUser->check('tables_modify', $shortcut['table'])) {
continue;
}
$record = BackendUtility::getRecord($shortcut['table'], (int)$shortcut['recordid']);
// Check if requested record exists
if ($record === null || $record === []) {
continue;
}
// Store the page id of the record in question
$pageId = ($shortcut['table'] === 'pages' ? (int)($record['uid'] ?? 0) : (int)($record['pid']));
} else {
// In case this is no record edit shortcut, treat a possible "id" as page id
$pageId = (int)$this->extractIdFromShortcutUrl($moduleName, $row['url'] ?? '');
}
if ($pageId > 0 && !$backendUser->isAdmin()) {
// Check for webmount access
if ($backendUser->isInWebMount($pageId) === null) {
continue;
}
// Check for record access
$pageRow = BackendUtility::getRecord('pages', $pageId);

if ($pageRow === null) {
continue;
}

if (!$backendUser->doesUserHaveAccess($pageRow, Permission::PAGE_SHOW)) {
if ($pageRow === null || !$backendUser->doesUserHaveAccess($pageRow, Permission::PAGE_SHOW)) {
continue;
}
}
Expand All @@ -558,7 +593,7 @@ protected function initShortcuts(): array
$shortcut['group'] = $shortcutGroup;
$shortcut['icon'] = $this->getShortcutIcon($row, $shortcut);
$shortcut['iconTitle'] = $this->getShortcutIconTitle($shortcut['label'], $row['module_name'], $row['M_module_name']);
$shortcut['action'] = 'jump(' . GeneralUtility::quoteJSvalue($this->getTokenUrl($row['url'])) . ',' . GeneralUtility::quoteJSvalue($moduleName) . ',' . GeneralUtility::quoteJSvalue($moduleParts[0]) . ', ' . (int)$pageId . ');';
$shortcut['action'] = 'jump(' . GeneralUtility::quoteJSvalue($this->getTokenUrl($row['url'])) . ',' . GeneralUtility::quoteJSvalue($moduleName) . ',' . GeneralUtility::quoteJSvalue($moduleParts[0]) . ', ' . $pageId . ');';

$shortcuts[] = $shortcut;
}
Expand Down Expand Up @@ -679,14 +714,28 @@ protected function getShortcutIconTitle(string $shortcutLabel, string $moduleNam
}

/**
* Return the ID of the page in the URL if found.
* Get the id from the shortcut url. This could
* either be the page id or the combined identifier.
*
* @param string $url The URL of the current shortcut link
* @return int If a page ID was found, it is returned. Otherwise: 0
* @param string $moduleName
* @param string $shortcutUrl
* @return string
*/
protected function extractPageIdFromShortcutUrl(string $url): int
protected function extractIdFromShortcutUrl(string $moduleName, string $shortcutUrl): string
{
return (int)preg_replace('/.*[\\?&]id=([^&]+).*/', '$1', $url);
$parsedUrl = parse_url($shortcutUrl);
$queryParams = [];
parse_str($parsedUrl['query'] ?? '', $queryParams);
$id = (string)($queryParams['id'] ?? '');

if ($id === '' && $moduleName === 'xMOD_alt_doc.php' && ($queryParams['returnUrl'] ?? '') !== '') {
$parsedReturlUrl = parse_url($queryParams['returnUrl']);
$returnUrlQueryParams = [];
parse_str($parsedReturlUrl['query'] ?? '', $returnUrlQueryParams);
$id = (string)($returnUrlQueryParams['id'] ?? '');
}

return $id;
}

/**
Expand Down

0 comments on commit a06a256

Please sign in to comment.