From e4fb92a85bed8067d45d2c25c4b559642f206248 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Tue, 12 May 2020 11:22:10 +0200
Subject: [PATCH] [SECURITY] Avoid insecure deserialization of $BE_USER->uc
properties
General and unscoped collection of user settings in $BE_USER->uc is
vulnerable to insecure deserialization, triggered by lots of different
consumers invoking `unserialize()`.
Class deserialization is denied by using option
`['allowed_classes' => false]`.
Resolves: #90313
Releases: master, 9.5
Change-Id: Ic969441bcd4e85fcdbbde23f539bfbcb629ffbb4
Security-Bulletin: TYPO3-CORE-SA-2020-005
Security-References: CVE-2020-11067
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64469
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
.../AbstractUserAuthentication.php | 2 +-
.../Core/Widget/AjaxWidgetContextHolder.php | 34 +++++++--
.../Classes/Core/Widget/WidgetContext.php | 69 ++++++++++++++++---
.../Unit/Core/Widget/WidgetContextTest.php | 4 +-
.../Controller/Remote/ActionHandler.php | 4 +-
5 files changed, 91 insertions(+), 22 deletions(-)
diff --git a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
index 5620c51b97bb..7b30845cf628 100644
--- a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
+++ b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
@@ -1161,7 +1161,7 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
public function unpack_uc($theUC = '')
{
if (!$theUC && isset($this->user['uc'])) {
- $theUC = unserialize($this->user['uc']);
+ $theUC = unserialize($this->user['uc'], ['allowed_classes' => false]);
}
if (is_array($theUC)) {
$this->uc = $theUC;
diff --git a/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php b/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
index 7ec47ceb2769..24edbcbdd36c 100644
--- a/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
+++ b/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
@@ -32,7 +32,7 @@ class AjaxWidgetContextHolder implements SingletonInterface
* An array $ajaxWidgetIdentifier => $widgetContext
* which stores the widget context.
*
- * @var array
+ * @var WidgetContext[]
*/
protected $widgetContexts = [];
@@ -55,9 +55,13 @@ class AjaxWidgetContextHolder implements SingletonInterface
protected function loadWidgetContexts()
{
if (isset($GLOBALS['TSFE']) && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController) {
- $this->widgetContexts = unserialize($GLOBALS['TSFE']->fe_user->getKey('ses', $this->widgetContextsStorageKey));
+ $this->widgetContexts = $this->buildWidgetContextsFromArray(
+ json_decode($GLOBALS['TSFE']->fe_user->getKey('ses', $this->widgetContextsStorageKey ?? null), true) ?? []
+ );
} else {
- $this->widgetContexts = isset($GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey]) ? unserialize($GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey]) : [];
+ $this->widgetContexts = $this->buildWidgetContextsFromArray(
+ json_decode($GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey] ?? '', true) ?? []
+ );
$GLOBALS['BE_USER']->writeUC();
}
}
@@ -66,7 +70,7 @@ class AjaxWidgetContextHolder implements SingletonInterface
* Get the widget context for the given $ajaxWidgetId.
*
* @param string $ajaxWidgetId
- * @return \TYPO3\CMS\Fluid\Core\Widget\WidgetContext
+ * @return WidgetContext
*/
public function get($ajaxWidgetId)
{
@@ -80,7 +84,7 @@ class AjaxWidgetContextHolder implements SingletonInterface
* Stores the WidgetContext inside the Context, and sets the
* AjaxWidgetIdentifier inside the Widget Context correctly.
*
- * @param \TYPO3\CMS\Fluid\Core\Widget\WidgetContext $widgetContext
+ * @param WidgetContext $widgetContext
*/
public function store(WidgetContext $widgetContext)
{
@@ -96,11 +100,27 @@ class AjaxWidgetContextHolder implements SingletonInterface
protected function storeWidgetContexts()
{
if (isset($GLOBALS['TSFE']) && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController) {
- $GLOBALS['TSFE']->fe_user->setKey('ses', $this->widgetContextsStorageKey, serialize($this->widgetContexts));
+ $GLOBALS['TSFE']->fe_user->setKey('ses', $this->widgetContextsStorageKey, json_encode($this->widgetContexts));
$GLOBALS['TSFE']->fe_user->storeSessionData();
} else {
- $GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey] = serialize($this->widgetContexts);
+ $GLOBALS['BE_USER']->uc[$this->widgetContextsStorageKey] = json_encode($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
+ {
+ $widgetContexts = [];
+ foreach ($data as $widgetId => $widgetContextData) {
+ $widgetContexts[$widgetId] = WidgetContext::fromArray($widgetContextData);
+ }
+ return $widgetContexts;
+ }
}
diff --git a/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php b/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
index 0049f86a4a58..5f2035182ade 100644
--- a/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
+++ b/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
@@ -29,8 +29,19 @@ use TYPO3Fluid\Fluid\Core\Rendering\RenderingContextInterface;
*
* @internal It is a purely internal class which should not be used outside of Fluid.
*/
-class WidgetContext
+class WidgetContext implements \JsonSerializable
{
+ protected const JSON_SERIALIZABLE_PROPERTIES = [
+ 'widgetIdentifier',
+ 'ajaxWidgetIdentifier',
+ 'widgetConfiguration',
+ 'controllerObjectName',
+ 'parentPluginNamespace',
+ 'parentExtensionName',
+ 'parentPluginName',
+ 'widgetViewHelperClassName',
+ ];
+
/**
* Uniquely identifies a Widget Instance on a certain page.
*
@@ -47,7 +58,8 @@ class WidgetContext
/**
* User-supplied widget configuration, available inside the widget
- * controller as $this->widgetConfiguration.
+ * controller as $this->widgetConfiguration. Array items must be
+ * null, scalar, array or implement \JsonSerializable interface.
*
* @var array
*/
@@ -97,6 +109,51 @@ class WidgetContext
*/
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
*/
@@ -266,12 +323,4 @@ class WidgetContext
{
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 1adc67137ad6..14035d27ede4 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 sleepReturnsExpectedPropertyNames()
+ public function jsonEncodeContainsExpectedPropertyNames()
{
self::assertEquals(
[
'widgetIdentifier', 'ajaxWidgetIdentifier', 'widgetConfiguration', 'controllerObjectName',
'parentPluginNamespace', 'parentExtensionName', 'parentPluginName', 'widgetViewHelperClassName'
],
- $this->widgetContext->__sleep()
+ array_keys(json_decode(json_encode($this->widgetContext), true))
);
}
}
diff --git a/typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php b/typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php
index dfad0e82d5bb..2b05be4e7247 100644
--- a/typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php
+++ b/typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php
@@ -368,7 +368,7 @@ class ActionHandler
}
$beUserRecord = BackendUtility::getRecord('be_users', (int)$userUid);
if (is_array($beUserRecord) && $beUserRecord['email'] !== '') {
- $uc = $beUserRecord['uc'] ? unserialize($beUserRecord['uc']) : [];
+ $uc = $beUserRecord['uc'] ? unserialize($beUserRecord['uc'], ['allowed_classes' => false]) : [];
$recipients[$beUserRecord['email']] = [
'email' => $beUserRecord['email'],
'lang' => $uc['lang'] ?? $beUserRecord['lang']
@@ -386,7 +386,7 @@ class ActionHandler
continue;
}
if (!isset($recipients[$preselectedBackendUser['email']])) {
- $uc = (!empty($preselectedBackendUser['uc']) ? unserialize($preselectedBackendUser['uc']) : []);
+ $uc = (!empty($preselectedBackendUser['uc']) ? unserialize($preselectedBackendUser['uc'], ['allowed_classes' => false]) : []);
$recipients[$preselectedBackendUser['email']] = [
'email' => $preselectedBackendUser['email'],
'lang' => $uc['lang'] ?? $preselectedBackendUser['lang']
--
GitLab