Security Fix for E-mail Verification Bypass - huntr.dev

[huntr-helper]

https://huntr.dev/users/alromh87 has fixed the E-mail Verification Bypass vulnerability 🔨. alromh87 has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 💵. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A Version Affected | ALL Bug Fix | YES Original Pull Request | https://github.com/418sec/UserFrosting/pull/3 Vulnerability README | https://github.com/418sec/huntr/blob/staging/bounties/packagist/userfrosting/userfrosting/3/vulnerability.json

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/3-packagist-userfrosting%2Fuserfrosting

⚙️ Description *

UserFrosting is following strict email verification but can be bypassed by changing email in account settings.

Bypass is fixed by requiring verification before updating to new email.

💻 Technical Description *

Existent verification mecanism was extended to handle email address change. If mail-verification is enabled in settings, acount settings update will store newEmail as a request and send an email with link for verification, after link is open by new email owner settings will be updated to new email.

🐛 Proof of Concept (PoC) *

1. Setup UserFrosting repo
2. Create a user using valid email
3. Verify new acount by following verification link
4. Log in and go to My Account 5 Change the email and save
5. New email will be enabled without verification
6. Now you can login with unverified email

🔥 Proof of Fix (PoF) *

After fix verification email is sent to requested address and account information won't be updated untill verification is completed:

To verify follow received link:

After verification new email can be used, account information is updated:

👍 User Acceptance Testing (UAT)

e-mail can be updated using verification and application works normally

[alexweissman]

Thanks for finding this and submitting a fix! My main concern is with adding a new column to the users table. Having the new email address there seems like poor encapsulation and generally a bad code smell. Would it not make more sense to add an email column to the verifications table?

We could even consider expanding this to allow users to have multiple email addresses, each of which may or may not be verified, and one email address designated as the primary address. Then we change the concept from "verified user" to "verified email address". Privileges (including the ability to sign in) could still be managed depending on whether the user's primary email address has been verified or not. See for example the Github email settings page.

[Silic0nS0ldier]

Related: https://github.com/userfrosting/UserFrosting/issues/820