From df7909b6a1cf0f12a42994d0cc3376b607746142 Mon Sep 17 00:00:00 2001 From: Benni Mack Date: Tue, 14 May 2024 10:01:44 +0200 Subject: [PATCH] [SECURITY] Protect frame GET parameter in tx_cms_showpic eID The "frame" parameter is no longer evaluated in the showpic eID as it allowed uncontrolled resource consumption. This parameter was actually never used by ContentObjectRenderer and existed since the initial commit and is therefore put behind a feature flag. Resolves: #103306 Releases: main, 13.1, 12.4, 11.5 Change-Id: I87019e58c078c8ccafc0b7ce42fe28b49dc068e4 Security-Bulletin: TYPO3-CORE-SA-2024-010 Security-References: CVE-2024-34358 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84259 Reviewed-by: Oliver Hader Tested-by: Oliver Hader --- .../Configuration/DefaultConfiguration.php | 1 + .../DefaultConfigurationDescription.yaml | 3 ++ ...ETParameterInTx_cms_showpicEIDDisabled.rst | 32 +++++++++++++++++++ .../Controller/ShowImageController.php | 12 ++++++- .../frontend/Configuration/Services.yaml | 3 ++ .../Controller/ShowImageControllerTest.php | 2 ++ 6 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 typo3/sysext/core/Documentation/Changelog/11.5.x/Important-103306-FrameGETParameterInTx_cms_showpicEIDDisabled.rst diff --git a/typo3/sysext/core/Configuration/DefaultConfiguration.php b/typo3/sysext/core/Configuration/DefaultConfiguration.php index dcbef9995933..da11acd1a91b 100644 --- a/typo3/sysext/core/Configuration/DefaultConfiguration.php +++ b/typo3/sysext/core/Configuration/DefaultConfiguration.php @@ -78,6 +78,7 @@ 'security.frontend.enforceContentSecurityPolicy' => false, 'security.frontend.allowInsecureSiteResolutionByQueryParameters' => false, 'security.usePasswordPolicyForFrontendUsers' => false, + 'security.frontend.allowInsecureFrameOptionInShowImageController' => false, ], 'createGroup' => '', 'sitename' => 'TYPO3', diff --git a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml index b648384da143..4e756db4f5eb 100644 --- a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml +++ b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml @@ -208,6 +208,9 @@ SYS: security.frontend.enforceContentSecurityPolicy: type: bool description: 'If on, HTTP Content-Security-Policy header will be applied for each HTTP frontend request.' + security.frontend.allowInsecureFrameOptionInShowImageController: + type: bool + description: 'If on, the eID Script "tx_cms_showpic" respects the GET parameter "frame" without being signed. Should not be enabled as this allows uncontrolled resource consumption.' security.frontend.allowInsecureSiteResolutionByQueryParameters: type: bool description: 'If on, site resolution can be overwritten by `&id=...&L=...` parameters, URI path & host are just used as default.' diff --git a/typo3/sysext/core/Documentation/Changelog/11.5.x/Important-103306-FrameGETParameterInTx_cms_showpicEIDDisabled.rst b/typo3/sysext/core/Documentation/Changelog/11.5.x/Important-103306-FrameGETParameterInTx_cms_showpicEIDDisabled.rst new file mode 100644 index 000000000000..37e39c767858 --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/11.5.x/Important-103306-FrameGETParameterInTx_cms_showpicEIDDisabled.rst @@ -0,0 +1,32 @@ +.. include:: /Includes.rst.txt + +.. _important-103306-1714976257: + +======================================================================= +Important: #103306 - Frame GET parameter in tx_cms_showpic eID disabled +======================================================================= + +See :issue:`103306` + +Description +=========== + +The show image controller (eID `tx_cms_showpic`) lacks a cryptographic +HMAC-signature on the frame HTTP query parameter (e.g. +`/index.php?eID=tx_cms_showpic?file=3&...&frame=12345`). +This allows adversaries to instruct the system to produce an arbitrary number of +thumbnail images on the server side. + +To prevent uncontrolled resource consumption, the frame HTTP query parameter is +now ignored, since it could not be used by core APIs. + +The new feature flag +`security.frontend.allowInsecureFrameOptionInShowImageController` — which is +disabled per default — can be used to reactivate the previous behavior: + +.. code-block:: php + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['features']['security.frontend.allowInsecureFrameOptionInShowImageController'] = true; + + +.. index:: Frontend, NotScanned, ext:frontend diff --git a/typo3/sysext/frontend/Classes/Controller/ShowImageController.php b/typo3/sysext/frontend/Classes/Controller/ShowImageController.php index 1f9faae73308..70bcfa4a6fbc 100644 --- a/typo3/sysext/frontend/Classes/Controller/ShowImageController.php +++ b/typo3/sysext/frontend/Classes/Controller/ShowImageController.php @@ -19,6 +19,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use TYPO3\CMS\Core\Configuration\Features; use TYPO3\CMS\Core\Exception; use TYPO3\CMS\Core\Http\Response; use TYPO3\CMS\Core\Resource\File; @@ -110,6 +111,10 @@ class ShowImageController */ protected $imageTag = '###alt###'; + public function __construct( + protected readonly Features $features + ) {} + /** * Init function, setting the input vars in the global space. * @@ -154,7 +159,12 @@ public function initialize() throw new Exception('File processing for local storage is denied', 1594043425); } - $this->frame = $this->request->getQueryParams()['frame'] ?? null; + if ($this->features->isFeatureEnabled('security.frontend.allowInsecureFrameOptionInShowImageController')) { + $frameValue = $this->request->getQueryParams()['frame'] ?? null; + if ($frameValue !== null && MathUtility::canBeInterpretedAsInteger($frameValue)) { + $this->frame = (int)$frameValue; + } + } } /** diff --git a/typo3/sysext/frontend/Configuration/Services.yaml b/typo3/sysext/frontend/Configuration/Services.yaml index bde0a2e1b044..985f8c5fb594 100644 --- a/typo3/sysext/frontend/Configuration/Services.yaml +++ b/typo3/sysext/frontend/Configuration/Services.yaml @@ -15,6 +15,9 @@ services: arguments: $cache: '@cache.assets' + TYPO3\CMS\Frontend\Controller\ShowImageController: + public: true + TYPO3\CMS\Frontend\ContentObject\ContentDataProcessor: public: true diff --git a/typo3/sysext/frontend/Tests/Functional/Controller/ShowImageControllerTest.php b/typo3/sysext/frontend/Tests/Functional/Controller/ShowImageControllerTest.php index 76baec2c584c..c0befd77219d 100644 --- a/typo3/sysext/frontend/Tests/Functional/Controller/ShowImageControllerTest.php +++ b/typo3/sysext/frontend/Tests/Functional/Controller/ShowImageControllerTest.php @@ -22,6 +22,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use Psr\Http\Message\ServerRequestInterface; +use TYPO3\CMS\Core\Configuration\Features; use TYPO3\CMS\Core\Resource\FileInterface; use TYPO3\CMS\Core\Resource\ProcessedFile; use TYPO3\CMS\Core\Resource\ResourceFactory; @@ -49,6 +50,7 @@ protected function setUp(): void ->disableOriginalConstructor() ->getMock(); $this->subject = $this->getMockBuilder(ShowImageController::class) + ->setConstructorArgs([new Features()]) ->onlyMethods(['processImage']) ->getMock(); GeneralUtility::setSingletonInstance(ResourceFactory::class, $this->resourceFactory);