ext/openssl: Defer pkcs7/cms verify output writes until verification succeeds#65
Open
iliaal wants to merge 1 commit into
Open
ext/openssl: Defer pkcs7/cms verify output writes until verification succeeds#65iliaal wants to merge 1 commit into
iliaal wants to merge 1 commit into
Conversation
…succeeds openssl_pkcs7_verify() and openssl_cms_verify() take a $content output path (6th positional) and a $output_filename p7b path (7th). Both were opened via php_openssl_bio_new_file() in write mode before the verify call, which truncated the destination files on open. When verify failed, the caller's pre-existing content at those paths was destroyed and replaced with nothing. Reachable from any workflow that points $content at a long-lived destination (mail-decode pipeline, signed-message inbox, log file) and processes an attacker-supplied envelope: every failed verify wipes the destination. Replace the dataout file BIO with an in-memory BIO during the verify call. On success, open the destination file and copy the verified bytes out of the memory BIO into it. On failure, return without touching the file. Defer the p7bout file open until inside the success branch, after the signers section, so the file is only created and truncated when the PKCS7/CMS structure is actually going to be written. The memory-BIO buffering is bounded by the size of the verified content, which is typically small for PKCS#7 / CMS envelopes (kB range).
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
1 participant
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.
openssl_pkcs7_verify()andopenssl_cms_verify()take a$contentoutput path (6th positional) and a$output_filenamep7b path (7th). Both were opened viaphp_openssl_bio_new_file()in write mode before the verify call, which truncated the destination files on open. When verify failed, the caller's pre-existing content at those paths was destroyed and replaced with nothing. Reachable from any workflow that points$contentat a long-lived destination (mail-decode pipeline, signed-message inbox, log file) and processes an attacker-supplied envelope.Replace the
dataoutfile BIO with an in-memory BIO during the verify call. On success, open the destination file and copy the verified bytes out of the memory BIO into it. On failure, return without touching the file. Defer thep7boutfile open until inside the success branch, after the signers section, so the file is only created and truncated when the PKCS7/CMS structure is actually going to be written.The memory-BIO buffering is bounded by the size of the verified content, which is typically small for PKCS#7 / CMS envelopes.
Regression test exercises the failure shape with a real signed message and a CA bundle that does not include the signer.