Project

General

Profile

Actions

Bug #99902

closed

"Login Form" -> "Display Password Recovery Link" does not respect "User Storage Page"

Added by Timo Poppinga about 1 year ago. Updated 7 months ago.

Status:
Closed
Priority:
Should have
Assignee:
Category:
felogin
Target version:
-
Start date:
2023-02-09
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
11
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

I'm sorry, but the problem continues to occur, it could also be a security problem:

In the Action:

    public function recoveryAction(string $userIdentifier = null)
    {
        if (empty($userIdentifier)) {
            return $this->htmlResponse();
        }

        $email = $this->userRepository->findEmailByUsernameOrEmailOnPages(
            $userIdentifier,
            $this->getStorageFolders()
        );

        if ($email) {
            $this->recoveryService->sendRecoveryEmail($email);
        }

        if ($this->exposeNoneExistentUser($email)) {
            $this->addFlashMessage(
                $this->getTranslation('forgot_reset_message_error'),
                '',
                AbstractMessage::ERROR
            );
        } else {
            $this->addFlashMessage($this->getTranslation('forgot_reset_message_emailSent'));
        }

        $this->redirect('login', 'Login', 'felogin');
    }

the "User Storage Page" is respect well.

But then the e-mail is forwarded to TYPO3\CMS\FrontendLogin\Service\RecoveryService->sendRecoveryEmail

    public function sendRecoveryEmail(string $emailAddress): void
    {
        $hash = $this->recoveryConfiguration->getForgotHash();
        // @todo: This repository method call should be moved to PasswordRecoveryController, since its
        // @todo: unexpected that it happens here. Would also drop the dependency to FrontendUserRepository
        // @todo: in this sendRecoveryEmail() method and the class.
        $this->userRepository->updateForgotHashForUserByEmail($emailAddress, GeneralUtility::hmac($hash));
        $userInformation = $this->userRepository->fetchUserInformationByEmail($emailAddress);
        $receiver = new Address($emailAddress, $this->getReceiverName($userInformation));
        $email = $this->prepareMail($receiver, $hash);

        $event = new SendRecoveryEmailEvent($email, $userInformation);
        $this->eventDispatcher->dispatch($event);
        $this->mailer->send($event->getEmail());
    }

and this function just fetches the first record in the fe_users table with this mail


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #95132: felogin forgot password with email address - the felogin_forgotHash will be set for all fe_users with the same eMail addressClosedTorben Hansen2021-09-07

Actions
Actions #1

Updated by Oliver Hader about 1 year ago

  • Private changed from No to Yes
Actions #2

Updated by Oliver Hader about 1 year ago

I did not look into the details. However, in case it might be a security problem, please do not publish this on public issue trackers like this one.

Either send the details to or create a new report ("advisory draft") at https://github.com/TYPO3/typo3/security/advisories/new
It's fine right now, I made this issue private.

Actions #3

Updated by Oliver Hader about 1 year ago

If I understood the report correctly, the recovery link is not limited to that particular user in given storage page IDs.

Actions #4

Updated by Timo Poppinga about 1 year ago

At Oliver, yes the TYPO3 resets the first user in the table fe_users with the given E-Mail Address.

Actions #5

Updated by Oliver Hader 10 months ago

  • Project changed from TYPO3 Core to 1716
  • Category deleted (Security)
Actions #6

Updated by Oliver Hader 10 months ago

  • Private changed from Yes to No
Actions #7

Updated by Oliver Hader 10 months ago

  • Project changed from 1716 to TYPO3 Core
Actions #8

Updated by Oliver Hader 10 months ago

  • Category set to felogin

We just discussed this topic concerning security aspects. When focussing on CIA (confidentiality, integrity, availability), we were not able to identify any security impact. Thus, this issue can be handled in public - basically it's a regular bug, the storage-PID should be handed over to the recovery process as well + the recovery hash should contain the UID of the corresponding user-record.

Actions #9

Updated by Torben Hansen 7 months ago

  • Status changed from New to Closed

This problem has already been fixed in v12 with #95132. The change ensures, that the hash is only set to the user found for the given username/email in the configured storage page and that the link is then only sent to exactly this user. The function signature of sendRecoveryEmail() changed accordingly to require the array with user data (including the email address) and the updateForgotHashForUserByEmail() function has been renamed to updateForgotHashForUserByUid().

Since this is a breaking change, it has not been backported to v11.

I'll close this ticket for now.

Actions #10

Updated by Oliver Hader 7 months ago

  • Related to Bug #95132: felogin forgot password with email address - the felogin_forgotHash will be set for all fe_users with the same eMail address added
Actions

Also available in: Atom PDF