[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/+/64464 Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · May 12, 2020 · 7d4159f · 7d4159f
1 parent fa3992d
commit 7d4159f
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 31 deletions.
diff --git a/typo3/sysext/backend/Classes/Controller/FormInlineAjaxController.php b/typo3/sysext/backend/Classes/Controller/FormInlineAjaxController.php
@@ -662,7 +662,7 @@ protected function getInlineExpandCollapseStateArray()
             return [];
         }
 
-        $inlineView = unserialize($backendUser->uc['inlineView']);
+        $inlineView = unserialize($backendUser->uc['inlineView'], ['allowed_classes' => false]);
         if (!is_array($inlineView)) {
             $inlineView = [];
         }

diff --git a/typo3/sysext/backend/Classes/Form/FormDataProvider/TcaInlineExpandCollapseState.php b/typo3/sysext/backend/Classes/Form/FormDataProvider/TcaInlineExpandCollapseState.php
@@ -35,7 +35,7 @@ public function addData(array $result)
             if (!empty($result['inlineTopMostParentUid']) && !empty($result['inlineTopMostParentTableName'])) {
                 // Happens in inline ajax context, top parent uid and top parent table are set
                 if (isset($this->getBackendUser()->uc['inlineView'])) {
-                    $fullInlineState = unserialize($this->getBackendUser()->uc['inlineView']);
+                    $fullInlineState = unserialize($this->getBackendUser()->uc['inlineView'], ['allowed_classes' => false]);
                     if (!is_array($fullInlineState)) {
                         $fullInlineState = [];
                     }
@@ -53,7 +53,7 @@ public function addData(array $result)
                 $result['inlineExpandCollapseStateArray'] = $inlineStateForTable;
             } else {
                 // Default case for a single record
-                $fullInlineState = !empty($this->getBackendUser()->uc['inlineView']) ? unserialize($this->getBackendUser()->uc['inlineView']) : [];
+                $fullInlineState = !empty($this->getBackendUser()->uc['inlineView']) ? unserialize($this->getBackendUser()->uc['inlineView'], ['allowed_classes' => false]) : [];
                 if (!is_array($fullInlineState)) {
                     $fullInlineState = [];
                 }

diff --git a/typo3/sysext/backend/Classes/Form/Utility/FormEngineUtility.php b/typo3/sysext/backend/Classes/Form/Utility/FormEngineUtility.php
@@ -147,7 +147,7 @@ public static function updateInlineView(&$uc, $tce)
     {
         $backendUser = static::getBackendUserAuthentication();
         if (isset($uc['inlineView']) && is_array($uc['inlineView'])) {
-            $inlineView = (array)unserialize($backendUser->uc['inlineView']);
+            $inlineView = (array)unserialize($backendUser->uc['inlineView'], ['allowed_classes' => false]);
             foreach ($uc['inlineView'] as $topTable => $topRecords) {
                 foreach ($topRecords as $topUid => $childElements) {
                     foreach ($childElements as $childTable => $childRecords) {

diff --git a/typo3/sysext/backend/Classes/Tree/View/AbstractTreeView.php b/typo3/sysext/backend/Classes/Tree/View/AbstractTreeView.php
@@ -625,7 +625,7 @@ public function expandNext($id)
     public function initializePositionSaving()
     {
         // Get stored tree structure:
-        $this->stored = unserialize($this->BE_USER->uc['browseTrees'][$this->treeName]);
+        $this->stored = unserialize($this->BE_USER->uc['browseTrees'][$this->treeName], ['allowed_classes' => false]);
         // PM action
         // (If an plus/minus icon has been clicked, the PM GET var is sent and we
         // must update the stored positions in the tree):

diff --git a/typo3/sysext/backend/Classes/Tree/View/FolderTreeView.php b/typo3/sysext/backend/Classes/Tree/View/FolderTreeView.php
@@ -588,7 +588,7 @@ public function getNumberOfSubfolders(Folder $folderObject)
     public function initializePositionSaving()
     {
         // Get stored tree structure:
-        $this->stored = unserialize($this->BE_USER->uc['browseTrees'][$this->treeName]);
+        $this->stored = unserialize($this->BE_USER->uc['browseTrees'][$this->treeName], ['allowed_classes' => false]);
         $this->getShortHashNumberForStorage();
         // PM action:
         // (If an plus/minus icon has been clicked,

diff --git a/typo3/sysext/backend/Classes/Utility/BackendUtility.php b/typo3/sysext/backend/Classes/Utility/BackendUtility.php
@@ -537,7 +537,7 @@ public static function openPageTree($pid, $clearExpansion)
         if ($clearExpansion) {
             $expandedPages = [];
         } else {
-            $expandedPages = unserialize($beUser->uc['browseTrees']['browsePages']);
+            $expandedPages = unserialize($beUser->uc['browseTrees']['browsePages'], ['allowed_classes' => false]);
         }
         // Get rootline:
         $rL = self::BEgetRootLine($pid);

diff --git a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
@@ -1227,7 +1227,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;

diff --git a/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php b/typo3/sysext/fluid/Classes/Core/Widget/AjaxWidgetContextHolder.php
@@ -29,7 +29,7 @@ class AjaxWidgetContextHolder implements \TYPO3\CMS\Core\SingletonInterface
      * An array $ajaxWidgetIdentifier => $widgetContext
      * which stores the widget context.
      *
-     * @var array
+     * @var WidgetContext[]
      */
     protected $widgetContexts = [];
 
@@ -52,9 +52,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();
         }
     }
@@ -63,7 +67,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)
     {
@@ -77,9 +81,9 @@ 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(\TYPO3\CMS\Fluid\Core\Widget\WidgetContext $widgetContext)
+    public function store(WidgetContext $widgetContext)
     {
         $ajaxWidgetId = md5(uniqid(mt_rand(), true));
         $widgetContext->setAjaxWidgetIdentifier($ajaxWidgetId);
@@ -93,11 +97,27 @@ public function store(\TYPO3\CMS\Fluid\Core\Widget\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;
+    }
 }
diff --git a/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php b/typo3/sysext/fluid/Classes/Core/Widget/WidgetContext.php
@@ -25,8 +25,20 @@
  *
  * @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',
+        'parentVendorName',
+        'parentExtensionName',
+        'parentPluginName',
+        'widgetViewHelperClassName',
+    ];
+
     /**
      * Uniquely identifies a Widget Instance on a certain page.
      *
@@ -43,7 +55,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
      */
@@ -98,6 +111,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
      */
@@ -287,12 +345,4 @@ public function getViewHelperChildNodeRenderingContext()
     {
         return $this->viewHelperChildNodeRenderingContext;
     }
-
-    /**
-     * @return array
-     */
-    public function __sleep()
-    {
-        return ['widgetIdentifier', 'ajaxWidgetIdentifier', 'widgetConfiguration', 'controllerObjectName', 'parentPluginNamespace', 'parentVendorName', '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
@@ -126,15 +126,15 @@ public function viewHelperChildNodesCanBeReadAgain()
     /**
      * @test
      */
-    public function sleepReturnsExpectedPropertyNames()
+    public function jsonEncodeContainsExpectedPropertyNames()
     {
         $this->assertEquals(
             [
                 'widgetIdentifier', 'ajaxWidgetIdentifier', 'widgetConfiguration', 'controllerObjectName',
                 'parentPluginNamespace', 'parentVendorName', 'parentExtensionName', 'parentPluginName',
                 'widgetViewHelperClassName'
             ],
-            $this->widgetContext->__sleep()
+            array_keys(json_decode(json_encode($this->widgetContext), true))
         );
     }
 }
diff --git a/typo3/sysext/taskcenter/Classes/Controller/TaskModuleController.php b/typo3/sysext/taskcenter/Classes/Controller/TaskModuleController.php
@@ -470,7 +470,7 @@ public function renderListMenu($items, $mainMenu = false)
 
         // Change the sorting of items to the user's one
         if ($mainMenu) {
-            $userSorting = unserialize($this->getBackendUser()->uc['taskcenter']['sorting']);
+            $userSorting = unserialize($this->getBackendUser()->uc['taskcenter']['sorting'], ['allowed_classes' => false]);
             if (is_array($userSorting)) {
                 $newSorting = [];
                 foreach ($userSorting as $item) {

diff --git a/typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php b/typo3/sysext/workspaces/Classes/Controller/Remote/ActionHandler.php
@@ -365,7 +365,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']
@@ -383,7 +383,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']