laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
31.86k stars 10.81k forks source link

Security vulnerability in email reset flow #13658

Closed lukasoppermann closed 7 years ago

lukasoppermann commented 8 years ago

Hey,

When a user resets a password with the default flow and enters an email address that is not used the following message is displayed.

We can't find a user with that e-mail address.

The problem is, that an attacker can now find out which email address actually is in the system.

The best practise would be to display a message like this

An email with further instructions has been sent to the provided email address.

The system would always send an email:

If the user exists the reset link would be sent.

If the user does not exist an email like this would be sent:

You or somebody tried to reset an account at domain.com with this email address. However this email address does not exist in our system.

This way an attacker can not know if the email exists or not.

Of course this opens the system up to potential DDoS attacks, but maybe this could be handle with a reset throttle?

barryvdh commented 8 years ago

I think they usually just say 'If an account with this email exists, a password reset link will be sent'. So you don't know if the email actually exists, but the output is still the same (and you don't get spammed with mails if it isn't you). But that is a bit less user-friendly indeed.

I'm guessing that in this case, it is done by design, but if you think you've discovered a security issue, please don't create an issue but send an email directly, as stated in the readme.

If you discover a security vulnerability within Laravel, please send an e-mail to Taylor Otwell at taylor@laravel.com. All security vulnerabilities will be promptly addressed.

lukasoppermann commented 8 years ago

Well I am.pretty sure that best practice is to always send this Email, not just if it exists, but this would of course be a solution as well, maybe it could be configurable.

However, thanks for the info, I will send an Email.

vlakoff commented 8 years ago

This is a bit "security through obscurity", though very valid and to be taken into consideration.

However, if the email isn't registered, don't send anything to it. Otherwise some drunk douchebag could use the website to spam people.

lukasoppermann commented 8 years ago

@vlakoff I have to disagree with both your points. :-) Firstly, its rather security through not disclosing user data publicly and secondly, you need to send the email, otherwise it is actually a big usability no-go, as you might try to reset with a valid email-address you own, but which you did not use for the system.

To prohibit spamming you could simply throttle requests and maybe even block requests for 15min or so, after 5 requests.

vlakoff commented 8 years ago

you might try to reset with a valid email-address you own, but which you did not use for the system.

I think we don't have to hold user's hand this far. If they even forgot the email they used, just let them try the several email addresses they have.

So, as long as the registered emails aren't disclosed (and that there's a throttling mechanism), we're good. If you're working on "cheat on your wife" or "shameful porn" websites, your customers don't want to be this easily trackable :)

laurencei commented 8 years ago

This is only a solution to leaks if your website does not allow self registration. And since most sites do allow self-registration - then this is not required.

i.e. Most websites allow a user to signup, and use a 'unique' email address as part of that process. If an attacker wants to get information about a certain email account in use or not - they can just try and sign up with that email to your system.

The system must tell them the email is already in use, and so the attacker now has that piece of information.

So unless your site does not allow self registration (and/or does not use an email in the signup process) - then you are not preventing anything.

Personally I do not think this change is needed at all. You have the ability to customise the flow if you feel your situation is unique.

johnpaulmedina commented 8 years ago

I wouldn't send an email if it doesn't exist. I also was going to point out self registration as well because that's telling the potential "hacker" or script w.e that the user account already exist. First it waste resources all though very miniscual, it's just useless you can implement better solutions. Also if I was attempting to "hack" or deceive I would probably use a fake test email first that I probably have access to and see that it sends a warning. There would be no difference to telling me that the user doesn't exist on the application GUI or in that inbox or on registration. At that point it's the applications duty to protect the account of true users who do exist. If you are concerned about bots building list on user accounts or data scraping then you should definitely rate limit these login, forgot password, and registration pages as well as implement a 2FA. I do not find that to be standard with any popular systems or design standards to email a user not actually associated with the system.

lukasoppermann commented 8 years ago

@johnpaulmedina this is wrong, there is a difference, because if you do not own my address you will not know if I received a warning email or a reset email, so the email address is NOT disclosed.

@theshiftexchange how do you know there are only a few services without self registration? I do not think this is true. Also in a registration process you may need to go through 2 or more steps before an invalid email warning is shown, thus slowing down automated scripts.

I don't see how this would in any way use up resources.

If there is real data showing that only a few apps have a different registration flow, please provide a link. Otherwise I think using more state of the art security would only be beneficial.

lucasmichot commented 8 years ago

I agree with @lukasoppermann

johnpaulmedina commented 8 years ago

I'm not saying I have access to your email. What myself and @theshiftexchange is saying is that we want to know if you are a user there period. What does it matter if I found it through registration or even the forgot password page I would still know.

The issue of knowing still exist at some level without having access to the inbox.

