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

Potential Security Issues #172

Closed
najdanovicivan opened this issue Mar 25, 2020 · 11 comments
Closed

Potential Security Issues #172

najdanovicivan opened this issue Mar 25, 2020 · 11 comments

Comments

@najdanovicivan
Copy link
Contributor

There are two issues I'm noticing

  1. There are different error messages for invalid username and invalid password
    Screenshot 2020-03-25 at 13 02 21
    Screenshot 2020-03-25 at 13 02 38

This allows bots/exploits to check if username exist easily

  1. Forgot password redirects has different messages for existing and non-existing email addresses. Correct address even redirect to password reset page

Screenshot 2020-03-25 at 13 04 51
Screenshot 2020-03-25 at 13 22 06

This allows bots to check which emails are valid and can be used for many bad things not only for exploiting the login. One example is checking if email is being used when crawling mail lists for spam purposes. This should be reworked so that form always return message in some sort of "You will get an email shortly if the address exist in our system."

@lonnieezell
Copy link
Owner

I will have to look it up again, but I think I recall the 2019 NIST guidelines recommending to give useful message like these in favor or more cryptic messages they previously recommended.

@lonnieezell
Copy link
Owner

And, to be fair, with the 1 billion+ hacked databases having been released freely on the internet, there's a pretty good likelihood they already know the email exists. :)

@Ihabafia
Copy link

You just need to change vendor/myth/auth/src/Authentication/LocalAuthenticator.php Line 158 to $this->error = lang('Auth.badAttempt');

@najdanovicivan
Copy link
Contributor Author

@Ihabafia It’s not the issue how to do it. The issue here is what’s the proper way to be done and if any changes are required to be done those should be merged to the official repository

@Vizzielli
Copy link
Contributor

+1 for the initial post.
Also if attacker know the mail exists, it isn't sure that the same user and email could be used in your website too. It is also a privacy issue. If a person who knows you uses the form to request a reset on a website to understand if you use it or not, in this way you give him/her the information they search for: if that person is subscribed or not. The user could consider that as confidential info.

@michalsn
Copy link
Contributor

michalsn commented Apr 5, 2020

I wouldn't be very concerned about this. User experience is also very important.

  1. You can always try to check if the given username is already "taken" by trying to register.
  2. Thank God. Users have to know what is going on. What if I can't remember which email address I have used for this website?

IMO if someone has specific needs for security, he should edit language files and flow in the controller by himself.

The only thing we can think about here is more advanced throttling. Because right now I don't think we take advantage of it everywhere.

That's my two cents.

@fefo-p
Copy link
Contributor

fefo-p commented Apr 5, 2020

agree with @michalsn and @lonnieezell

@Vizzielli
Copy link
Contributor

I think it is something that should be extended to sign up too, I explain it better:

  • in that case if someone try to sign up using existing email, it should all work as usual, but: the system shouldn't update the login details, just send an email to inform the user about sign up try and give the links to login page or password reset page if he doesnt remember the current one.

It is just an opinion, clearly, considering it is something easy to edit no problem in any case, I'll always chose that direction to protect user privacy as possible.

@najdanovicivan
Copy link
Contributor Author

najdanovicivan commented Apr 5, 2020

So many pros and cons related to this. So maybe the best way will be to have parameter in config so that it can be used in both ways

@lonnieezell
Copy link
Owner

I've been slammed by other stuff lately so haven't had the chance to dig through the pages and pages of acedemia of the current NIST recommendations, but I'm positive when I was going through them previously that user experience won over in the latest recommendations. Since all it requires is changing out a language string, it's simple enough for someone to customize on their own app. I don't think we need any config checks on this.

in that case if someone try to sign up using existing email, it should all work as usual, but: the system shouldn't update the login details, just send an email to inform the user about sign up try and give the links to login page or password reset page if he doesnt remember the current one.

I don't want this in the core, as years of looking at logs tells me there are way too many instances of bots doing misc automated attacks that it would scare off users and make them feel that the app you're building isn't a safe app, when it's irrelevant to the safety built in.

However, I do think adding triggering events at register, login, forgot password, etc makes sense so the app can be extended easily and customized.

@Vizzielli would you care to pull that out into it's own issue?

@bsoliveira
Copy link

I agree with @lonnieezell the more guidance for the user to correct errors the better. About robot crawling and brute force attacks should be an implementation of firewel, where you can discern a number of attempts per time related to an IP. this may be a complementary module in the future.

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

No branches or pull requests

7 participants