[TASK] Decouple TemplateService->getFileName 03/57403/10
authorBenni Mack <benni@typo3.org>
Thu, 28 Jun 2018 07:22:46 +0000 (09:22 +0200)
committerBenni Mack <benni@typo3.org>
Mon, 2 Jul 2018 10:48:49 +0000 (12:48 +0200)
TemplateService in TYPO3 frontend is responsible for
fetching sys_template records and sorting TypoScript
related things out, but also contains one method,
which is nowadays completely separate from
sys_template: "getFileName".

This method checks for valid syntax and returns
a proper string then (resolves "EXT:... syntax"
and valdiates against available paths).

The logic is frontend-related, thus moved
to EXT:frontend/FilePathSanitizer.

The old functionality in TemplateService is
deprecated, along with the public property "fileCache".

Resolves: #85445
Releases: master
Change-Id: Id52ef476d0cd6a67de2560a0e427339b7310427e
Reviewed-on: https://review.typo3.org/57403
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
14 files changed:
typo3/sysext/core/Classes/TypoScript/TemplateService.php
typo3/sysext/core/Documentation/Changelog/master/Deprecation-85445-TemplateService-getFileName.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/TypoScript/TemplateServiceTest.php
typo3/sysext/core/Tests/UnitDeprecated/TypoScript/TemplateServiceTest.php [new file with mode: 0644]
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/frontend/Classes/ContentObject/FileContentObject.php
typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
typo3/sysext/frontend/Classes/Imaging/GifBuilder.php
typo3/sysext/frontend/Classes/Page/PageGenerator.php
typo3/sysext/frontend/Classes/Resource/FilePathSanitizer.php [new file with mode: 0644]
typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
typo3/sysext/frontend/Tests/Unit/Resource/FilePathSanitizerTest.php [new file with mode: 0644]
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyPublicMatcher.php

index 62e7fe4..b76b61d 100644 (file)
@@ -25,15 +25,19 @@ use TYPO3\CMS\Core\Database\Query\Restriction\AbstractRestrictionContainer;
 use TYPO3\CMS\Core\Database\Query\Restriction\DefaultRestrictionContainer;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
 use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction;
+use TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidFileException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidFileNameException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidPathException;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
 use TYPO3\CMS\Core\Utility\ArrayUtility;
 use TYPO3\CMS\Core\Utility\ExtensionManagementUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
-use TYPO3\CMS\Core\Utility\PathUtility;
 use TYPO3\CMS\Frontend\Configuration\TypoScript\ConditionMatching\ConditionMatcher;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use TYPO3\CMS\Frontend\Page\PageRepository;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
 
 /**
  * Template object that is responsible for generating the TypoScript template based on template records.
@@ -63,6 +67,7 @@ class TemplateService
         'sectionsMatch' => 'Using $sectionsMatch of class TemplateService from the outside is discouraged, as this variable is only used for internal storage.',
         'frames' => 'Using $frames of class TemplateService from the outside is discouraged, as this variable is only used for internal storage.',
         'MPmap' => 'Using $MPmap of class TemplateService from the outside is discouraged, as this variable is only used for internal storage.',
+        'fileCache' => 'Using $fileCache of class TemplateService from the outside is discouraged, the property will be removed in v10.',
     ];
 
     /**
@@ -296,8 +301,9 @@ class TemplateService
      * Used by getFileName for caching of references to file resources
      *
      * @var array
+     * @deprecated Will be removed in v10
      */
