Skip to content

Commit

Permalink
[SECURITY] Avoid insecure deserialization of $BE_USER->uc properties
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@ohader
ohader committed May 12, 2020
1 parent ab4fec2 commit e4fb92a
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 22 deletions.
2 changes: 1 addition & 1 deletion typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
View file
Expand Up @@ -1161,7 +1161,7 @@ public function writeUC($variable = '')
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;
Expand Down
34 changes: 27 additions & 7 deletions typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
View file
Expand Up @@ -32,7 +32,7 @@ class AjaxWidgetContextHolder implements SingletonInterface
* An array $ajaxWidgetIdentifier => $widgetContext
* which stores the widget context.
*
* @var array
* @var WidgetContext[]
*/
protected $widgetContexts = [];

Expand All @@ -55,9 +55,13 @@ public function __construct()
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();
}
}
Expand All @@ -66,7 +70,7 @@ protected function loadWidgetContexts()
* Get the widget context for the given $ajaxWidgetId.
*
* @param string $ajaxWidgetId
* @return \TYPO3\CMS\Fluid\Core\Widget\WidgetContext
* @return WidgetContext
*/
public function get($ajaxWidgetId)
{
Expand All @@ -80,7 +84,7 @@ public function get($ajaxWidgetId)
* 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)
{
Expand All @@ -96,11 +100,27 @@ public function store(WidgetContext $widgetContext)
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;
}
}
69 changes: 59 additions & 10 deletions typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
View file
Expand Up @@ -29,8 +29,19 @@
*
* @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.
*
Expand All @@ -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
*/
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -266,12 +323,4 @@ public function getViewHelperChildNodeRenderingContext()
{
return $this->viewHelperChildNodeRenderingContext;
}

/**
* @return array
*/
public function __sleep()
{
return ['widgetIdentifier', 'ajaxWidgetIdentifier', 'widgetConfiguration', 'controllerObjectName', 'parentPluginNamespace', 'parentExtensionName', 'parentPluginName', 'widgetViewHelperClassName'];
}
}
4 changes: 2 additions & 2 deletions typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetContextTest.php
View file
Expand Up @@ -129,14 +129,14 @@ public function viewHelperChildNodesCanBeReadAgain()
/**
* @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))
);
}
}
4 changes: 2 additions & 2 deletions typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php
View file
Expand Up @@ -368,7 +368,7 @@ public function getRecipientList(array $uidOfRecipients, $additionalRecipients,
}
$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']
Expand All @@ -386,7 +386,7 @@ public function getRecipientList(array $uidOfRecipients, $additionalRecipients,
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']
Expand Down

0 comments on commit e4fb92a

Please sign in to comment.