[BUGFIX] Re-enables fileDenyPattern check for admin users 10/32610/12
authorTorben Hansen <derhansen@gmail.com>
Thu, 26 May 2016 19:20:23 +0000 (21:20 +0200)
committerHelmut Hummel <helmut.hummel@typo3.org>
Fri, 27 May 2016 11:43:33 +0000 (13:43 +0200)
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>
typo3/sysext/core/Classes/Resource/ResourceStorage.php
typo3/sysext/core/Tests/Unit/Resource/ResourceStorageTest.php

index b8bb973..4ecb615 100644 (file)
@@ -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;
     }
 
     /**
index a286dbb..791aac9 100644 (file)
@@ -149,6 +149,57 @@ class ResourceStorageTest extends BaseTestCase
     /**
      * @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
+     */
     public function isWithinFileMountBoundariesDataProvider()
     {
         return array(