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

Base64::decodeUrlSafe() validation is inconsistent #257

Closed
TimWolla opened this issue Aug 31, 2022 · 5 comments · Fixed by #259
Closed

Base64::decodeUrlSafe() validation is inconsistent #257

TimWolla opened this issue Aug 31, 2022 · 5 comments · Fixed by #259
Assignees
Labels
dependencies Pull requests that update a dependency file
Milestone

Comments

@TimWolla
Copy link
Contributor

In Webauthn\Util\Base64::decodeUrlSafe() the Assertion at the beginning rejects = (i.e. the padding character), but when decoding with the constant time decoder the ::decode() method instead of ::decodeNoPadding() is used. This is inconsistent and requires the decoder to do extra work.

@mayestik1
Copy link

mayestik1 commented Aug 31, 2022

I am facing the issue "Invalid Base 64 Url Safe character"
[stacktrace] #0 vendor/beberlei/assert/lib/Assert/Assertion.php(779): Assert\\Assertion::createException() #1 vendor/web-auth/webauthn-lib/src/Util/Base64.php(16): Assert\\Assertion::regex()

Any fixes please?

@Spomky Spomky added this to the 5.0.0 milestone Aug 31, 2022
@Spomky
Copy link
Contributor

Spomky commented Aug 31, 2022

Hi @TimWolla,

This is correct, the method Webauthn\Util\Base64::decodeUrlSafe() could be avoided in favor of a direct call to ParagonIE\ConstantTime\Base64::decodeNoPadding().
The framework do it this way because when I started to use this dependency, the available version was 2.4 and this method did not exist (was introduced in 2.6).
I try to keep things working and to avoid BC breaks, I rarely change the dependencies for the same major release. That is why I won't change that for now, but this will be done for the next major version. I will keep track of it for 5.0.0.

Regards.

@Spomky Spomky added the dependencies Pull requests that update a dependency file label Aug 31, 2022
@Spomky Spomky self-assigned this Aug 31, 2022
@TimWolla
Copy link
Contributor Author

Thanks for the explanation, makes sense.

@Spomky
Copy link
Contributor

Spomky commented Aug 31, 2022

@mayestik1

Can you elaborate more on this? What is the input data?

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants