[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/+/77100 Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · Dec 13, 2022 · 28be9cd · 28be9cd
1 parent 194fc85
commit 28be9cd
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 13 deletions.
diff --git a/typo3/sysext/felogin/Classes/Controller/LoginController.php b/typo3/sysext/felogin/Classes/Controller/LoginController.php
@@ -103,13 +103,13 @@ public function loginAction(): ResponseInterface
             [
                 'cookieWarning' => $this->showCookieWarning,
                 'messageKey' => $this->getStatusMessageKey(),
-                'storagePid' => implode(',', $this->getStorageFolders()),
                 'permaloginStatus' => $this->getPermaloginStatus(),
                 'redirectURL' => $this->redirectHandler->getLoginFormRedirectUrl($this->configuration, $this->isRedirectDisabled()),
                 'redirectReferrer' => $this->request->hasArgument('redirectReferrer') ? (string)$this->request->getArgument('redirectReferrer') : '',
                 'referer' => $this->requestHandler->getPropertyFromGetAndPost('referer'),
                 'noRedirect' => $this->isRedirectDisabled(),
-                'requestToken' => RequestToken::create('core/user-auth/fe'),
+                'requestToken' => RequestToken::create('core/user-auth/fe')
+                    ->withMergedParams(['pid' => implode(',', $this->getStorageFolders())]),
             ]
         );
 
@@ -154,7 +154,6 @@ public function logoutAction(int $redirectPageLogout = 0): ResponseInterface
             [
                 'cookieWarning' => $this->showCookieWarning,
                 'user' => $this->userService->getFeUserData(),
-                'storagePid' => implode(',', $this->getStorageFolders()),
                 'noRedirect' => $this->isRedirectDisabled(),
                 'actionUri' => $this->redirectHandler->getLogoutFormRedirectUrl($this->configuration, $redirectPageLogout, $this->isRedirectDisabled()),
             ]

diff --git a/typo3/sysext/felogin/Classes/Event/ProcessRequestTokenListener.php b/typo3/sysext/felogin/Classes/Event/ProcessRequestTokenListener.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\FrontendLogin\Event;
+
+use TYPO3\CMS\Core\Authentication\Event\BeforeRequestTokenProcessedEvent;
+use TYPO3\CMS\Core\Security\RequestToken;
+use TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication;
+
+/**
+ * Process request token.
+ */
+final class ProcessRequestTokenListener
+{
+    public function __invoke(BeforeRequestTokenProcessedEvent $event): void
+    {
+        $user = $event->getUser();
+        $requestToken = $event->getRequestToken();
+        if (!$user instanceof FrontendUserAuthentication || !$requestToken instanceof RequestToken) {
+            return;
+        }
+        $pidParam = (string)($requestToken->params['pid'] ?? '');
+        if ($user->checkPid) {
+            $user->checkPid_value = $pidParam;
+        }
+    }
+}
diff --git a/typo3/sysext/felogin/Configuration/Services.yaml b/typo3/sysext/felogin/Configuration/Services.yaml
@@ -6,3 +6,8 @@ services:
 
   TYPO3\CMS\FrontendLogin\:
     resource: '../Classes/*'
+
+  TYPO3\CMS\FrontendLogin\Event\ProcessRequestTokenListener:
+    tags:
+      - name: event.listener
+        identifier: felogin-process-request-token
diff --git a/typo3/sysext/felogin/Resources/Private/Templates/Login/Login.html b/typo3/sysext/felogin/Resources/Private/Templates/Login/Login.html
@@ -74,7 +74,6 @@ <h3>
 
         <div class="felogin-hidden">
             <f:form.hidden name="logintype" value="login"/>
-            <f:form.hidden name="pid" value="{storagePid}"/>
             <f:if condition="{redirectURL}!=''">
                 <f:form.hidden name="redirect_url" value="{redirectURL}" />
             </f:if>

diff --git a/typo3/sysext/felogin/Resources/Private/Templates/Login/Logout.html b/typo3/sysext/felogin/Resources/Private/Templates/Login/Logout.html
@@ -28,7 +28,6 @@ <h3>
 
         <div class="felogin-hidden">
             <f:form.hidden name="logintype" value="logout"/>
-            <f:form.hidden name="pid" value="{storagePid}"/>
         </div>
     </fieldset>
 </f:form>

diff --git a/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php b/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php
@@ -210,14 +210,8 @@ public function isRefreshTimeBasedCookie()
     public function getLoginFormData(ServerRequestInterface $request)
     {
         $loginData = parent::getLoginFormData($request);
-        // List of page IDs where to look for frontend user records during login
-        if ($loginData['status'] === LoginType::LOGIN) {
-            $pid = $request->getParsedBody()['pid'] ?? $request->getQueryParams()['pid'] ?? 0;
-            if ($pid) {
-                $this->checkPid_value = implode(',', GeneralUtility::intExplode(',', (string)$pid));
-            }
-        } else {
-            // Needed in order to fetch users which are already logged-in due to fetching from session
+        // Needed in order to fetch users which are already logged-in due to fetching from session
+        if ($loginData['status'] !== LoginType::LOGIN) {
             $this->checkPid_value = null;
         }