It's pointless to notify someone who is not a user, in fact you will actually create a bad reputation with email service providers if this starts to become an issue when other unknown users flag your message as spam thus marking your domain with these email providers.

I think this is a personal opinion, not saying you are 100% wrong but that your use case just might be different.

Google, Yahoo, Amazon, AWS, GitHub, Instagram, Facebook - don't do this. It's not a standard.

Apply two form authentication. Add captcha on registration. Rate limit forgot password request. Rate limit login. Rate limit registration. If this is something you are concerned about.

Those features are standard. Respectfully I would have to disagree with this being so much as a security flaw more so use case UI/UX.

As far as resources: In some case you may be working with queues and time sensitive actions. For instance I work with leads and it's determined after X hours if we haven't reached them it's basically useless for us to waste time putting resource to get in touch with that person when our focus can be on the newer opportunities. In this email case, if you are queuing emails and firing transactional items why waste your time with a user who has no relations with the website itself. People get hacked most of the time not so much the systems in place.

You can't defend the people, but you can protect your users and your application. I think your putting to much thought into something that can simply say.

"If a user exist with that email we'll send over the password reset instructions"

antonkomarev commented 7 years ago

+1 to not display if email was actually sent with message "If a user exist with that email we'll send over the password reset instructions".

Feature to send email to unregistered users could be as useful as harmful.

Useful

Harmful

It's good idea, but this should be configurable.

laurencei commented 7 years ago

@lukasoppermann - "how do you know there are only a few services without self registration? I do not think this is true"

Sorry - what I should have said is that the Laravel scaffolding for auth, which includes the password reset workflow that you describe, also includes a default registration workflow.

What do you propose the default registration workflow can be, that does not leak email validation information, to be included in a default Laravel package? Because there is no point having a password reset feature change that does not address the other leakage spot.

I feel this is an issue that is unique to a few specific websites, not most.

If you feel that strongly about the issue - the easiest solution you can do is just change your lang file and change the text from this:

'sent' => 'We have e-mailed your password reset link!',
'user' => "We can't find a user with that e-mail address.",

to this

'sent' => 'If your email exists in our system, we have sent you an email with the password reset details',
'user' => "If your email exists in our system, we have sent you an email with the password reset details",

and you achieve the same security effect without any changes required to the framework.

But my personal opinion is the loss of user experience is not beneficial here (especially with a self registration website).

lukasoppermann commented 7 years ago

@TheShiftExchange to counter the UX loss, you would send an email. This will mean that a user gets an email in any case.

Imagine you have multiple email addresses (a very common case in my experience), receiving this email would help you identify your which email address I used, without disclosing the email address that exist in the database.

The registration problem can be solved similarly:

  1. User signs up using her email
  2. a) (email in use) An email is sent to her (or the email used) telling her that she used is already registered and asking if she wanted to login instead.
  3. b) (email not in use) An email is sent to her (or the email used) asking to verify the email by clicking on a link.

This could be a 2nd option, you could make it so that you can set it to use the "secure registration and reset" or the "default".

@johnpaulmedina I did no mean to say it is a standard as in used everywhere, but rather it is the standard secure way of doing it, as defined by security researchers.

In my opinion Laravel should definitely at least offer this as an option, since its aim is to be easy to use for beginners, providing sensible out-of-the-box solutions for typical tasks (at least thats what I understood when Taylor mentioned in a podcast that he was inspired by the well documented Codeigniter, that let you start out without being an expert). Security is a topic most beginners do not really know to much about, so it would be good to provide a very safe standard.

themsaid commented 7 years ago

@lukasoppermann did you get the chance to open a PR about that and suggest the changes?

lukasoppermann commented 7 years ago

No, not yet. I think it will be quite a bit of work and I am very busy. I do not want to put this work into it, if it has no or very little chance of being added. If this is something that you and @taylor think should be added, let me know and I will be happy to send a PR. Maybe we should discuss how this should be implemented first though. 😉

themsaid commented 7 years ago

Alright :)

May I ask why do you believe an email should be sent anyway?

lukasoppermann commented 7 years ago

This is needed for the case a user is legit but tried to login with a wrong email address that he owns.

E.g. you signed up with you@mail.com but tried to login with youtoo@mail.com. Since you own the email address you will get an email that tells you that you might have used a wrong email address.

themsaid commented 7 years ago

it's already really easy to tell who is in the system by just attempting to register and seeing if the email is already taken. So I don't really think it's a big deal.

That's Taylor.

That said, I think this issue isn't actually a security vulnerability :)

lukasoppermann commented 7 years ago

It is a security vulnerability, it even has a name: "enumeration". The Signup can be protected as well though. For example by requiring a verification via email before you login a user.

themsaid commented 7 years ago

Yeah but you can still know who's in the system by trying to register using an email, you'll be able to know if that email is in the system or not.

