Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Username leakage from password reset mechanism #1770

Closed
edwardsph opened this issue Mar 14, 2024 · 4 comments · Fixed by #1775
Closed

Username leakage from password reset mechanism #1770

edwardsph opened this issue Mar 14, 2024 · 4 comments · Fixed by #1775
Assignees
Labels

Comments

@edwardsph
Copy link

In Reset Password /account/password/reset, if you enter a valid username you get
image-20240312-170029
An invalid username results in
image-20240312-170052
This can be used to confirm whether or not a username exists. It would be better if both cases resulted in the same message. That could be the original message or perhaps "A Reset Password link has been sent to the email associated with this username"

This is a simple fix but as it is a security issue, please could you do an immediate release.

See also: #1758

@bourgeoa
Copy link
Member

It seems strange to have a red error message saying that there was success.

What do you think of sending : Failed to send a Reset Password email ?
Which is more generic.

@edwardsph
Copy link
Author

The best practice recommendation is that the response is the same either way as in https://cheatsheetseries.owasp.org/cheatsheets/Forgot_Password_Cheat_Sheet.html

@zg009
Copy link
Contributor

zg009 commented Mar 15, 2024

Hello Alain,

If the change needs to be made I already found the code which would be edited to fix.

static handlePost (request) {
return Promise.resolve()
.then(() => request.validate())
.then(() => request.loadUser())
.then(userAccount => request.sendResetLink(userAccount))
.then(() => request.renderSuccess())
.catch(error => request.error(error))

Simply applying renderSuccess() on the final then and catch would suffice, though it may warrant renaming the method.

Edit: Another option is rendering the same view for success and failure but having a debug message which indicates success or failure to individuals developing their own server.

@bourgeoa
Copy link
Member

bourgeoa commented Mar 15, 2024

I would do it by replacing that line

throw new Error('Account not found for that username')

this will maintain all the logic, and I will add a comment line, why there is no throw Error

Please do a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants