Password Broker has a security risk on accepting email parameter on GET requests

[mpyw]

[5.8] Pass notifiable email through reset link by dwightwatson · Pull Request #28475 · laravel/framework

From issue comment:

@dwightwatson @taylorotwell @driesvints Sending email parameter on GET requests seems to have a large security risk. We should prevent servers from logging sensitive information.

I believe it's a bad approach "Retrieve from email, and then verify token". Just follow "Retrieve from token".

[dwightwatson]

Is it a large security risk to have emails appear in your server logs?

Do you have any additional information on this, as well as alternative approaches - rather than just believing that one approach is bad?

You could always put together a PR that implements your proposal and goes over the pros/cons of that approach.

[mpyw]

The handling of personal information in my company is very strict, and email is treated as sensitive information. Although not as strict as raw password, I believe that unnecessary logging should be avoided.

You could always put together a PR that implements your proposal and goes over the pros/cons of that approach.

Yes, I'll submit PR when I have time.

[dwightwatson]

If it's a specific company policy then it's not exactly a large security risk, but rather a good opportunity for you to override the default notification in your application. Alternative approaches to this can be explored through PRs in the meantime.

[mpyw]

You're right, but I think the current implementation has fewer advantages over the new proposal.

Current

• (+) Combining email and token may a bit slightly increase entropy
• (+) No cost to change the implementation
• (-) Logging them unnecessarily increases a security risk

Proposal

• (+) Access logs are kept clean and secure
• (+) Simpler implementation

There are certainly passive reasons to keep the current implementation, but there seems to be no active reason.

[dwightwatson]

From a quick look it doesn't seem like password reset tokens are unique, so you can't look up a user from the token alone - which is what I think you're proposing. That proposal would also require that the tokens are required to be unique.

The simplest solution would be to remove the email address from the link and require the user enters it again manually when choosing a new password, but that's a pretty poor user experience.

[mpyw]

@dwightwatson Absolutely.

"email hash" (almost unique) + "secret" looks better, but it loses simplicity...

[mpyw]

@dwightwatson I realized that there are much room for rethinking. I'll discuss with my colleagues, thank you.