From 18e3f4f7db2c2453579082c84c95ee8b2105e41b Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Sat, 16 May 2020 12:05:11 +0200
Subject: [PATCH] [BUGFIX] Allow arbitrary objects in widget context
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
TYPO3-CORE-SA-2020-005 caused side-effects on Fluid AJAX widgets which
unfortunatelly support any class instance to be temporarily stored in
the current user-session. With mentioned change to address an insecure
deserialization vulnerability it was limited to items that could be
JSON-serialized.
This limitation is removed again by switching back to `unserialize()`,
but using an encryption-key-based HMAC signature on the payload.
Due to its architecture there is no better approach available.
This partially reverts commit e4fb92a85bed8067d45d2c25c4b559642f206248.
Resolves: #91382
Releases: master, 9.5
Change-Id: I68cbd15e7df2f536180f174fa63cf27f8a19cfcd
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64501
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Jonas Götze <jonnsn@gmail.com>
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Susanne Moog <look@susi.dev>
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Susanne Moog <look@susi.dev>
---
.../Core/Widget/AjaxWidgetContextHolder.php | 50 ++++++++------
.../Classes/Core/Widget/WidgetContext.php | 69 +++----------------
.../Unit/Core/Widget/WidgetContextTest.php | 4 +-
3 files changed, 42 insertions(+), 81 deletions(-)
diff --git a/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php b/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
index 24edbcbdd36c..3476e580e499 100644
--- a/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
+++ b/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
@@ -16,6 +16,8 @@
namespace TYPO3\CMS\Fluid\Core\Widget;
use TYPO3\CMS\Core\SingletonInterface;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Extbase\Security\Cryptography\HashService;
use TYPO3\CMS\Fluid\Core\Widget\Exception\WidgetContextNotFoundException;
use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
@@ -41,11 +43,17 @@ class AjaxWidgetContextHolder implements SingletonInterface
*/
protected $widgetContextsStorageKey = 'TYPO3\\CMS\\Fluid\\Core\\Widget\\AjaxWidgetContextHolder_widgetContexts';
+ /**
+ * @var HashService
+ */
+ protected $hashService;
+
/**
* Constructor
*/
public function __construct()
{
+ $this->hashService = GeneralUtility::makeInstance(HashService::class);
$this->loadWidgetContexts();
}
@@ -55,13 +63,9 @@ class AjaxWidgetContextHolder implements SingletonInterface
protected function loadWidgetContexts()
{
if (isset($GLOBALS['TSFE']) && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController) {
- $this->widgetContexts = $this->buildWidgetContextsFromArray(
- json_decode($GLOBALS['TSFE']->fe_user->getKey('ses', $this->widgetContextsStorageKey ?? null), true) ?? []
- );
+ $this->widgetContexts = $this->unserializeWithHmac($GLOBALS['TSFE']->fe_user->getKey('ses', $this->widgetContextsStorageKey));
} else {
- $this->widgetContexts = $this->buildWidgetContextsFromArray(
- json_decode($GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey] ?? '', true) ?? []
- );
+ $this->widgetContexts = isset($GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey]) ? $this->unserializeWithHmac($GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey]) : [];
$GLOBALS['BE_USER']->writeUC();
}
}
@@ -100,27 +104,33 @@ class AjaxWidgetContextHolder implements SingletonInterface
protected function storeWidgetContexts()
{
if (isset($GLOBALS['TSFE']) && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController) {
- $GLOBALS['TSFE']->fe_user->setKey('ses', $this->widgetContextsStorageKey, json_encode($this->widgetContexts));
+ $GLOBALS['TSFE']->fe_user->setKey('ses', $this->widgetContextsStorageKey, $this->serializeWithHmac($this->widgetContexts));
$GLOBALS['TSFE']->fe_user->storeSessionData();
} else {
- $GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey] = json_encode($this->widgetContexts);
+ $GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey] = $this->serializeWithHmac($this->widgetContexts);
$GLOBALS['BE_USER']->writeUC();
}
}
- /**
- * Builds WidgetContext instances from JSON representation,
- * this is basically required for AJAX widgets only.
- *
- * @param array $data
- * @return WidgetContext[]
- */
- protected function buildWidgetContextsFromArray(array $data): array
+ protected function serializeWithHmac(array $widgetContexts): string
{
- $widgetContexts = [];
- foreach ($data as $widgetId => $widgetContextData) {
- $widgetContexts[$widgetId] = WidgetContext::fromArray($widgetContextData);
+ $data = serialize($widgetContexts);
+ return $this->hashService->appendHmac($data);
+ }
+
+ protected function unserializeWithHmac(?string $data): array
+ {
+ if ($data === null || $data === '') {
+ return [];
+ }
+ try {
+ $data = $this->hashService->validateAndStripHmac($data);
+ // widget contexts literally can contain everything, that why using
+ // HMAC-signed `unserialize()` is the only option here unless Extbase
+ // structures have been refactored further
+ $widgetContexts = unserialize($data);
+ } finally {
+ return is_array($widgetContexts ?? null) ? $widgetContexts : [];
}
- return $widgetContexts;
}
}
diff --git a/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php b/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
index 5f2035182ade..0049f86a4a58 100644
--- a/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
+++ b/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
@@ -29,19 +29,8 @@ use TYPO3Fluid\Fluid\Core\Rendering\RenderingContextInterface;
*
* @internal It is a purely internal class which should not be used outside of Fluid.
*/
-class WidgetContext implements \JsonSerializable
+class WidgetContext
{
- protected const JSON_SERIALIZABLE_PROPERTIES = [
- 'widgetIdentifier',
- 'ajaxWidgetIdentifier',
- 'widgetConfiguration',
- 'controllerObjectName',
- 'parentPluginNamespace',
- 'parentExtensionName',
- 'parentPluginName',
- 'widgetViewHelperClassName',
- ];
-
/**
* Uniquely identifies a Widget Instance on a certain page.
*
@@ -58,8 +47,7 @@ class WidgetContext implements \JsonSerializable
/**
* User-supplied widget configuration, available inside the widget
- * controller as $this->widgetConfiguration. Array items must be
- * null, scalar, array or implement \JsonSerializable interface.
+ * controller as $this->widgetConfiguration.
*
* @var array
*/
@@ -109,51 +97,6 @@ class WidgetContext implements \JsonSerializable
*/
protected $widgetViewHelperClassName;
- public static function fromArray(array $data): WidgetContext
- {
- $target = new WidgetContext();
- foreach ($data as $propertyName => $propertyValue) {
- if (!in_array($propertyName, self::JSON_SERIALIZABLE_PROPERTIES, true)) {
- continue;
- }
- if (!property_exists($target, $propertyName)) {
- continue;
- }
- $target->{$propertyName} = $propertyValue;
- }
- return $target;
- }
-
- public function jsonSerialize()
- {
- $properties = array_fill_keys(self::JSON_SERIALIZABLE_PROPERTIES, true);
- $data = array_intersect_key(get_object_vars($this), $properties);
-
- if (is_array($data['widgetConfiguration'] ?? null)) {
- $data['widgetConfiguration'] = array_filter(
- $data['widgetConfiguration'],
- function ($value): bool {
- if ($value === null || is_scalar($value) || is_array($value)
- || is_object($value) && $value instanceof \JsonSerializable
- ) {
- return true;
- }
- throw new Exception(
- sprintf(
- 'Widget configuration items either must be null, scalar, array or JSON serializable, got "%s"',
- is_object($value) ? get_class($value) : gettype($value)
- ),
- 1588178538
- );
- }
- );
- } else {
- $data['widgetConfiguration'] = null;
- }
-
- return $data;
- }
-
/**
* @return string
*/
@@ -323,4 +266,12 @@ class WidgetContext implements \JsonSerializable
{
return $this->viewHelperChildNodeRenderingContext;
}
+
+ /**
+ * @return array
+ */
+ public function __sleep()
+ {
+ return ['widgetIdentifier', 'ajaxWidgetIdentifier', 'widgetConfiguration', 'controllerObjectName', 'parentPluginNamespace', 'parentExtensionName', 'parentPluginName', 'widgetViewHelperClassName'];
+ }
}
diff --git a/typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetContextTest.php b/typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetContextTest.php
index 14035d27ede4..1adc67137ad6 100644
--- a/typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetContextTest.php
+++ b/typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetContextTest.php
@@ -129,14 +129,14 @@ class WidgetContextTest extends UnitTestCase
/**
* @test
*/
- public function jsonEncodeContainsExpectedPropertyNames()
+ public function sleepReturnsExpectedPropertyNames()
{
self::assertEquals(
[
'widgetIdentifier', 'ajaxWidgetIdentifier', 'widgetConfiguration', 'controllerObjectName',
'parentPluginNamespace', 'parentExtensionName', 'parentPluginName', 'widgetViewHelperClassName'
],
- array_keys(json_decode(json_encode($this->widgetContext), true))
+ $this->widgetContext->__sleep()
);
}
}
--
GitLab