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

Potentially the empty token could be treated as signed #1265

Closed
SCIF opened this issue Dec 27, 2024 · 4 comments
Closed

Potentially the empty token could be treated as signed #1265

SCIF opened this issue Dec 27, 2024 · 4 comments

Comments

@SCIF
Copy link

SCIF commented Dec 27, 2024

This is unlikely a security vulnerability but rather bad coding: https://github.com/lexik/LexikJWTAuthenticationBundle/blob/3.x/Services/JWSProvider/LcobucciJWSProvider.php#L103-L106

        $e = $token = null;
        try {
            $token = $this->getSignedToken($jws);
        } catch (\InvalidArgumentException) {
        }

        return new CreatedJWS((string) $token, null === $e);

In case InvalidArgumentException was thrown the token will be casted to '' and second argument will be true so the empty token will be considered as properly formed and signed.

@SCIF
Copy link
Author

SCIF commented Dec 27, 2024

I assume the original idea was to override $e on line 103:

} catch (\InvalidArgumentException $e) {

@chalasr
Copy link
Collaborator

chalasr commented Dec 27, 2024

Yes, that’s a a regular bug

@SCIF
Copy link
Author

SCIF commented Dec 28, 2024

Hi @chalasr , thanks for the confirmation! Any idea why InvalidArgumentException could be ignored? To me this looks like a bad practice

@chalasr
Copy link
Collaborator

chalasr commented Jan 6, 2025

It was a bad move to uncapturing catches. Fixed and simplified in #1267

@chalasr chalasr closed this as completed Jan 6, 2025
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

No branches or pull requests

2 participants