From 05ab82f4763157721274ccb257731bc7f9fde57e Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 13 May 2026 15:47:38 -0400 Subject: [PATCH] ext/openssl: Defer pkcs7/cms verify output writes until verification 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). --- ext/openssl/openssl.c | 89 +++++++++++++------ ...enssl_pkcs7_verify_failed_no_truncate.phpt | 57 ++++++++++++ 2 files changed, 118 insertions(+), 28 deletions(-) create mode 100644 ext/openssl/tests/openssl_pkcs7_verify_failed_no_truncate.phpt diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 6d179cebabda..3a652aad06c4 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -5750,19 +5750,11 @@ PHP_FUNCTION(openssl_pkcs7_verify) goto clean_exit; } if (datafilename) { - dataout = php_openssl_bio_new_file( - datafilename, datafilename_len, 6, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY)); + dataout = BIO_new(BIO_s_mem()); if (dataout == NULL) { goto clean_exit; } } - if (p7bfilename) { - p7bout = php_openssl_bio_new_file( - p7bfilename, p7bfilename_len, 7, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY)); - if (p7bout == NULL) { - goto clean_exit; - } - } #if DEBUG_SMIME zend_printf("Calling PKCS7 verify\n"); #endif @@ -5771,6 +5763,24 @@ PHP_FUNCTION(openssl_pkcs7_verify) RETVAL_TRUE; + if (datafilename) { + BIO *fileout = php_openssl_bio_new_file( + datafilename, datafilename_len, 6, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY)); + if (fileout) { + char *buf; + long buf_len = BIO_get_mem_data(dataout, &buf); + if (buf_len > 0 && BIO_write(fileout, buf, (int)buf_len) != buf_len) { + php_openssl_store_errors(); + RETVAL_LONG(-1); + php_error_docref(NULL, E_WARNING, "Failed to write verified data to %s", datafilename); + } + BIO_free(fileout); + } else { + php_error_docref(NULL, E_WARNING, "Signature OK, but cannot open %s for writing", datafilename); + RETVAL_LONG(-1); + } + } + if (signersfilename) { BIO *certout; @@ -5801,11 +5811,18 @@ PHP_FUNCTION(openssl_pkcs7_verify) RETVAL_LONG(-1); } - if (p7bout) { - if (PEM_write_bio_PKCS7(p7bout, p7) == 0) { - php_error_docref(NULL, E_WARNING, "Failed to write PKCS7 to file"); - php_openssl_store_errors(); - RETVAL_FALSE; + if (p7bfilename) { + p7bout = php_openssl_bio_new_file( + p7bfilename, p7bfilename_len, 7, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY)); + if (p7bout) { + if (PEM_write_bio_PKCS7(p7bout, p7) == 0) { + php_error_docref(NULL, E_WARNING, "Failed to write PKCS7 to file"); + php_openssl_store_errors(); + RETVAL_FALSE; + } + } else { + php_error_docref(NULL, E_WARNING, "Signature OK, but cannot open %s for writing", p7bfilename); + RETVAL_LONG(-1); } } } @@ -6362,26 +6379,35 @@ PHP_FUNCTION(openssl_cms_verify) } if (datafilename) { - dataout = php_openssl_bio_new_file( - datafilename, datafilename_len, 6, PHP_OPENSSL_BIO_MODE_W(CMS_BINARY)); + dataout = BIO_new(BIO_s_mem()); if (dataout == NULL) { goto clean_exit; } } - - if (p7bfilename) { - p7bout = php_openssl_bio_new_file( - p7bfilename, p7bfilename_len, 7, PHP_OPENSSL_BIO_MODE_W(CMS_BINARY)); - if (p7bout == NULL) { - goto clean_exit; - } - } #if DEBUG_SMIME zend_printf("Calling CMS verify\n"); #endif if (CMS_verify(cms, others, store, datain, dataout, (unsigned int)flags)) { RETVAL_TRUE; + if (datafilename) { + BIO *fileout = php_openssl_bio_new_file( + datafilename, datafilename_len, 6, PHP_OPENSSL_BIO_MODE_W(CMS_BINARY)); + if (fileout) { + char *buf; + long buf_len = BIO_get_mem_data(dataout, &buf); + if (buf_len > 0 && BIO_write(fileout, buf, (int)buf_len) != buf_len) { + php_openssl_store_errors(); + RETVAL_FALSE; + php_error_docref(NULL, E_WARNING, "Failed to write verified data to %s", datafilename); + } + BIO_free(fileout); + } else { + php_error_docref(NULL, E_WARNING, "Signature OK, but cannot open %s for writing", datafilename); + RETVAL_FALSE; + } + } + if (signersfilename) { certout = php_openssl_bio_new_file( signersfilename, signersfilename_len, 3, PHP_OPENSSL_BIO_MODE_W(CMS_BINARY)); @@ -6408,10 +6434,17 @@ PHP_FUNCTION(openssl_cms_verify) RETVAL_FALSE; } - if (p7bout) { - if (PEM_write_bio_CMS(p7bout, cms) == 0) { - php_error_docref(NULL, E_WARNING, "Failed to write CMS to file"); - php_openssl_store_errors(); + if (p7bfilename) { + p7bout = php_openssl_bio_new_file( + p7bfilename, p7bfilename_len, 7, PHP_OPENSSL_BIO_MODE_W(CMS_BINARY)); + if (p7bout) { + if (PEM_write_bio_CMS(p7bout, cms) == 0) { + php_error_docref(NULL, E_WARNING, "Failed to write CMS to file"); + php_openssl_store_errors(); + RETVAL_FALSE; + } + } else { + php_error_docref(NULL, E_WARNING, "Signature OK, but cannot open %s for writing", p7bfilename); RETVAL_FALSE; } } diff --git a/ext/openssl/tests/openssl_pkcs7_verify_failed_no_truncate.phpt b/ext/openssl/tests/openssl_pkcs7_verify_failed_no_truncate.phpt new file mode 100644 index 000000000000..d6ae523fe3a4 --- /dev/null +++ b/ext/openssl/tests/openssl_pkcs7_verify_failed_no_truncate.phpt @@ -0,0 +1,57 @@ +--TEST-- +openssl_pkcs7_verify does not truncate output files on verification failure +--EXTENSIONS-- +openssl +--FILE-- + 2048, 'private_key_type' => OPENSSL_KEYTYPE_RSA]); + $csr = openssl_csr_new(['commonName' => $cn], $pk); + $crt = openssl_csr_sign($csr, null, $pk, 1); + openssl_x509_export($crt, $crtPem); + openssl_pkey_export($pk, $pkPem); + file_put_contents($certFile, $crtPem); + file_put_contents($keyFile, $pkPem); +} + +mkpair('signer-A', $aCrt, $aKey); +mkpair('untrusted-B', $bCrt, $dir . 'pkcs7_verify_nt_b.key.tmp'); + +file_put_contents($plain, "hello\n"); +if (!openssl_pkcs7_sign($plain, $signed, "file://$aCrt", "file://$aKey", [], 0)) { + echo "sign failed\n"; + exit(1); +} + +$sentinel = "DO-NOT-OVERWRITE\n"; +file_put_contents($content, $sentinel); +file_put_contents($p7bout, $sentinel); +file_put_contents($signers, $sentinel); + +// Verify against a CA that does NOT include the signer. Chain validation fails. +$r = @openssl_pkcs7_verify($signed, 0, $signers, [$bCrt], null, $content, $p7bout); + +echo "verify result: " . var_export($r, true) . "\n"; +echo "content sentinel intact? " . (file_get_contents($content) === $sentinel ? "YES" : "NO") . "\n"; +echo "p7bout sentinel intact? " . (file_get_contents($p7bout) === $sentinel ? "YES" : "NO") . "\n"; +echo "signers sentinel intact? " . (file_get_contents($signers) === $sentinel ? "YES" : "NO") . "\n"; + +foreach ([$plain, $signed, $aCrt, $aKey, $bCrt, $dir . 'pkcs7_verify_nt_b.key.tmp', $content, $p7bout, $signers] as $f) { + @unlink($f); +} +?> +--EXPECTF-- +verify result: false +content sentinel intact? YES +p7bout sentinel intact? YES +signers sentinel intact? YES