Skip to content

Commit

Permalink
[SECURITY] Protect frame GET parameter in tx_cms_showpic eID
Browse files Browse the repository at this point in the history
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 <[email protected]>
Tested-by: Oliver Hader <[email protected]>
  • Loading branch information
bmack authored and ohader committed May 14, 2024
1 parent d774642 commit df7909b
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 1 deletion.
1 change: 1 addition & 0 deletions typo3/sysext/core/Configuration/DefaultConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
'security.frontend.enforceContentSecurityPolicy' => false,
'security.frontend.allowInsecureSiteResolutionByQueryParameters' => false,
'security.usePasswordPolicyForFrontendUsers' => false,
'security.frontend.allowInsecureFrameOptionInShowImageController' => false,
],
'createGroup' => '',
'sitename' => 'TYPO3',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Expand Down
Original file line number Diff line number Diff line change
@@ -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
12 changes: 11 additions & 1 deletion typo3/sysext/frontend/Classes/Controller/ShowImageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -110,6 +111,10 @@ class ShowImageController
*/
protected $imageTag = '<img src="###publicUrl###" alt="###alt###" title="###title###" width="###width###" height="###height###" />';

public function __construct(
protected readonly Features $features
) {}

/**
* Init function, setting the input vars in the global space.
*
Expand Down Expand Up @@ -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;
}
}
}

/**
Expand Down
3 changes: 3 additions & 0 deletions typo3/sysext/frontend/Configuration/Services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ services:
arguments:
$cache: '@cache.assets'

TYPO3\CMS\Frontend\Controller\ShowImageController:
public: true

TYPO3\CMS\Frontend\ContentObject\ContentDataProcessor:
public: true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit df7909b

Please sign in to comment.