Skip to content

ext/mysqlnd/mysqlnd_auth.c: Add error handling for invalid public key… #18663

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

Merged

Conversation

andr3y-k0v4l3v
Copy link
Contributor

… size

Reported-by: Pavel Nekrasov p.nekrasov@fobos-nt.ru

… size

Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
Signed-off-by: Andrey Kovalev <ded@altlinux.org>
@andr3y-k0v4l3v andr3y-k0v4l3v force-pushed the fix-key-handling-ext-mysqlnd-mysqlnd_auth branch from d0018f8 to 2054da6 Compare June 2, 2025 07:34
Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify this is very unlikely to ever happen - it would have to be some buggy engine / provider maybe but even then I'm not sure if it's possible to fail on size here. The key is read from PEM using PEM_read_bio_PUBKEY so it should always provide a key that returns size. Anyway it won't probably hurt to do the check but no backport needed for this.


int pkey_size = EVP_PKEY_size(server_public_key);

if (pkey_size <= 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's never negative

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not issue to keep it like that if API ever changed but just to be clear that this will never overflow with the current OpenSSL versions. If it was 0, it would also fail on password check.

@bukka bukka merged commit b871261 into php:master Jun 2, 2025
8 of 9 checks passed
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.

4 participants