Fix username enumeration

[mufeedvh]

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-packagist-pagekit

⚙️ Description *

pagekit is vulnerable to Username Enumeration as the response message on the Reset Password page when an email exists or vise-versa differs making it easy for an attacker to assume an account exists or not.

💻 Technical Description *

An attacker can know if a certain email/user exists or not just by giving in a victim's email address.

If the email exists, the request responds with:

Check your email for the confirmation link.

If it doesn't, it responds with:

Unknown email address.

The function resides in the /pagekit/app/system/modules/user/src/Controller/ResetPasswordController.php:

public function requestAction($email)
{
    try {

        if (App::user()->isAuthenticated()) {
            return App::redirect();
        }

        if (!App::csrf()->validate()) {
            throw new Exception(__('Invalid token. Please try again.'));
        }

        if (empty($email)) {
            throw new Exception(__('Enter a valid email address.'));
        }

        if (!$user = User::findByEmail($email)) {
            throw new Exception(__('Unknown email address.'));
        }

        if ($user->isBlocked()) {
            throw new Exception(__('Your account has not been activated or is blocked.'));
        }

        $key = App::get('auth.random')->generateString(32);
        $url = App::url('@user/resetpassword/confirm', compact('key'), 0);

        try {

            $mail = App::mailer()->create();
            $mail->setTo($user->email)
                ->setSubject(__('Reset password for %site%.', ['%site%' => App::module('system/site')->config('title')]))
                ->setBody(App::view('system/user:mails/reset.php', compact('user', 'url', 'mail')), 'text/html')
                ->send();

        } catch (\Exception $e) {
            throw new Exception(__('Unable to send confirmation link.'));
        }

        $user->activation = $key;
        $user->save();

        App::message()->success(__('Check your email for the confirmation link.'));

        return App::redirect('@user/login');

    } catch (Exception $e) {
        return [
            '$view' => [
                'title' => __('Reset'),
                'name' => 'system/user/reset-request.php',
            ],
            'error' => $e->getMessage()
        ];
    }
}

As you can see, the response comes from these exceptions:

if (!$user = User::findByEmail($email)) {
    throw new Exception(__('Unknown email address.'));
}

and

App::message()->success(__('Check your email for the confirmation link.'));

Just changing these strings are enough to fix the issue. To not enable the attacker to enumerate users, we can change the strings to:

If this email exists, you will receive an email with the reset instructions.

🐛 Proof of Concept (PoC) *

The PoC is nicely detailed in this issue: pagekit#935

As you can see it's just a string response, so changing them are enough to fix the issue.

🔥 Proof of Fix (PoF) *

Changed the two strings to If this email exists, you will receive an email with the reset instructions. making it unable for an attacker to identify account existence.

if (!$user = User::findByEmail($email)) {
    throw new Exception(__('If this email exists, you will receive an email with the reset instructions.'));
}

App::message()->success(__('If this email exists, you will receive an email with the reset instructions.'));

👍 User Acceptance Testing (UAT)

Just a string change, no breaking changes are introduced.

[JamieSlome]

@mufeedvh - make sure you have your bounty URL included in your comments!

[Mik317]

Hi @mufeedvh 😄
great fix!!! The only problem I think could make the fix bypassable is the color of the message (if the frontend receives a exception, won't the message color be red and if successfull green even if the message is the same?).

Regards,
Mik

[toufik-airane]

Nice. 👍

[huntr-helper]

Congratulations mufeedvh - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section. Your bounty is on its way - keep hunting!

[humantex]

Referenced in my response here: pagekit#935 (comment)