Skip to content

[Security] Ignore empty username or password login attempts #59079

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

Closed
wants to merge 1 commit into from

Conversation

bobvandevijver
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes (CVE)
New feature? no
Deprecations? no
Issues Partially fix #59077
License MIT

Backport of #53851 & #57378 to fix CVE-2024-36611. Note that it was originally merged as a new feature, which means it is noted in the changelog of 7.1 (with the wrong exception). Another PR is incoming to remove it from the changelog.

@carsonbot
Copy link

Hey!

Oh no, it looks like you have made this PR towards a branch that is not maintained anymore. :/
Could you update the PR base branch to target one of these branches instead? 6.4, 7.1, 7.2, 7.3.

Cheers!

Carsonbot

@bobvandevijver
Copy link
Contributor Author

Test errors seem to be unrelated.

@derrabus
Copy link
Member

derrabus commented Dec 3, 2024

See the discussion on #59077. We do not consider this change a security fix and dispute the CVE in question which has not been filed by us.

@derrabus derrabus closed this Dec 3, 2024
fabpot added a commit that referenced this pull request Dec 6, 2024
… item (bobvandevijver)

This PR was merged into the 7.1 branch.

Discussion
----------

[Security] Update incorrect form authenticator changelog item

| Q             | A
| ------------- | ---
| Branch?       | 7.1 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | # <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Related to #53851, #57378 & #59079. ~~Whether or not this an actual CVE, I believe this should be removed from the changelog anyways as it does not throw a bad request anymore.~~

~~If we do keep considering it a new feature, it should probably be changed to reflect the correct exception.~~

As discussed, now only an update to note the actual exception being thrown.

Commits
-------

38f8ec2 Fix change log to mentioned thrown exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants