Commit e4fb92a8 authored by Oliver Hader's avatar Oliver Hader Committed by Oliver Hader
Browse files

[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's avatarOliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
parent ab4fec2a
......@@ -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;
......
......@@ -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;
}
}
......@@ -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'];
}
}
......@@ -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))
);
}
}
......@@ -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']
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment