-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Fail on empty password verification (without warning on any implementation) #35497
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
Conversation
Can you add the same test case on the SodiumPasswordEncoder? |
My description could have been better :-) - without this PR, the call to As for the target branch: The release info on https://symfony.com/releases/4.3 already marks 4.3 as security-fixes-only. As this is not a security fix, I targetted 4.4 - which one is correct? |
There will be a last 4.3 in a few days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(for 4.4 now that 4.3 is closed)
@@ -73,6 +73,9 @@ public function encodePassword($raw, $salt) | |||
*/ | |||
public function isPasswordValid($encoded, $raw, $salt) | |||
{ | |||
if ('' === $raw) { | |||
return false; | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be a blank line after this one, same below
Thank you @umulmrum. |
…y implementation) (Stefan Kruppa) This PR was submitted for the 4.3 branch but it was merged into the 4.4 branch instead (closes #35497). Discussion ---------- Fail on empty password verification (without warning on any implementation) | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | sort of | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | When using the sodium extension, an empty $raw string will issue a warning during validation, but the standard `password_verify()` does not. This PR aims to provide identical behavior independent of the underlying implementation. Two assumptions were made (please doublecheck if they are correct): - Empty password is never valid. - Empty password is not that severe that anybody needs to be informed using a warning or exception. Commits ------- 4d920f0 Fail on empty password verification (without warning on any implementation)
When using the sodium extension, an empty $raw string will issue a warning during validation, but the standard
password_verify()
does not. This PR aims to provide identical behavior independent of the underlying implementation. Two assumptions were made (please doublecheck if they are correct):