lukasoppermann commented 7 years ago

no, you will not. You will get an email to either verify your email, or you will get an email telling you that "you or somebody" tried to register but you already have an account.

ianwalter commented 7 years ago

@themsaid That's a good argument in general, but not in practice. @lukasoppermann scenario is valid, but also, an app's registration process may be more expensive to conduct user enumeration against than the password reset process. For example, a registration process could require a valid invite code before any user email information is leaked. Another scenario might be a registration process that requires purchasing something or submitting valid credit card info before leaking user information.

I think how DigitalOcean handles this is ideal. When you submit the password reset form with a valid email address it responds with "If the email you specified exists in our system, we've sent a password reset link to it." I think this would cover the most broad set of use cases and users could override the controller if they want to tell their users whether the email exists in their system or not.

kaleb commented 7 years ago

I ran into this issue using laravel/spark . As spark is a paid product with a private repository, should I ask that this issue be re-opened, or open a separate issue there, @themsaid ?

laurencei commented 7 years ago

It's really a non-issue.

And you can solve this yourself in your own code by just changing the language file and putting the same "message" for success and unsuccessful messages.

It's a simple fix for those that want it...

ianwalter commented 7 years ago

@laurencei No, it's not that simple. Illuminate sends back the valid email with a status message and the invalid email with errors. See laravel/framework/src/Illuminate/Foundation/Auth/SendsPasswordResetEmails.php.

e.g. in Spark:

screen shot 2017-07-15 at 1 30 47 pm
laurencei commented 7 years ago

@ianwalter - all you do is change your view validation there.

If you are saying "any" email will return a "maybe" - then by definition you cant have a validation error (because you accept anything) - so just have that single page ignore any errors and always display "email sent if it exists" if anything comes back (error or not).

Or change your controller function and just have it always return a success from there (and ignore the validation error).

It's all trivial to implement if you feel that strongly about the issue. But overall it is a loss of UX for little security gain.

ianwalter commented 7 years ago

@laurencei I'm aware of all sorts of hacks you can do that don't work really well but it doesn't change the fact that its a security best practice that is more easily implemented in the code I referenced than in the users code.

Again, I'm not looking for solutions to this problem that I can implement. I am suggesting this be fixed in the framework because I am with the others that think this is the best default. If you don't agree, thats fine, but I'm not spending more time explaining why your other solutions aren't ideal.

johnpaulmedina commented 7 years ago

you can make correction do a pull request following the implementation instructions on the framework repo

devcircus commented 7 years ago

Everyone's needs are different. This code is not behind the vendor wall for this reason. Customizing your auth process is not a hack.

Benjamin0000 commented 5 years ago

I think the best thing to do is just to respond to the user that the email has been sent. If the email actually exist then the reset link will be sent from the back end but if the email does not exist, the email will not be sent although a massage will be given to the user that email has been sent to the provided email address.

m0lmk commented 4 years ago

Did anything get done on this?

Can everyone agree that it's not best practice to let a bad actor know if a specific email address is in use on a system?

I would be pretty annoyed if I used a discrete service and a bad actor could enter my email address and be told that it is in use. Taking an example to the extreme, once a bad actor knows that kevin@example.com has an account with a specific service, the user can then be targeted with very specific and potentially effective phishing campaigns or even blackmail if it's a sensitive service.

laurencei commented 4 years ago

@m0lmk - but what's the alternative when you consider they can do the same attack on a sign up page.

lucasmichot commented 4 years ago

@m0lmk Just update your frontend to change the error message.

m0lmk commented 4 years ago

@laurencei The alternative is check that the email address is a valid format and then display a message like "We have sent an email with further instructions". Registration/login/password reset should never expose any account information.

@lucasmichot I'm not sure that updating the error message is the way forward. As it stands, just changing the error message as documented above will not work.

I don't understand why anyone thinks it is acceptable to leak user information in this way. Take a dating website as an example. Is it OK at any friend of mine could go a a dating website and check to see if I was a user? It the very least it's a breach of data protection, at the worst it could open doors for blackmail.

Troy Hunt explains it far better then I can - https://www.troyhunt.com/everything-you-ever-wanted-to-know/

Miguel-Serejo commented 4 years ago

@m0lmk How do you prevent your friends from just trying to register on the website with your email? This will leak the same information.

The existence of an email in a database is not sufficient proof that whoever owns that email address is a user of the system.

m0lmk commented 4 years ago

The whole point here is that the system should not confirm that any email is in use. It should not leak personally identifiable information.

You don't prevent anyone from registering any email address, you just don't confirm if it is in use to anyone except that actual owner of that email address (which is done by sending them an email!).

laurencei commented 4 years ago

Take a dating website as an example. Is it OK at any friend of mine could go a a dating website and check to see if I was a user?

