[SECURITY] Deny arbitrary code execution possibility for editors 04/23604/2
authorHelmut Hummel <helmut.hummel@typo3.org>
Wed, 4 Sep 2013 11:23:29 +0000 (13:23 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Wed, 4 Sep 2013 11:23:33 +0000 (13:23 +0200)
Because the filename is sanitized in the driver
after the check for denied file extensions is
performed, it was still possible to rename files
with denied file extensions.

We now perform the file extension check
on the final filename which is going to be used
by the driver.

This change makes the sanitizing method public
and introduces a basic implementation in
AbstractDriver to not break existing driver
implementations.

Change-Id: I74b3596e194e79135d7affa3111b3f9e40a72693
Fixes: #51495
Releases: 6.2, 6.1, 6.0
Security-Commit: d9aa75ad0169e90065656ca78b19ab966d1c76c1
Security-Bulletin: TYPO3-CORE-SA-2013-003
Reviewed-on: https://review.typo3.org/23604
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
typo3/sysext/core/Classes/Resource/Driver/AbstractDriver.php
typo3/sysext/core/Classes/Resource/Driver/LocalDriver.php
typo3/sysext/core/Classes/Resource/ResourceStorage.php

index 59d3c6d..491dbd5 100644 (file)
@@ -381,6 +381,18 @@ abstract class AbstractDriver {
        abstract public function getFileInfoByIdentifier($identifier);
 
        /**
+        * Basic implementation of the method that does directly return the
+        * file name as is.
+        *
+        * @param string $fileName Input string, typically the body of a fileName
+        * @param string $charset Charset of the a fileName (defaults to current charset; depending on context)
+        * @return string Output string with any characters not matching [.a-zA-Z0-9_-] is substituted by '_' and trailing dots removed
+        */
+       public function sanitizeFileName($fileName, $charset = '') {
+               return $fileName;
+       }
+
+       /**
         * Returns information about a file for a given file object.
         *
         * @param \TYPO3\CMS\Core\Resource\FileInterface $file
index e6f25ed..c5baf71 100644 (file)
@@ -248,8 +248,9 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @param string $fileName Input string, typically the body of a fileName
         * @param string $charset Charset of the a fileName (defaults to current charset; depending on context)
         * @return string Output string with any characters not matching [.a-zA-Z0-9_-] is substituted by '_' and trailing dots removed
+        * @throws \TYPO3\CMS\Core\Resource\Exception\InvalidFileNameException
         */
-       protected function sanitizeFileName($fileName, $charset = '') {
+       public function sanitizeFileName($fileName, $charset = '') {
                // Handle UTF-8 characters
                if ($GLOBALS['TYPO3_CONF_VARS']['SYS']['UTF8filesystem']) {
                        // Allow ".", "-", 0-9, a-z, A-Z and everything beyond U+C0 (latin capital letter a with grave)
index 940e728..694ba30 100644 (file)
@@ -687,6 +687,7 @@ class ResourceStorage {
                if (!$this->evaluatePermissions) {
                        return TRUE;
                }
+               $fileName = $this->driver->sanitizeFileName($fileName);
                $isAllowed = GeneralUtility::verifyFilenameAgainstDenyPattern($fileName);
                if ($isAllowed) {
                        $fileInfo = GeneralUtility::split_fileref($fileName);