Skip to content

[Security][LoginLink] Invalid parameters throw 500 #60347

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
davidszkiba opened this issue May 5, 2025 · 2 comments
Closed

[Security][LoginLink] Invalid parameters throw 500 #60347

davidszkiba opened this issue May 5, 2025 · 2 comments

Comments

@davidszkiba
Copy link
Contributor

Symfony version(s) affected

7.2.4

Description

When login_check is requested and the request parameters have the wrong type (userIdentifier, hash, expires) it throws 500 exceptions because the acceptSignatureHash method expects the parameters to be of certain types ($userIdentifier a string, $expires an int and $hash a string).

How to reproduce

Implement LoginLink and then request login via link:
https://example.com/login/link_check?expires=%E2%80%AA1747385844%E2%80%AC&hash=somehash&user=%E2%80%AA123456789%E2%80%AC

Here the expires parameter is flanked by zero-width spaces. This leads to a 500 error because expires is expected to be an int but here it is a string. I'm not exactly sure why the zero-width spaces are added (maybe some email clients do that?), but in any case, I think this should not lead to a 500 error.

Possible Solution

Be more strict in the LoginLinkHandler. In addition to checking if the required parameters exist (which is already done after this PR #48292), the type should also be checked.

Additional Context

Symfony\Component\Security\Core\Signature\SignatureHasher::acceptSignatureHash(): Argument #2 ($expires) must be of type int, string given, called in /srv/portal/vendor/symfony/security-http/LoginLink/LoginLinkHandler.php on line 98

@chalasr
Copy link
Member

chalasr commented May 5, 2025

Do you want to work on a PR?

@davidszkiba
Copy link
Contributor Author

Do you want to work on a PR?

Yes, already doing so :)

fabpot added a commit that referenced this issue May 7, 2025
… invalid parameters (davidszkiba)

This PR was merged into the 6.4 branch.

Discussion
----------

[Security][LoginLink] Throw `InvalidLoginLinkException` on invalid parameters

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #60347
| License       | MIT

With this change, in addition to checking the presence of the required request parameters, also the type is checked.

Commits
-------

0dc4d0b [Security][LoginLink] Throw InvalidLoginLinkException on invalid parameters
@fabpot fabpot closed this as completed May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants