phar: free EVP_PKEY on EVP_MD_CTX_create/EVP_VerifyInit failure in phar_verify_signature#21801
Closed
iliaal wants to merge 1 commit intophp:PHP-8.4from
Closed
phar: free EVP_PKEY on EVP_MD_CTX_create/EVP_VerifyInit failure in phar_verify_signature#21801iliaal wants to merge 1 commit intophp:PHP-8.4from
iliaal wants to merge 1 commit intophp:PHP-8.4from
Conversation
…ar_verify_signature In phar_verify_signature, PEM_read_bio_PUBKEY allocates EVP_PKEY *key. When EVP_MD_CTX_create() returns NULL or EVP_VerifyInit() fails, the early return freed md_ctx but not key, leaking the object in OpenSSL's heap. The existing failure: label already calls both EVP_PKEY_free and EVP_MD_CTX_destroy. This adds EVP_PKEY_free(key) to the earlier exit branch, mirroring the symmetric fix in phar_create_signature from phpGH-19563 (f5a3a64). ASAN catches the leak when EVP_VerifyInit fails, typically in FIPS-mode deployments with SHA-1 disabled.
Contributor
Author
|
The WINDOWS_X64_ZTS failure seems like a CI/CD error, tests/compilation doesn't even run. Looks like something with firebird |
Member
|
Next time, ask your agent to look for a duplicate first: #21750 |
Contributor
Author
|
My bad, was looking at phar as a whole, didn't see that one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
phar_verify_signature,PEM_read_bio_PUBKEYallocatesEVP_PKEY *key. WhenEVP_MD_CTX_create()returns NULL orEVP_VerifyInit()fails, the early return freedmd_ctxbut did not callEVP_PKEY_free(key), leaking the object in OpenSSL's heap.The existing
failure:label (reached on bad signature orEVP_VerifyUpdateerrors) already calls bothEVP_PKEY_freeandEVP_MD_CTX_destroy. The fix addsEVP_PKEY_free(key)to the earlier exit branch, mirroring the symmetric fix inphar_create_signaturefrom GH-19563 (f5a3a64).ASAN catches the leak when
EVP_VerifyInitfails, in FIPS-mode deployments with SHA-1 disabled.