From 43e37d7c1e7b345f22d30f10e9b08248f94f1a43 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() opened the content and p7b output paths in write mode before verify, emptying existing files when verification failed. Buffer verified content in a memory BIO and write it only on success; open the p7b path only in that same success branch. Sibling audit: both verify functions shared the same early-open pattern; sign/encrypt paths already write after success. --- NEWS | 4 + ext/openssl/openssl.c | 89 +++++++++++++------ ...openssl_cms_verify_failed_no_truncate.phpt | 70 +++++++++++++++ ...enssl_pkcs7_verify_failed_no_truncate.phpt | 70 +++++++++++++++ 4 files changed, 205 insertions(+), 28 deletions(-) create mode 100644 ext/openssl/tests/openssl_cms_verify_failed_no_truncate.phpt create mode 100644 ext/openssl/tests/openssl_pkcs7_verify_failed_no_truncate.phpt diff --git a/NEWS b/NEWS index 3346d38ea898..9f5aebdf0b1c 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ PHP NEWS registrations are freed while still reachable from the cycle collector. (Ilia Alshanetsky) +- OpenSSL: + . Fixed openssl_pkcs7_verify() and openssl_cms_verify() truncating output + files when verification fails. (Ilia Alshanetsky) + 24 Sep 2026, PHP 8.4.26 diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 36d5a7ccb73c..0adee9810b74 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -5763,19 +5763,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 @@ -5784,6 +5776,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; @@ -5814,11 +5824,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); } } } @@ -6375,26 +6392,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)); @@ -6421,10 +6447,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_cms_verify_failed_no_truncate.phpt b/ext/openssl/tests/openssl_cms_verify_failed_no_truncate.phpt new file mode 100644 index 000000000000..0f020a8605bf --- /dev/null +++ b/ext/openssl/tests/openssl_cms_verify_failed_no_truncate.phpt @@ -0,0 +1,70 @@ +--TEST-- +openssl_cms_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 . 'cms_verify_nt_b.key.tmp'); + +file_put_contents($plain, "hello\n"); +if (!openssl_cms_sign($plain, $signed, "file://$aCrt", "file://$aKey", [])) { + 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_cms_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"; +?> +--CLEAN-- + +--EXPECT-- +verify result: false +content sentinel intact? YES +p7bout sentinel intact? YES +signers sentinel intact? YES 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..55b8cfc11422 --- /dev/null +++ b/ext/openssl/tests/openssl_pkcs7_verify_failed_no_truncate.phpt @@ -0,0 +1,70 @@ +--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"; +?> +--CLEAN-- + +--EXPECT-- +verify result: false +content sentinel intact? YES +p7bout sentinel intact? YES +signers sentinel intact? YES