Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedJan 28, 2020
Can you add the same test case on the SodiumPasswordEncoder? |
umulmrum commentedJan 28, 2020
My description could have been better :-) - without this PR, the call to As for the target branch: The release info onhttps://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? |
nicolas-grekas commentedJan 28, 2020
There will be a last 4.3 in a few days. |
nicolas-grekas left a comment
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)
| { | ||
| if ('' ===$raw) { | ||
| returnfalse; | ||
| } |
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
fabpot commentedFeb 3, 2020
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)
Uh oh!
There was an error while loading.Please reload this page.
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):