Commit d3c9706c authored by Marc Bastian Heinrichs's avatar Marc Bastian Heinrichs Committed by Benni Mack
Browse files

[SECURITY] Prevent edit of file metadata of files with no access

By forging edit URLs it was possible to edit
meta data records of files which were not
within a user mount.

Implement several hooks to check access to the file
and only grant access to a meta data record if the
user has access to the file.

Resolves: #56644
Releases: master, 6.2
Security-Bulletin: TYPO3-CORE-SA-2015-002
Change-Id: I0f0704af2e7f01d16b9420f9ba4ac1a7846b5270
Reviewed-on: http://review.typo3.org/40804


Reviewed-by: Benni Mack's avatarBenjamin Mack <benni@typo3.org>
Tested-by: Benni Mack's avatarBenjamin Mack <benni@typo3.org>
Reviewed-by: default avatarHelmut Hummel <helmut.hummel@typo3.org>
Tested-by: default avatarHelmut Hummel <helmut.hummel@typo3.org>
parent edd2a1c5
......@@ -654,6 +654,20 @@ class InlineRecordContainer extends AbstractContainer {
if (!$backendUser->check('tables_modify', $table)) {
$hasAccess = FALSE;
}
if (
!empty($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tceforms_inline.php']['checkAccess'])
&& is_array($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tceforms_inline.php']['checkAccess'])
) {
foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tceforms_inline.php']['checkAccess'] as $_funcRef) {
$_params = array(
'table' => $table,
'uid' => $theUid,
'cmd' => $cmd,
'hasAccess' => $hasAccess
);
$hasAccess = GeneralUtility::callUserFunction($_funcRef, $_params, $this);
}
}
if (!$hasAccess) {
$deniedAccessReason = $backendUser->errorMsg;
if ($deniedAccessReason) {
......
<?php
namespace TYPO3\CMS\Core\Resource\Security;
/*
* 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!
*/
use TYPO3\CMS\Backend\Utility\BackendUtility;
use TYPO3\CMS\Core\DataHandling\DataHandler;
use TYPO3\CMS\Core\DataHandling\DataHandlerCheckModifyAccessListHookInterface;
use TYPO3\CMS\Core\Resource\ResourceFactory;
use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Core\Utility\MathUtility;
/**
* We do not have AOP in TYPO3 for now, thus the aspect which
* deals with file metadata data security is a assembly of hooks to
* check permissions on files belonging to file meta data records
*/
class FileMetadataPermissionsAspect implements DataHandlerCheckModifyAccessListHookInterface, SingletonInterface {
/**
* This hook is called before any write operation by DataHandler
*
* @param string $table
* @param int $id
* @param array $fileMetadataRecord
* @param int|NULL $otherHookGrantedAccess
* @param \TYPO3\CMS\Core\DataHandling\DataHandler $dataHandler
* @return int|null
*/
public function checkRecordUpdateAccess($table, $id, $fileMetadataRecord, $otherHookGrantedAccess, DataHandler $dataHandler) {
$accessAllowed = $otherHookGrantedAccess;
if ($table === 'sys_file_metadata' && $accessAllowed !== 0) {
$existingFileMetadataRecord = BackendUtility::getRecord('sys_file_metadata', $id);
if ($existingFileMetadataRecord === NULL || (empty($existingFileMetadataRecord['file']) && !empty($fileMetadataRecord['file']))) {
$existingFileMetadataRecord = $fileMetadataRecord;
}
$accessAllowed = $this->checkFileWriteAccessForFileMetaData($existingFileMetadataRecord) ? 1 : 0;
}
return $accessAllowed;
}
/**
* Hook that determines whether a user has access to modify a table.
* We "abuse" it here to actually check if access is allowed to sys_file_metadata.
*
*
* @param int &$accessAllowed Whether the user has access to modify a table
* @param string $table The name of the table to be modified
* @param DataHandler $parent The calling parent object
* @throws \UnexpectedValueException
* @return void
*/
public function checkModifyAccessList(&$accessAllowed, $table, DataHandler $parent) {
if ($table === 'sys_file_metadata') {
if (isset($parent->cmdmap[$table]) && is_array($parent->cmdmap[$table])) {
foreach ($parent->cmdmap[$table] as $id => $command) {
if (empty($id) || !MathUtility::canBeInterpretedAsInteger($id)) {
throw new \UnexpectedValueException(
'Integer expected for data manipulation command.
This can only happen in the case of an attack attempt or when something went horribly wrong.
To not compromise security, we exit here.',
1399982816
);
}
$fileMetadataRecord = BackendUtility::getRecord('sys_file_metadata', $id);
$accessAllowed = $this->checkFileWriteAccessForFileMetaData($fileMetadataRecord);
if (!$accessAllowed) {
// If for any item in the array, access is not allowed, we deny the whole operation
break;
}
}
}
if (isset($parent->datamap[$table]) && is_array($parent->datamap[$table])) {
foreach ($parent->datamap[$table] as $id => $data) {
$recordAccessAllowed = FALSE;
if (strpos($id, 'NEW') === FALSE) {
$fileMetadataRecord = BackendUtility::getRecord('sys_file_metadata', $id);
if ($fileMetadataRecord !== NULL) {
if ($parent->isImporting && empty($fileMetadataRecord['file'])) {
// When importing the record was added with an empty file relation as first step
$recordAccessAllowed = TRUE;
} else {
$recordAccessAllowed = $this->checkFileWriteAccessForFileMetaData($fileMetadataRecord);
}
}
} else {
// For new records record access is allowed
$recordAccessAllowed = TRUE;
}
if (isset($data['file'])) {
if ($parent->isImporting && empty($data['file'])) {
// When importing the record will be created with an empty file relation as first step
$dataAccessAllowed = TRUE;
} elseif (empty($data['file'])) {
$dataAccessAllowed = FALSE;
} else {
$dataAccessAllowed = $this->checkFileWriteAccessForFileMetaData($data);
}
} else {
$dataAccessAllowed = TRUE;
}
if (!$recordAccessAllowed || !$dataAccessAllowed) {
// If for any item in the array, access is not allowed, we deny the whole operation
$accessAllowed = FALSE;
break;
}
}
}
}
}
/**
* Deny access to the edit form. This is not mandatory, but better to show this right away that access is denied.
*
* @param array $parameters
* @return bool
*/
public function isAllowedToShowEditForm(array $parameters) {
$table = $parameters['table'];
$uid = $parameters['uid'];
$cmd = $parameters['cmd'];
$accessAllowed = $parameters['hasAccess'];
if ($accessAllowed && $table === 'sys_file_metadata' && $cmd === 'edit') {
$fileMetadataRecord = BackendUtility::getRecord('sys_file_metadata', $uid);
$accessAllowed = $this->checkFileWriteAccessForFileMetaData($fileMetadataRecord);
}
return $accessAllowed;
}
/**
* Checks write access to the file belonging to a metadata entry
*
* @param array $fileMetadataRecord
* @return bool
*/
protected function checkFileWriteAccessForFileMetaData($fileMetadataRecord) {
$accessAllowed = FALSE;
if (is_array($fileMetadataRecord) && !empty($fileMetadataRecord['file'])) {
$file = $fileMetadataRecord['file'];
// The file relation could be written as sys_file_[uid], strip this off before checking the rights
if (strpos($file, 'sys_file_') !== FALSE) {
$file = substr($file, strlen('sys_file_'));
}
$fileObject = ResourceFactory::getInstance()->getFileObject((int)$file);
$accessAllowed = $fileObject->checkActionPermission('write');
}
return $accessAllowed;
}
}
=========================================================================
Feature: #56644 - Hook for InlineRecordContainer::checkAccess()
=========================================================================
Description
===========
Hook to post-process ``InlineRecordContainer::checkAccess``
result. ``InlineRecordContainer::checkAccess`` is used to check
the access to related inline records. It's implemented in the same way to
the hook $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['typo3/alt_doc.php']['makeEditForm_accessCheck']
in the EditDocumentController.
Register like this:
.. code-block:: php
$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tceforms_inline.php']['checkAccess'][] = 'My\\Package\\HookClass->hookMethod';
\ No newline at end of file
......@@ -5,12 +5,19 @@ defined('TYPO3_MODE') or die();
$signalSlotDispatcher = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(\TYPO3\CMS\Extbase\SignalSlot\Dispatcher::class);
if (TYPO3_MODE === 'BE' && !(TYPO3_REQUESTTYPE & TYPO3_REQUESTTYPE_INSTALL)) {
// FAL SECURITY CHECKS
$signalSlotDispatcher->connect(
\TYPO3\CMS\Core\Resource\ResourceFactory::class,
\TYPO3\CMS\Core\Resource\ResourceFactoryInterface::SIGNAL_PostProcessStorage,
\TYPO3\CMS\Core\Resource\Security\StoragePermissionsAspect::class,
'addUserPermissionsToStorage'
);
$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['processDatamapClass'][] = 'TYPO3\\CMS\\Core\\Resource\\Security\\FileMetadataPermissionsAspect';
$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['typo3/alt_doc.php']['makeEditForm_accessCheck'][] = 'TYPO3\\CMS\\Core\\Resource\\Security\\FileMetadataPermissionsAspect->isAllowedToShowEditForm';
$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tceforms_inline.php']['checkAccess'][] = 'TYPO3\\CMS\\Core\\Resource\\Security\\FileMetadataPermissionsAspect->isAllowedToShowEditForm';
$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['checkModifyAccessList'][] = 'TYPO3\\CMS\\Core\\Resource\\Security\\FileMetadataPermissionsAspect';
// PACKAGE MANAGEMENT
$signalSlotDispatcher->connect(
'PackageManagement',
'packagesMayHaveChanged',
......
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