From a3635263d849db4ae1ceaab98305d702e4efbb7f Mon Sep 17 00:00:00 2001
From: Torben Hansen <derhansen@gmail.com>
Date: Thu, 26 May 2016 21:20:23 +0200
Subject: [PATCH] [BUGFIX] Re-enables fileDenyPattern check for admin users
When an admin user tries to upload a file which has a fileextension
that is included in the fileDenyPattern, the upload is denied.
With the security fix in #51326 admin users are now able to change
the extension of a file to any value, since the fileDenyPattern is
not checked for admin users. This leads to the situation, that admin
users can create/rename files in the filelist with a fileextension
of their choice.
To keep the behavior consistent, this patch re-enables the check
of the fileDenyPattern for admin users in the filelist.
Resolves: #60173
Releases: master, 7.6, 6.2
Change-Id: I3b819e70cf2218a4580203ac7b7a6b0c3c5087ab
Reviewed-on: https://review.typo3.org/32610
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Nicole Cordes <typo3@cordes.co>
Tested-by: Nicole Cordes <typo3@cordes.co>
Reviewed-by: Helmut Hummel <helmut.hummel@typo3.org>
Tested-by: Helmut Hummel <helmut.hummel@typo3.org>
---
.../core/Classes/Resource/ResourceStorage.php | 7 +--
.../Unit/Resource/ResourceStorageTest.php | 51 +++++++++++++++++++
2 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/typo3/sysext/core/Classes/Resource/ResourceStorage.php b/typo3/sysext/core/Classes/Resource/ResourceStorage.php
index b8bb97357ddc..4ecb61569247 100644
--- a/typo3/sysext/core/Classes/Resource/ResourceStorage.php
+++ b/typo3/sysext/core/Classes/Resource/ResourceStorage.php
@@ -707,12 +707,9 @@ class ResourceStorage implements ResourceStorageInterface
*/
protected function checkFileExtensionPermission($fileName)
{
- if (!$this->evaluatePermissions) {
- return true;
- }
$fileName = $this->driver->sanitizeFileName($fileName);
$isAllowed = GeneralUtility::verifyFilenameAgainstDenyPattern($fileName);
- if ($isAllowed) {
+ if ($isAllowed && $this->evaluatePermissions) {
$fileExtension = strtolower(PathUtility::pathinfo($fileName, PATHINFO_EXTENSION));
// Set up the permissions for the file extension
$fileExtensionPermissions = $GLOBALS['TYPO3_CONF_VARS']['BE']['fileExtensions']['webspace'];
@@ -739,7 +736,7 @@ class ResourceStorage implements ResourceStorageInterface
return true;
}
}
- return false;
+ return $isAllowed;
}
/**
diff --git a/typo3/sysext/core/Tests/Unit/Resource/ResourceStorageTest.php b/typo3/sysext/core/Tests/Unit/Resource/ResourceStorageTest.php
index a286dbbd5a7d..791aac90fe60 100644
--- a/typo3/sysext/core/Tests/Unit/Resource/ResourceStorageTest.php
+++ b/typo3/sysext/core/Tests/Unit/Resource/ResourceStorageTest.php
@@ -146,6 +146,57 @@ class ResourceStorageTest extends BaseTestCase
return $driver;
}
+ /**
+ * @return array
+ */
+ public function fileExtensionPermissionDataProvider()
+ {
+ return array(
+ 'Permissions evaluated, extension not in allowed list' => array(
+ 'fileName' => 'foo.txt',
+ 'configuration' => array('allow' => 'jpg'),
+ 'evaluatePermissions' => true,
+ 'isAllowed' => true,
+ ),
+ 'Permissions evaluated, extension in deny list' => array(
+ 'fileName' => 'foo.txt',
+ 'configuration' => array('deny' => 'txt'),
+ 'evaluatePermissions' => true,
+ 'isAllowed' => false,
+ ),
+ 'Permissions not evaluated, extension is php' => array(
+ 'fileName' => 'foo.php',
+ 'configuration' => array(),
+ 'evaluatePermissions' => false,
+ 'isAllowed' => false,
+ ),
+ 'Permissions evaluated, extension is php' => array(
+ 'fileName' => 'foo.php',
+ // It is not possible to allow php file extension through configuration
+ 'configuration' => array('allow' => 'php'),
+ 'evaluatePermissions' => true,
+ 'isAllowed' => false,
+ ),
+ );
+ }
+
+ /**
+ * @param string $fileName
+ * @param array $configuration
+ * @param bool $evaluatePermissions
+ * @param bool $isAllowed
+ * @test
+ * @dataProvider fileExtensionPermissionDataProvider
+ */
+ public function fileExtensionPermissionIsWorkingCorrectly($fileName, array $configuration, $evaluatePermissions, $isAllowed)
+ {
+ $GLOBALS['TYPO3_CONF_VARS']['BE']['fileExtensions']['webspace'] = $configuration;
+ $driverMock = $this->getMockForAbstractClass(AbstractDriver::class, array(), '', false);
+ $subject = $this->getAccessibleMock(ResourceStorage::class, array('dummy'), array($driverMock, array()));
+ $subject->_set('evaluatePermissions', $evaluatePermissions);
+ $this->assertSame($isAllowed, $subject->_call('checkFileExtensionPermission', $fileName));
+ }
+
/**
* @return array
*/
--
GitLab