-    public $fileCache = [];
+    protected $fileCache = [];
 
     /**
      * Keys are frame names and values are type-values, which must be used to refer correctly to the content of the frames.
@@ -1389,53 +1395,33 @@ class TemplateService
      *
      * @param string $fileFromSetup TypoScript "resource" data type value.
      * @return string|null Resulting filename, is either a full absolute URL or a relative path. Returns NULL if invalid filename or a directory is given
+     * @deprecated since TYPO3 v9.4, will be removed in TYPO3 v10.0.
      */
     public function getFileName($fileFromSetup)
     {
-        $file = trim($fileFromSetup);
-        if (!$file) {
-            return null;
-        }
-        if (strpos($file, '../') !== false) {
-            if ($this->tt_track) {
-                $this->getTimeTracker()->setTSlogMessage('File path "' . $file . '" contained illegal string "../"!', 3);
+        trigger_error('TemplateService->getFileName() will be removed in TYPO3 v10.0. Use FilePathSanitizer->sanitize() of EXT:frontend instead.', E_USER_DEPRECATED);
+        try {
+            $file = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize((string)$fileFromSetup);
+            $hash = md5($file);
+            if (!isset($this->fileCache[$hash])) {
+                $this->fileCache[$hash] = $file;
             }
-            return null;
-        }
-        // Cache
-        $hash = md5($file);
-        if (isset($this->fileCache[$hash])) {
-            return $this->fileCache[$hash];
-        }
-
-        // if this is an URL, it can be returned directly
-        $urlScheme = parse_url($file, PHP_URL_SCHEME);
-        if ($urlScheme === 'https' || $urlScheme === 'http' || is_file(Environment::getPublicPath() . '/' . $file)) {
             return $file;
-        }
-
-        // this call also resolves EXT:myext/ files
-        $file = GeneralUtility::getFileAbsFileName($file);
-        if (!$file) {
+        } catch (InvalidFileNameException $e) {
+            // Empty file name
+        } catch (InvalidPathException $e) {
+            if ($this->tt_track) {
+                $this->getTimeTracker()->setTSlogMessage('File path "' . $fileFromSetup . '" contained illegal string "../"!', 3);
+            }
+        } catch (FileDoesNotExistException $e) {
             if ($this->tt_track) {
                 $this->getTimeTracker()->setTSlogMessage('File "' . $fileFromSetup . '" was not found!', 3);
             }
-            return null;
-        }
-
-        $file = PathUtility::stripPathSitePrefix($file);
-
-        // Check if the found file is in the allowed paths
-        foreach ($this->allowedPaths as $val) {
-            if (GeneralUtility::isFirstPartOfStr($file, $val)) {
-                $this->fileCache[$hash] = $file;
-                return $file;
+        } catch (InvalidFileException $e) {
+            if ($this->tt_track) {
+                $this->getTimeTracker()->setTSlogMessage('"' . $fileFromSetup . '" was not located in the allowed paths: (' . implode(',', $this->allowedPaths) . ')', 3);
             }
         }
-
-        if ($this->tt_track) {
-            $this->getTimeTracker()->setTSlogMessage('"' . $file . '" was not located in the allowed paths: (' . implode(',', $this->allowedPaths) . ')', 3);
-        }
         return null;
     }
 
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85445-TemplateService-getFileName.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85445-TemplateService-getFileName.rst
new file mode 100644 (file)
index 0000000..d4ee56f
--- /dev/null
@@ -0,0 +1,37 @@
+.. include:: ../../Includes.txt
+
+==================================================
+Deprecation: #85445 - TemplateService->getFileName
+==================================================
+
+See :issue:`85445`
+
+Description
+===========
+
+The PHP method :php:`TYPO3\CMS\Core\TypoScript\TemplateService->getFileName()` has been marked as deprecated, as
+it is technically extracted into separate functionality with modern architecture throwing PHP Exceptions when
+a file name is invalid.
+
+Along with the method comes the public property :php:`$fileCache` which acted as a simple first-level
+in-memory cache, its access is deprecated, too.
+
+
+Impact
+======
+
+Calling the method directly or accessing the public property will trigger a PHP deprecation message.
+
+
+Affected Installations
+======================
+
+Any TYPO3 installation dealing with PHP code in Frontend (e.g. `$TSFE->tmpl->getFileName()`).
+
+
+Migration
+=========
+
+Use :php:`TYPO3\CMS\Frontend\Resource\FilePathSanitizer->sanitize($filePath)` instead.
+
+.. index:: Frontend, PHP-API, FullyScanned
index b756361..0075d3e 100644 (file)
@@ -31,7 +31,7 @@ use TYPO3\TestingFramework\Core\AccessibleObjectInterface;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 /**
- * Testcase for \TYPO3\CMS\Core\TypoScript\TemplateService
+ * Test case
  */
 class TemplateServiceTest extends UnitTestCase
 {
@@ -207,38 +207,4 @@ class TemplateServiceTest extends UnitTestCase
         $this->templateServiceMock->_set('rootLine', $originalRootline);
         $this->templateServiceMock->updateRootlineData($newInvalidRootline);
     }
-
-    /**
-     * @test
-     */
-    public function getFileNameReturnsUrlCorrectly(): void
-    {
-        $this->assertSame('http://example.com', $this->templateService->getFileName('http://example.com'));
-        $this->assertSame('https://example.com', $this->templateService->getFileName('https://example.com'));
-    }
-
-    /**
-     * @test
-     */
-    public function getFileNameReturnsFileCorrectly(): void
-    {
-        $this->assertSame('typo3/index.php', $this->templateService->getFileName('typo3/index.php'));
-    }
-
-    /**
-     * @test
-     */
-    public function getFileNameReturnsNullIfDirectory(): void
-    {
-        $this->assertNull($this->templateService->getFileName('typo3/'));
-    }
-
-    /**
-     * @test
-     */
-    public function getFileNameReturnsNullWithInvalidFileName(): void
-    {
-        $this->assertNull($this->templateService->getFileName('  '));
-        $this->assertNull($this->templateService->getFileName('something/../else'));
-    }
 }
diff --git a/typo3/sysext/core/Tests/UnitDeprecated/TypoScript/TemplateServiceTest.php b/typo3/sysext/core/Tests/UnitDeprecated/TypoScript/TemplateServiceTest.php
new file mode 100644 (file)
index 0000000..a5be928
--- /dev/null
@@ -0,0 +1,75 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Core\Tests\UnitDeprecated\TypoScript;
+
+/*
+ * 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\Core\Context\Context;
+use TYPO3\CMS\Core\TypoScript\TemplateService;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+/**
+ * Test case
+ */
+class TemplateServiceTest extends UnitTestCase
+{
+    /**
+     * @var TemplateService
+     */
+    protected $templateService;
+
+    /**
+     * Set up
+     */
+    protected function setUp(): void
+    {
+        $this->templateService = new TemplateService(new Context());
+        $this->templateService->tt_track = false;
+    }
+
+    /**
+     * @test
+     */
+    public function getFileNameReturnsUrlCorrectly(): void
+    {
+        $this->assertSame('http://example.com', $this->templateService->getFileName('http://example.com'));
+        $this->assertSame('https://example.com', $this->templateService->getFileName('https://example.com'));
+    }
+
+    /**
+     * @test
+     */
+    public function getFileNameReturnsFileCorrectly(): void
+    {
+        $this->assertSame('typo3/index.php', $this->templateService->getFileName('typo3/index.php'));
+    }
+
+    /**
+     * @test
+     */
+    public function getFileNameReturnsNullIfDirectory(): void
+    {
+        $this->assertNull($this->templateService->getFileName(__DIR__));
+    }
+
+    /**
+     * @test
+     */
+    public function getFileNameReturnsNullWithInvalidFileName(): void
+    {
+        $this->assertNull($this->templateService->getFileName('  '));
+        $this->assertNull($this->templateService->getFileName('something/../else'));
+    }
+}
index e80cd19..4f02937 100644 (file)
@@ -63,6 +63,7 @@ use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use TYPO3\CMS\Frontend\Http\UrlProcessorInterface;
 use TYPO3\CMS\Frontend\Imaging\GifBuilder;
 use TYPO3\CMS\Frontend\Page\PageRepository;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
 use TYPO3\CMS\Frontend\Service\TypoLinkCodecService;
 use TYPO3\CMS\Frontend\Typolink\AbstractTypolinkBuilder;
 use TYPO3\CMS\Frontend\Typolink\UnableToLinkException;
@@ -4537,7 +4538,6 @@ class ContentObjectRenderer implements LoggerAwareInterface
             $fileArray = (array)$fileArray;
         }
         $imageResource = null;
-        $tsfe = $this->getTypoScriptFrontendController();
         if ($file === 'GIFBUILDER') {
             /** @var GifBuilder $gifCreator */
             $gifCreator = GeneralUtility::makeInstance(GifBuilder::class);
@@ -4644,16 +4644,17 @@ class ContentObjectRenderer implements LoggerAwareInterface
         // If image was processed by GIFBUILDER:
         // ($imageResource indicates that it was processed the regular way)
         if (!isset($imageResource)) {
-            $theImage = $tsfe->tmpl->getFileName($file);
-            if ($theImage) {
+            try {
+                $theImage = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize((string)$file);
                 $gifCreator = GeneralUtility::makeInstance(GifBuilder::class);
-                /** @var $gifCreator GifBuilder */
                 $gifCreator->init();
                 $info = $gifCreator->imageMagickConvert($theImage, 'WEB');
                 $info['origFile'] = $theImage;
                 // This is needed by \TYPO3\CMS\Frontend\Imaging\GifBuilder, ln 100ff in order for the setup-array to create a unique filename hash.
                 $info['origFile_mtime'] = @filemtime($theImage);
                 $imageResource = $info;
+            } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                // do nothing in case the file path is invalid
             }
         }
         // Hook 'getImgResource': Post-processing of image resources
@@ -4921,7 +4922,12 @@ class ContentObjectRenderer implements LoggerAwareInterface
                         $retVal = $tsfe->sL('LLL:' . $key);
                         break;
                     case 'path':
-                        $retVal = $tsfe->tmpl->getFileName($key);
+                        try {
+                            $retVal = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($key);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            // do nothing in case the file path is invalid
+                            $retVal = null;
+                        }
                         break;
                     case 'cobj':
                         switch ($key) {
index 303db51..e370119 100644 (file)
@@ -13,9 +13,11 @@ namespace TYPO3\CMS\Frontend\ContentObject;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
 use TYPO3\CMS\Core\Type\File\ImageInfo;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
 
 /**
  * Contains FILE class object.
@@ -32,22 +34,25 @@ class FileContentObject extends AbstractContentObject
     {
         $theValue = '';
         $file = isset($conf['file.']) ? $this->cObj->stdWrap($conf['file'], $conf['file.']) : $conf['file'];
-        $file = $this->getTypoScriptFrontendController()->tmpl->getFileName($file);
-        if ($file !== null && file_exists($file)) {
-            $fileInfo = GeneralUtility::split_fileref($file);
-            $extension = $fileInfo['fileext'];
-            if ($extension === 'jpg' || $extension === 'jpeg' || $extension === 'gif' || $extension === 'png') {
-                $imageInfo = GeneralUtility::makeInstance(ImageInfo::class, $file);
-                $altParameters = trim($this->cObj->getAltParam($conf, false));
-                $theValue = '<img src="'
-                            . htmlspecialchars($this->getTypoScriptFrontendController()->absRefPrefix . $file)
-                            . '" width="' . (int)$imageInfo->getWidth() . '" height="' . (int)$imageInfo->getHeight()
-                            . '"' . $this->cObj->getBorderAttr(' border="0"') . ' ' . $altParameters . ' />';
-            } elseif (filesize($file) < 1024 * 1024) {
-                $theValue = file_get_contents($file);
+        try {
+            $file = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($file);
+            if (file_exists($file)) {
+                $fileInfo = GeneralUtility::split_fileref($file);
+                $extension = $fileInfo['fileext'];
+                if ($extension === 'jpg' || $extension === 'jpeg' || $extension === 'gif' || $extension === 'png') {
+                    $imageInfo = GeneralUtility::makeInstance(ImageInfo::class, $file);
+                    $altParameters = trim($this->cObj->getAltParam($conf, false));
+                    $theValue = '<img src="'
+                        . htmlspecialchars($this->getTypoScriptFrontendController()->absRefPrefix . $file)
+                        . '" width="' . (int)$imageInfo->getWidth() . '" height="' . (int)$imageInfo->getHeight()
+                        . '"' . $this->cObj->getBorderAttr(' border="0"') . ' ' . $altParameters . ' />';
+                } elseif (filesize($file) < 1024 * 1024) {
+                    $theValue = file_get_contents($file);
+                }
             }
+        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+            // do nothing
         }
-
         $linkWrap = isset($conf['linkWrap.']) ? $this->cObj->stdWrap($conf['linkWrap'], $conf['linkWrap.']) : $conf['linkWrap'];
         if ($linkWrap) {
             $theValue = $this->cObj->linkWrap($theValue, $linkWrap);
index f70e64c..1e886aa 100644 (file)
@@ -65,6 +65,7 @@ use TYPO3\CMS\Frontend\Http\UrlHandlerInterface;
 use TYPO3\CMS\Frontend\Page\CacheHashCalculator;
 use TYPO3\CMS\Frontend\Page\PageAccessFailureReasons;
 use TYPO3\CMS\Frontend\Page\PageRepository;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
 
 /**
  * Class for the built TypoScript based frontend. Instantiated in
@@ -2534,9 +2535,12 @@ class TypoScriptFrontendController implements LoggerAwareInterface
                     $this->config['rootLine'] = $this->tmpl->rootLine;
                     // Class for render Header and Footer parts
                     if ($this->pSetup['pageHeaderFooterTemplateFile']) {
-                        $file = $this->tmpl->getFileName($this->pSetup['pageHeaderFooterTemplateFile']);
-                        if ($file) {
+                        try {
+                            $file = GeneralUtility::makeInstance(FilePathSanitizer::class)
+                                ->sanitize((string)$this->pSetup['pageHeaderFooterTemplateFile']);
                             $this->pageRenderer->setTemplateFile($file);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            // do nothing
                         }
                     }
                 }
index 4400f3a..ce8fc01 100644 (file)
@@ -24,6 +24,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
 use TYPO3\CMS\Core\Utility\PathUtility;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
 
 /**
  * GIFBUILDER
@@ -675,13 +676,17 @@ class GifBuilder extends GraphicalFunctions
      * Returns the reference to a "resource" in TypoScript.
      *
      * @param string $file The resource value.
-     * @return string Returns the relative filepath
+     * @return string|null Returns the relative filepath or null if it's invalid
      * @access private
      * @see TemplateService::getFileName()
      */
     public function checkFile($file)
     {
-        return $GLOBALS['TSFE']->tmpl->getFileName($file);
+        try {
+            return GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($file);
+        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+            return null;
+        }
     }
 
     /**
index a6936da..911d2d8 100644 (file)
@@ -26,6 +26,7 @@ use TYPO3\CMS\Core\Utility\MathUtility;
 use TYPO3\CMS\Core\Utility\PathUtility;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
 
 /**
  * Class for starting TypoScript page generation
@@ -91,9 +92,11 @@ class PageGenerator
             $pageRenderer->enableMoveJsFromHeaderToFooter();
         }
         if ($tsfe->config['config']['pageRendererTemplateFile']) {
-            $file = $tsfe->tmpl->getFileName($tsfe->config['config']['pageRendererTemplateFile']);
-            if ($file) {
+            try {
+                $file = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($tsfe->config['config']['pageRendererTemplateFile']);
                 $pageRenderer->setTemplateFile($file);
+            } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                // do nothing
             }
         }
         $headerComment = $tsfe->config['config']['headerComment'];
@@ -242,15 +245,19 @@ class PageGenerator
             $pageRenderer->setBaseUrl($tsfe->baseUrl);
         }
         if ($tsfe->pSetup['shortcutIcon']) {
-            $favIcon = ltrim($tsfe->tmpl->getFileName($tsfe->pSetup['shortcutIcon']), '/');
-            $iconFileInfo = GeneralUtility::makeInstance(ImageInfo::class, Environment::getPublicPath() . '/' . $favIcon);
-            if ($iconFileInfo->isFile()) {
-                $iconMimeType = $iconFileInfo->getMimeType();
-                if ($iconMimeType) {
-                    $iconMimeType = ' type="' . $iconMimeType . '"';
-                    $pageRenderer->setIconMimeType($iconMimeType);
+            try {
+                $favIcon = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($tsfe->pSetup['shortcutIcon']);
+                $iconFileInfo = GeneralUtility::makeInstance(ImageInfo::class, Environment::getPublicPath() . '/' . $favIcon);
+                if ($iconFileInfo->isFile()) {
+                    $iconMimeType = $iconFileInfo->getMimeType();
+                    if ($iconMimeType) {
+                        $iconMimeType = ' type="' . $iconMimeType . '"';
+                        $pageRenderer->setIconMimeType($iconMimeType);
+                    }
+                    $pageRenderer->setFavIcon(PathUtility::getAbsoluteWebPath($tsfe->absRefPrefix . $favIcon));
                 }
-                $pageRenderer->setFavIcon(PathUtility::getAbsoluteWebPath($tsfe->absRefPrefix . $favIcon));
+            } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                // do nothing
             }
         }
         // Including CSS files
@@ -290,7 +297,15 @@ class PageGenerator
                     if (isset($cssFileConfig['if.']) && !$tsfe->cObj->checkIf($cssFileConfig['if.'])) {
                         continue;
                     }
-                    $ss = $cssFileConfig['external'] ? $CSSfile : $tsfe->tmpl->getFileName($CSSfile);
+                    if ($cssFileConfig['external']) {
+                        $ss = $CSSfile;
+                    } else {
+                        try {
+                            $ss = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($CSSfile);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            $ss = null;
+                        }
+                    }
                     if ($ss) {
                         if ($cssFileConfig['import']) {
                             if (!$cssFileConfig['external'] && $ss[0] !== '/') {
@@ -324,7 +339,15 @@ class PageGenerator
                     if (isset($cssFileConfig['if.']) && !$tsfe->cObj->checkIf($cssFileConfig['if.'])) {
                         continue;
                     }
-                    $ss = $cssFileConfig['external'] ? $CSSfile : $tsfe->tmpl->getFileName($CSSfile);
+                    if ($cssFileConfig['external']) {
+                        $ss = $CSSfile;
+                    } else {
+                        try {
+                            $ss = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($CSSfile);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            $ss = null;
+                        }
+                    }
                     if ($ss) {
                         if ($cssFileConfig['import']) {
                             if (!$cssFileConfig['external'] && $ss[0] !== '/') {
@@ -388,7 +411,15 @@ class PageGenerator
                     if (isset($tsfe->pSetup['includeJSLibs.'][$key . '.']['if.']) && !$tsfe->cObj->checkIf($tsfe->pSetup['includeJSLibs.'][$key . '.']['if.'])) {
                         continue;
                     }
-                    $ss = $tsfe->pSetup['includeJSLibs.'][$key . '.']['external'] ? $JSfile : $tsfe->tmpl->getFileName($JSfile);
+                    if ($tsfe->pSetup['includeJSLibs.'][$key . '.']['external']) {
+                        $ss = $JSfile;
+                    } else {
+                        try {
+                            $ss = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($JSfile);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            $ss = null;
+                        }
+                    }
                     if ($ss) {
                         $jsFileConfig = &$tsfe->pSetup['includeJSLibs.'][$key . '.'];
                         $type = $jsFileConfig['type'];
@@ -424,7 +455,15 @@ class PageGenerator
                     if (isset($tsfe->pSetup['includeJSFooterlibs.'][$key . '.']['if.']) && !$tsfe->cObj->checkIf($tsfe->pSetup['includeJSFooterlibs.'][$key . '.']['if.'])) {
                         continue;
                     }
-                    $ss = $tsfe->pSetup['includeJSFooterlibs.'][$key . '.']['external'] ? $JSfile : $tsfe->tmpl->getFileName($JSfile);
+                    if ($tsfe->pSetup['includeJSFooterlibs.'][$key . '.']['external']) {
+                        $ss = $JSfile;
+                    } else {
+                        try {
+                            $ss = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($JSfile);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            $ss = null;
+                        }
+                    }
                     if ($ss) {
                         $jsFileConfig = &$tsfe->pSetup['includeJSFooterlibs.'][$key . '.'];
                         $type = $jsFileConfig['type'];
@@ -461,7 +500,15 @@ class PageGenerator
                     if (isset($tsfe->pSetup['includeJS.'][$key . '.']['if.']) && !$tsfe->cObj->checkIf($tsfe->pSetup['includeJS.'][$key . '.']['if.'])) {
                         continue;
                     }
-                    $ss = $tsfe->pSetup['includeJS.'][$key . '.']['external'] ? $JSfile : $tsfe->tmpl->getFileName($JSfile);
+                    if ($tsfe->pSetup['includeJS.'][$key . '.']['external']) {
+                        $ss = $JSfile;
+                    } else {
+                        try {
+                            $ss = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($JSfile);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            $ss = null;
+                        }
+                    }
                     if ($ss) {
                         $jsConfig = &$tsfe->pSetup['includeJS.'][$key . '.'];
                         $type = $jsConfig['type'];
@@ -496,7 +543,15 @@ class PageGenerator
                     if (isset($tsfe->pSetup['includeJSFooter.'][$key . '.']['if.']) && !$tsfe->cObj->checkIf($tsfe->pSetup['includeJSFooter.'][$key . '.']['if.'])) {
                         continue;
                     }
-                    $ss = $tsfe->pSetup['includeJSFooter.'][$key . '.']['external'] ? $JSfile : $tsfe->tmpl->getFileName($JSfile);
+                    if ($tsfe->pSetup['includeJSFooter.'][$key . '.']['external']) {
+                        $ss = $JSfile;
+                    } else {
+                        try {
+                            $ss = GeneralUtility::makeInstance(FilePathSanitizer::class)->sanitize($JSfile);
+                        } catch (\TYPO3\CMS\Core\Resource\Exception $e) {
+                            $ss = null;
+                        }
+                    }
                     if ($ss) {
                         $jsConfig = &$tsfe->pSetup['includeJSFooter.'][$key . '.'];
                         $type = $jsConfig['type'];
diff --git a/typo3/sysext/frontend/Classes/Resource/FilePathSanitizer.php b/typo3/sysext/frontend/Classes/Resource/FilePathSanitizer.php
new file mode 100644 (file)
index 0000000..07aefbe
--- /dev/null
@@ -0,0 +1,107 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Frontend\Resource;
+
+/*
+ * 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\Core\Core\Environment;
+use TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidFileException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidFileNameException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidPathException;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\PathUtility;
+
+/**
+ * Checks if a given file path is allowed to be used in TYPO3 Frontend.
+ *
+ * Currently allowed is:
+ * - a file (which must exist) from any of the allowedPaths option, without any ".." inside the path name
+ * - an external URL
+ *
+ * The sanitize method either returns a full URL (in case it's a valid http/https resource)
+ * or a path relative to the public folder of the TYPO3 Frontend.
+ */
+class FilePathSanitizer
+{
+    /**
+     * These are the only paths that are allowed for resources in TYPO3 Frontend.
+     * Additional paths can be added via $GLOBALS['TYPO3_CONF_VARS']['FE']['addAllowedPaths'], where all paths should
+     * be suffixed with a slash "/".
+     *
+     * @var array
+     */
+    protected $allowedPaths = [];
+
+    /**
+     * Sets the paths from where TypoScript resources are allowed to be used:
+     */
+    public function __construct()
+    {
+        $this->allowedPaths = [
+            $GLOBALS['TYPO3_CONF_VARS']['BE']['fileadminDir'],
+            'uploads/',
+            'typo3temp/',
+            TYPO3_mainDir . 'ext/',
+            TYPO3_mainDir . 'sysext/',
+            'typo3conf/ext/'
+        ];
+        if (!empty($GLOBALS['TYPO3_CONF_VARS']['FE']['addAllowedPaths'])) {
+            $paths = GeneralUtility::trimExplode(',', $GLOBALS['TYPO3_CONF_VARS']['FE']['addAllowedPaths'], true);
+            foreach ($paths as $path) {
+                if (is_string($path)) {
+                    $this->allowedPaths[] = $path;
+                }
+            }
+        }
+    }
+
+    /**
+     * Returns the reference used for the frontend inclusion, checks against allowed paths for inclusion.
+     *
+     * @param string $originalFileName
+     * @return string Resulting filename, is either a full absolute URL or a relative path.
+     */
+    public function sanitize(string $originalFileName): string
+    {
+        $file = trim($originalFileName);
+        if (empty($file)) {
+            throw new InvalidFileNameException('Empty file name given', 1530169746);
+        }
+        if (strpos($file, '../') !== false) {
+            throw new InvalidPathException('File path "' . $file . '" contains illegal string "../"', 1530169814);
+        }
+        // if this is an URL, it can be returned directly
+        $urlScheme = parse_url($file, PHP_URL_SCHEME);
+        if ($urlScheme === 'https' || $urlScheme === 'http' || is_file(Environment::getPublicPath() . '/' . $file)) {
+            return $file;
+        }
+
+        // this call also resolves EXT:myext/ files
+        $file = GeneralUtility::getFileAbsFileName($file);
+        if (!$file || is_dir($file)) {
+            throw new FileDoesNotExistException('File "' . $originalFileName . '" was not found', 1530169845);
+        }
+
+        $file = PathUtility::stripPathSitePrefix($file);
+
+        // Check if the found file is in the allowed paths
+        foreach ($this->allowedPaths as $allowedPath) {
+            if (strpos((string)$file, (string)$allowedPath, 0) === 0) {
+                return $file;
+            }
+        }
+        throw new InvalidFileException('"' . $file . '" was not located in the allowed paths', 1530169955);
+    }
+}
index 1defc4f..c583f7c 100644 (file)
@@ -87,11 +87,6 @@ class ContentObjectRendererTest extends UnitTestCase
     protected $frontendControllerMock;
 
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject|CacheFrontendInterface
-     */
-    protected $cacheFrontendMock;
-
-    /**
      * @var \PHPUnit_Framework_MockObject_MockObject|TemplateService
      */
     protected $templateServiceMock;
@@ -132,7 +127,7 @@ class ContentObjectRendererTest extends UnitTestCase
     {
         $this->templateServiceMock =
             $this->getMockBuilder(TemplateService::class)
-            ->setMethods(['getFileName', 'linkData'])->getMock();
+            ->setMethods(['linkData'])->getMock();
         $pageRepositoryMock =
             $this->getAccessibleMock(PageRepository::class, ['getRawRecord', 'getMountPointInfo']);
         $this->frontendControllerMock =
@@ -194,11 +189,12 @@ class ContentObjectRendererTest extends UnitTestCase
      */
     public function getImgResourceCallsGetImgResourcePostProcessHook()
     {
-        $this->templateServiceMock
-            ->expects($this->atLeastOnce())
-            ->method('getFileName')
-            ->with('typo3/clear.gif')
-            ->will($this->returnValue('typo3/clear.gif'));
+        $cacheManagerProphecy = $this->prophesize(CacheManager::class);
+        $cacheProphecy = $this->prophesize(CacheFrontendInterface::class);
+        $cacheManagerProphecy->getCache('cache_imagesizes')->willReturn($cacheProphecy->reveal());
+        $cacheProphecy->get(Argument::cetera())->willReturn(false);
+        $cacheProphecy->set(Argument::cetera())->willReturn(false);
+        GeneralUtility::setSingletonInstance(CacheManager::class, $cacheManagerProphecy->reveal());
 
         $resourceFactory = $this->createMock(ResourceFactory::class);
         $this->subject->expects($this->any())->method('getResourceFactory')->will($this->returnValue($resourceFactory));
@@ -214,7 +210,7 @@ class ContentObjectRendererTest extends UnitTestCase
             ->will($this->returnCallback([$this, 'isGetImgResourceHookCalledCallback']));
         $getImgResourceHookObjects = [$getImgResourceHookMock];
         $this->subject->_setRef('getImgResourceHookObjects', $getImgResourceHookObjects);
-        $this->subject->getImgResource('typo3/clear.gif', []);
+        $this->subject->getImgResource('typo3/sysext/core/Tests/Unit/Utility/Fixtures/clear.gif', []);
     }
 
     /**
@@ -229,8 +225,8 @@ class ContentObjectRendererTest extends UnitTestCase
      */
     public function isGetImgResourceHookCalledCallback($file, $fileArray, $imageResource, $parent)
     {
-        $this->assertEquals('typo3/clear.gif', $file);
-        $this->assertEquals('typo3/clear.gif', $imageResource['origFile']);
+        $this->assertEquals('typo3/sysext/core/Tests/Unit/Utility/Fixtures/clear.gif', $file);
+        $this->assertEquals('typo3/sysext/core/Tests/Unit/Utility/Fixtures/clear.gif', $imageResource['origFile']);
         $this->assertTrue(is_array($fileArray));
         $this->assertTrue($parent instanceof ContentObjectRenderer);
         return $imageResource;
@@ -1583,10 +1579,8 @@ class ContentObjectRendererTest extends UnitTestCase
      */
     public function getDataWithTypePath()
     {
-        $filenameIn = $this->getUniqueId('someValue');
-        $filenameOut = $this->getUniqueId('someValue');
-        $this->templateServiceMock->expects($this->atLeastOnce())->method('getFileName')->with($filenameIn)->will($this->returnValue($filenameOut));
-        $this->assertEquals($filenameOut, $this->subject->getData('path:' . $filenameIn));
+        $filenameIn = 'typo3/sysext/frontend/Public/Icons/Extension.svg';
+        $this->assertEquals($filenameIn, $this->subject->getData('path:' . $filenameIn));
     }
 
     /**
diff --git a/typo3/sysext/frontend/Tests/Unit/Resource/FilePathSanitizerTest.php b/typo3/sysext/frontend/Tests/Unit/Resource/FilePathSanitizerTest.php
new file mode 100644 (file)
index 0000000..c4b64b2
--- /dev/null
@@ -0,0 +1,67 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Frontend\Tests\Unit\Resource;
+
+/*
+ * 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\Core\Resource\Exception\FileDoesNotExistException;
+use TYPO3\CMS\Core\Resource\Exception\InvalidFileNameException;
+use TYPO3\CMS\Frontend\Resource\FilePathSanitizer;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+/**
+ * Testcase for \TYPO3\CMS\Frontend\Resource\FilePathSanitizer
+ */
+class FilePathSanitizerTest extends UnitTestCase
+{
+    /**
+     * @test
+     */
+    public function sanitizeReturnsUrlCorrectly(): void
+    {
+        $subject = new FilePathSanitizer();
+        $this->assertSame('http://example.com', $subject->sanitize('http://example.com'));
+        $this->assertSame('https://example.com', $subject->sanitize('https://example.com'));
+    }
+
+    /**
+     * @test
+     */
+    public function sanitizeReturnsFileCorrectly(): void
+    {
+        $subject = new FilePathSanitizer();
+        $this->assertSame('typo3/index.php', $subject->sanitize('typo3/index.php'));
+    }
+
+    /**
+     * @test
+     */
+    public function sanitizeFailsIfDirectoryGiven(): void
+    {
+        $this->expectException(FileDoesNotExistException::class);
+        $subject = new FilePathSanitizer();
+        $subject->sanitize(__DIR__);
+    }
+
+    /**
+     * @test
+     */
+    public function sanitizeThrowsExceptionWithInvalidFileName(): void
+    {
+        $this->expectException(InvalidFileNameException::class);
+        $this->assertNull((new FilePathSanitizer())->sanitize('  '));
+        $this->assertNull((new FilePathSanitizer())->sanitize('something/../else'));
+    }
+}
index 81b515b..05ad567 100644 (file)
@@ -2417,4 +2417,11 @@ return [
             'Deprecation-85196-ProtectSetupModuleController.rst'
         ],
     ],
+    'TYPO3\CMS\Core\TypoScript\TemplateService->getFileName' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-85445-TemplateService-getFileName.rst'
+        ],
+    ],
 ];
index 40a708f..218ea50 100644 (file)
@@ -411,29 +411,34 @@ return [
             'Deprecation-85125-UsagesOfCharsetConverterInCore.rst'
         ],
     ],
