[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/+/77084 Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · Dec 13, 2022 · 96ed3e6 · 96ed3e6
1 parent 73b46b6
commit 96ed3e6
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 4 deletions.
diff --git a/typo3/sysext/felogin/Classes/Controller/AbstractLoginFormController.php b/typo3/sysext/felogin/Classes/Controller/AbstractLoginFormController.php
@@ -19,6 +19,7 @@
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
+use TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication;
 
 abstract class AbstractLoginFormController extends ActionController
 {
@@ -49,4 +50,14 @@ protected function getStorageFolders(): array
 
         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
@@ -153,7 +153,7 @@ public function loginAction(): void
             [
                 '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'): '',
@@ -199,7 +199,7 @@ public function logoutAction(int $redirectPageLogout = 0): void
             [
                 '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
@@ -54,9 +54,14 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
     {
         $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) {
+        if ($pid && hash_equals($expectedHash, $givenHash)) {
             $frontendUser->checkPid_value = implode(',', GeneralUtility::intExplode(',', $pid));
         }