From 640a6f62858be87db69031f9112c9f378ea00aaa Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Tue, 13 Dec 2022 10:19:53 +0100
Subject: [PATCH] [SECURITY] Use signed storage PID during frontend
 authentication

This change ensures that individual storage page ids are
valid by signing corresponding values with an HMAC.

Resolves: #98010
Releases: main, 11.5, 10.4
Change-Id: I34d474ab23adca6bbcf20c108bb60acf6998bc6f
Security-Bulletin: TYPO3-CORE-SA-2022-013
Security-References: CVE-2022-23501
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77090
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
 .../Controller/AbstractLoginFormController.php        | 11 +++++++++++
 .../felogin/Classes/Controller/LoginController.php    |  4 ++--
 .../Classes/Middleware/FrontendUserAuthenticator.php  | 11 ++++++++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/typo3/sysext/felogin/Classes/Controller/AbstractLoginFormController.php b/typo3/sysext/felogin/Classes/Controller/AbstractLoginFormController.php
index 1448016ac1b7..5b1c6be9b7d4 100644
--- a/typo3/sysext/felogin/Classes/Controller/AbstractLoginFormController.php
+++ b/typo3/sysext/felogin/Classes/Controller/AbstractLoginFormController.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\FrontendLogin\Controller;
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
+use TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication;
 
 abstract class AbstractLoginFormController extends ActionController
 {
@@ -47,4 +48,14 @@ abstract class AbstractLoginFormController extends ActionController
 
         return array_unique($storagePids);
     }
+
+    protected function getSignedStorageFolders(): string
+    {
+        $pidList = implode(',', $this->getStorageFolders());
+        return sprintf(
+            '%s@%s',
+            $pidList,
+            GeneralUtility::hmac($pidList, FrontendUserAuthentication::class)
+        );
+    }
 }
diff --git a/typo3/sysext/felogin/Classes/Controller/LoginController.php b/typo3/sysext/felogin/Classes/Controller/LoginController.php
index f81fa3d99bf9..a1df224906f5 100644
--- a/typo3/sysext/felogin/Classes/Controller/LoginController.php
+++ b/typo3/sysext/felogin/Classes/Controller/LoginController.php
@@ -156,7 +156,7 @@ class LoginController extends AbstractLoginFormController
             [
                 'cookieWarning' => $this->showCookieWarning,
                 'messageKey' => $this->getStatusMessageKey(),
-                'storagePid' => implode(',', $this->getStorageFolders()),
+                'storagePid' => $this->getSignedStorageFolders(),
                 'permaloginStatus' => $this->getPermaloginStatus(),
                 'redirectURL' => $this->redirectHandler->getLoginFormRedirectUrl($this->configuration, $this->isRedirectDisabled()),
                 'redirectReferrer' => $this->request->hasArgument('redirectReferrer') ? (string)$this->request->getArgument('redirectReferrer') : '',
@@ -202,7 +202,7 @@ class LoginController extends AbstractLoginFormController
             [
                 'cookieWarning' => $this->showCookieWarning,
                 'user' => $this->userService->getFeUserData(),
-                'storagePid' => implode(',', $this->getStorageFolders()),
+                'storagePid' => $this->getSignedStorageFolders(),
                 'noRedirect' => $this->isRedirectDisabled(),
                 'actionUri' => $this->redirectHandler->getLogoutFormRedirectUrl($this->configuration, $redirectPageLogout, $this->isRedirectDisabled()),
             ]
diff --git a/typo3/sysext/frontend/Classes/Middleware/FrontendUserAuthenticator.php b/typo3/sysext/frontend/Classes/Middleware/FrontendUserAuthenticator.php
index 1dad4f4e33f1..bee3126ee4ab 100644
--- a/typo3/sysext/frontend/Classes/Middleware/FrontendUserAuthenticator.php
+++ b/typo3/sysext/frontend/Classes/Middleware/FrontendUserAuthenticator.php
@@ -63,10 +63,15 @@ class FrontendUserAuthenticator implements MiddlewareInterface, LoggerAwareInter
     {
         $frontendUser = GeneralUtility::makeInstance(FrontendUserAuthentication::class);
 
+        $pidValue = (string)($request->getParsedBody()['pid'] ?? $request->getQueryParams()['pid'] ?? '');
+        $pidParts = GeneralUtility::trimExplode('@', $pidValue, true, 2);
+        $pid = $pidParts[0] ?? '';
+        $givenHash = $pidParts[1] ?? '';
+        $expectedHash = GeneralUtility::hmac($pid, FrontendUserAuthentication::class);
+
         // List of page IDs where to look for frontend user records
-        $pid = $request->getParsedBody()['pid'] ?? $request->getQueryParams()['pid'] ?? 0;
-        if ($pid) {
-            $frontendUser->checkPid_value = implode(',', GeneralUtility::intExplode(',', (string)$pid));
+        if ($pid && hash_equals($expectedHash, $givenHash)) {
+            $frontendUser->checkPid_value = implode(',', GeneralUtility::intExplode(',', $pid));
         }
 
         // Rate Limiting
-- 
GitLab