-    'TYPO3\CMS\Core\Controller\TypoScriptFrontendController->showHiddenPage' => [
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->showHiddenPage' => [
         'restFiles' => [
             'Deprecation-85389-VariousPublicPropertiesInFavorOfContextAPI.rst'
         ],
     ],
-    'TYPO3\CMS\Core\Controller\TypoScriptFrontendController->showHiddenRecords' => [
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->showHiddenRecords' => [
         'restFiles' => [
             'Deprecation-85389-VariousPublicPropertiesInFavorOfContextAPI.rst'
         ],
     ],
-    'TYPO3\CMS\Core\Controller\TypoScriptFrontendController->gr_list' => [
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->gr_list' => [
         'restFiles' => [
             'Deprecation-85389-VariousPublicPropertiesInFavorOfContextAPI.rst'
         ],
     ],
-    'TYPO3\CMS\Core\Controller\TypoScriptFrontendController->loginUser' => [
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->loginUser' => [
         'restFiles' => [
             'Deprecation-85389-VariousPublicPropertiesInFavorOfContextAPI.rst'
         ],
     ],
-    'TYPO3\CMS\Core\Controller\TypoScriptFrontendController->beUserLogin' => [
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->beUserLogin' => [
         'restFiles' => [
             'Deprecation-85389-VariousPublicPropertiesInFavorOfContextAPI.rst'
         ],
     ],
+    'TYPO3\CMS\Core\TypoScript\TemplateService->fileCache' => [
+        'restFiles' => [
+            'Deprecation-85445-TemplateService-getFileName.rst'
+        ],
+    ],
 ];