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

[11.x] Prevent the max password length validator exceeding bcrypt limit #52269

Closed
wants to merge 1 commit into from

Conversation

Synchro
Copy link
Contributor

@Synchro Synchro commented Jul 25, 2024

The \Illuminate\Validation\Rules\Password::max method sets the $max property of the Password validator to any number. The bcrypt password hashing algorithm, which Laravel uses by default, cannot handle passwords longer than 72 bytes, and anything submitted beyond that will simply be silently truncated. Thus if a user sets this to a large number (I encountered it in an app which had set it to 500), it will not cause a validation error even if the password is actually unusable.

This limit does not apply to the newer argon and argon2id hashing functions.

This change sets a simple rule that if the requested size is bigger than 72, and the selected password hashing driver is bcrypt, it sets max to 72 so that even if user code sets it higher, it will still cause correct validation errors.

Given the way that the min value in the same class is handled, it might also be appropriate to ensure that max > min, but I wanted to limit this PR to just a single concern.

@Synchro Synchro changed the title Prevent the max password length validator exceeding bcrypt limit [11.x] Prevent the max password length validator exceeding bcrypt limit Jul 25, 2024
@taylorotwell
Copy link
Member

You can set it to 72 in your own application if you wish.

@Synchro
Copy link
Contributor Author

Synchro commented Jul 26, 2024

Of course you can, but I'm curious why you would deliberately choose to allow it to be broken by default. The same validator defaults to 8 chars as a minimum length – why do that if framework users can set that to what they wish too? Isn't this exactly the kind of thing that frameworks are meant to take care of?

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

Successfully merging this pull request may close these issues.

2 participants