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

Discovery attack on login form #9374

Closed
Berbe opened this issue Nov 27, 2018 · 16 comments
Closed

Discovery attack on login form #9374

Berbe opened this issue Nov 27, 2018 · 16 comments
Labels
partially a bug Architecture or design-imposed shortcomings

Comments

@Berbe
Copy link

Berbe commented Nov 27, 2018

Expected behaviour

When attempting to login, the use of bad credentials should provide a single and consistent error message saying credentials are invalid, without finer granularity.

Actual behaviour

When using an incorrect email address, the login form actually states the email is wrong, instead of the credentials pair.
That means email enumeration of registered users is possible ans consists itself in a form of discovery attack.

Steps to reproduce the problem

  1. Reach the login form of any mastodon instance
  2. Try to login with non-registered email address

Specifications

The instance that behavior has been noticed on was v2.6.1

@Gargron
Copy link
Member

Gargron commented Nov 27, 2018

Which language? English or another? The messages should be identical

@mkljczk
Copy link
Contributor

mkljczk commented Nov 27, 2018

@Berbe
Copy link
Author

Berbe commented Nov 27, 2018

Wow that is unexpected: how are translations able to tell one case (non-existent email) from the other (invalid credentials)?
Sounds like a design flaw.

Thanks for helping spot the problem anyway!

@nightpool
Copy link
Member

ehhhhh either we should say that this is an issue and fix it in the code, not the translations, or we should just ignore it. I find discovery attacks to be pretty overblown.

@Berbe
Copy link
Author

Berbe commented Nov 27, 2018

I'll set the personal opinion aside which, while dangerous, iss ill-placed in a bug report.

It is definitely a code bug to differentiate thoses error cases in any way, thus allowing to handle them differently in the translation engine. Would you be able to help spot where the problem comes from in the source code?
However, as a quick fix, I'd suggest to set the translated error message strictly identical in both cases for now.

@Gargron Gargron reopened this Nov 27, 2018
@Gargron
Copy link
Member

Gargron commented Nov 27, 2018

I've looked at the Devise code. If paranoid mode is active, it never uses the not_found_in_database locale. Paranoid mode is active for us:

https://github.com/tootsuite/mastodon/blob/master/config/initializers/devise.rb#L154

So, what the hell?

@nightpool
Copy link
Member

@Berbe ...... having an opinion is dangerous? having an opinion on the priority/validity of a bug report is ill-placed? that's the entire point of triage.

@Berbe
Copy link
Author

Berbe commented Nov 29, 2018

@nightpool Stop claiming your words for my own; Quibbling stops here & now.

@Gargron If I understand well, does that mean no Mastodon instance would be able to differentiate the error cases, ever?

@mkljczk
Copy link
Contributor

mkljczk commented Nov 29, 2018

What’s the instance where you encounter this problem? Maybe it has some custom changes related to authentication (for example to make sign in with username possible)

@Gargron
Copy link
Member

Gargron commented Nov 29, 2018

I am still confused because @m4sk1n you were able to reproduce the issue, despite Devise.paranoid being set to true?

@Berbe
Copy link
Author

Berbe commented Nov 29, 2018

I noticed that behavior on https://mastodon.xyz/

@mkljczk
Copy link
Contributor

mkljczk commented Nov 29, 2018

I noticed it on glitch.social (not dev. one), I think it has no auth-specific changes

@Gargron
Copy link
Member

Gargron commented Nov 29, 2018

I have identified the issue. The devise-two-factor gem ignores the paranoid setting in its two_factor_authentication strategy and always throws "not_found_in_database". However, if the user record does exist, execution moves to the standard devise_authenticatable strategy, which correctly always throws "invalid" when paranoid mode is activated.

This would have been a larger issue if the keys weren't identical in almost all translations.

@Gargron
Copy link
Member

Gargron commented Nov 29, 2018

Submitted fix: devise-two-factor/devise-two-factor#138

@Berbe
Copy link
Author

Berbe commented Nov 30, 2018

Great catch!

Thanks a lot, crossing my fingers for upstream correction and integration of their new version into mastodon ASAP.

@Gargron Gargron added the partially a bug Architecture or design-imposed shortcomings label May 1, 2019
@trwnh
Copy link
Member

trwnh commented May 14, 2023

fixed in upstream pr, i assume. if this is still happening then please comment here so it can be reopened

@trwnh trwnh closed this as completed May 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
partially a bug Architecture or design-imposed shortcomings
Projects
None yet
Development

No branches or pull requests

5 participants