From 74720a22f332cb09435405d50c312413c86af45e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 29 Mar 2025 23:36:57 +0100 Subject: [PATCH 1/4] Fix memory leak in openssl_sign() when passing invalid algorithm Closes GH-18185. --- NEWS | 4 ++++ ext/openssl/openssl.c | 1 + .../tests/openssl_sign_invalid_algorithm.phpt | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 ext/openssl/tests/openssl_sign_invalid_algorithm.phpt diff --git a/NEWS b/NEWS index 335903eee23c0..188c5a877968c 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ PHP NEWS . Fixed imagecrop() overflow with rect argument with x/width y/heigh usage in gdImageCrop(). (David Carlier) +- OpenSSL: + . Fix memory leak in openssl_sign() when passing invalid algorithm. + (nielsdos) + - Standard: . Fixed bug GH-18145 (php8ts crashes in php_clear_stat_cache()). (Jakub Zelenka) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index c5720ee97e38c..bd386dbe8ae7a 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -6959,6 +6959,7 @@ PHP_FUNCTION(openssl_sign) mdtype = php_openssl_get_evp_md_from_algo(method_long); } if (!mdtype) { + EVP_PKEY_free(pkey); php_error_docref(NULL, E_WARNING, "Unknown digest algorithm"); RETURN_FALSE; } diff --git a/ext/openssl/tests/openssl_sign_invalid_algorithm.phpt b/ext/openssl/tests/openssl_sign_invalid_algorithm.phpt new file mode 100644 index 0000000000000..c669a373a1079 --- /dev/null +++ b/ext/openssl/tests/openssl_sign_invalid_algorithm.phpt @@ -0,0 +1,18 @@ +--TEST-- +openssl_sign: invalid algorithm +--EXTENSIONS-- +openssl +--FILE-- + +--EXPECTF-- +Warning: openssl_sign(): Unknown digest algorithm in %s on line %d From 0dc600c69aaf90361775c2d9c83094e324fae3d5 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 30 Mar 2025 23:51:58 +0200 Subject: [PATCH 2/4] Fix openssl_random_pseudo_bytes() always setting strong_result to true This regressed in 62c7432f, prior to that commit the value was set to false in case random number generation failed, but now even if an exception is thrown it is set to true. This likely does not _really_ matter as the user will handle the exception, still the value in $strong_result is observable. --- ext/openssl/openssl.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index bd386dbe8ae7a..9eb052335b8d5 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -7961,17 +7961,15 @@ PHP_FUNCTION(openssl_random_pseudo_bytes) RETURN_THROWS(); } - if (zstrong_result_returned) { - ZEND_TRY_ASSIGN_REF_FALSE(zstrong_result_returned); - } - if ((buffer = php_openssl_random_pseudo_bytes(buffer_length))) { ZSTR_VAL(buffer)[buffer_length] = 0; RETVAL_NEW_STR(buffer); - } - if (zstrong_result_returned) { - ZEND_TRY_ASSIGN_REF_TRUE(zstrong_result_returned); + if (zstrong_result_returned) { + ZEND_TRY_ASSIGN_REF_TRUE(zstrong_result_returned); + } + } else if (zstrong_result_returned) { + ZEND_TRY_ASSIGN_REF_FALSE(zstrong_result_returned); } } /* }}} */ From 5e68671f88ef92f0612bfa3e2819e5107e624418 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 30 Mar 2025 23:54:56 +0200 Subject: [PATCH 3/4] Fix inverted call to php_openssl_store_errors() This calls php_openssl_store_errors() in the success path right now, change it to call php_openssl_store_errors() in the error path. --- ext/openssl/openssl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 9eb052335b8d5..bfbf51ff6442c 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -7940,11 +7940,10 @@ PHP_OPENSSL_API zend_string* php_openssl_random_pseudo_bytes(zend_long buffer_le PHP_OPENSSL_CHECK_LONG_TO_INT_NULL_RETURN(buffer_length, length); PHP_OPENSSL_RAND_ADD_TIME(); if (RAND_bytes((unsigned char*)ZSTR_VAL(buffer), (int)buffer_length) <= 0) { + php_openssl_store_errors(); zend_string_release_ex(buffer, 0); zend_throw_exception(zend_ce_exception, "Error reading from source device", 0); return NULL; - } else { - php_openssl_store_errors(); } return buffer; From 8a1f6711bf75da789fbc23a63629d16a1925e252 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 1 Apr 2025 22:54:17 +0200 Subject: [PATCH 4/4] Fix resource leak in iptcembed() on error Closes GH-18225. --- NEWS | 1 + ext/standard/iptc.c | 1 + 2 files changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 188c5a877968c..92b199270cf4d 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,7 @@ PHP NEWS . Fixed bug GH-18209 (Use-after-free in extract() with EXTR_REFS). (ilutov) . Fixed bug GH-18212 (fseek with SEEK_CUR whence value and negative offset leads to negative stream position). (David Carlier) + . Fix resource leak in iptcembed() on error. (nielsdos) 10 Apr 2025, PHP 8.3.20 diff --git a/ext/standard/iptc.c b/ext/standard/iptc.c index e4dd38637570a..44dd33bab10ac 100644 --- a/ext/standard/iptc.c +++ b/ext/standard/iptc.c @@ -204,6 +204,7 @@ PHP_FUNCTION(iptcembed) if (spool < 2) { if (zend_fstat(fileno(fp), &sb) != 0) { + fclose(fp); RETURN_FALSE; }