This is the exact reason why your idea does not work - because I can just go to the same dating website and try to sign you up. If I try to sign you up using your email address, and you already have an account, it wont let me and it'll tell me you already exist.

So then I know you have an account. So the information is leaked either way.

johnpaulmedina commented 4 years ago

The best thing to do is implement what you feel is the best and not rely on the entire framework or community to conform.

If your concern is confirmation of accounts simply change the message to read if an account matches this email we will send you a recovery code (or some variation). I would also put something to track multiple form hits in a rapid time frame to avoid bot attack against it.

As for the logic on the registration page, they have a solid point it’s the same case being made for the recovery page. To avoid that you can switch to usernames instead or not confirm any email account exists until some sort of 2FA has been enabled and authorized.

Regardless, we live in a world where if they wanted it bad enough, unfortunately they most likely will get the information they want one way or another. Using bots to scrape registration or password recovery pages and finding some way to block that won’t stop them confirming details.

To be quite honest the person being hacked is usually the most vulnerable source. I can just spoof the caller ID call and pretend I’m from your site and manipulate the user that way.

Implement standards, try not to break away with your own code too much (minimizes vulnerabilities) Secure your PII data Secure your databases ensure encryption tokens or any other auth tokens are not stored on a code repo Set password policies and enforce 2FA Validate login attempts to flag unusual behavior

Basics

Sent from my iPhone

On Sep 18, 2019, at 10:40 PM, Laurence Ioannou notifications@github.com wrote:

 Take a dating website as an example. Is it OK at any friend of mine could go a a dating website and check to see if I was a user?

This is the exact reason why your idea does not work - because I can just go to the same dating website and try to sign you up. If I try to sign you up using your email address, and you already have an account, it wont let me and it'll tell me you already exist.

So then I know you have an account. So the information is leaked either way.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

m0lmk commented 4 years ago

@laurencei I must not be articulating the process in a way clear way. If implemented correctly, the end user should only ever see a message saying something along the lines of "An email has been sent with further instructions" when they register or reset a password. They could enter any email they like and the message would always be the same regardless if there was an account on the system or not. The content of the email will contain the information about if the account is valid or not so only the owner of the email account can see that information.

@johnpaulmedina I think you're correct. I will just modify the registration and recovery process to suit my needs and ensure that no PII is being leaked. I would consider an email address PII so that's why I see the need to secure the processes and stop leaking valid email addresses.

Thanks all for the feedback.

rambo666 commented 4 years ago

Why shouldnt we use captchas. I think captcha can help us prevent bot attack and make people tired of applying captchas

janzankowski commented 3 years ago

Note that displaying the same message for "link sent" and "invalid user" is not enough to fix this problem.

(1) Laravel also has forgot password email throttling mechanism, which prevents resending the email within X seconds from last attempt, and displays a suitable error message. But: this throttling intervenes only when the email was actually sent. So, an attacker could still enumerate existing users by trying to send the reset email twice for each address. A throttling error on the second attempt would mean the user exists.

(2) Let's not forget about timing attacks - in my system it takes a noticeable fraction of a second to send an email via our transactional email provider.

In my application, I prevented both of these as well as the original invalid user problem - as follows. First, I overrode the method Illuminate\Foundation\Auth\SendsPasswordResetEmails::sendResetLinkEmail like this:

public function sendResetLinkEmail(Request $request)
{
    $this->validateEmail($request);

    // We will send the password reset link to this user. Once we have attempted
    // to send the link, we will examine the response then see the message we
    // need to show to the user. Finally, we'll send out a proper response.
    $response = $this->broker()->sendResetLink(
        $this->credentials($request)
    );

    // Prevent timing attack to enumerate users.
    usleep(500000 + random_int(0, 1500000));

    // Treat all three cases 'invalid user', 'reset link sent', and 'reset link not sent due to throttling'
    // the same, with a suitable message, again, to prevent user enumeration attack.
    return in_array($response, [Password::INVALID_USER, Password::RESET_LINK_SENT, Password::RESET_THROTTLED])
        ? $this->sendResetLinkResponse($request, Password::RESET_LINK_SENT)
        : $this->sendResetLinkFailedResponse($request, $response);
}

Second, I changed the "reset link sent" message in resources/lang/en/passwords.php to this: If such a user exists in our system, they will receive a password reset link email within a few minutes.

piotrknapik commented 3 years ago

@janzankowski nice one! I added reCaptcha validation line $request->validate(['g-recaptcha-response' => 'required|recaptcha']); but your solution works like a charm - thanks!

My site just went through a security review and that was pointed out as one of the vulnerabilities.

barryvdh commented 3 years ago

FYI, Symfony disclosed a similar issue as CVE issue: https://symfony.com/blog/cve-2021-21424-prevent-user-enumeration-in-authentication-mechanisms