From 02c67b47f728f915e6015c2fd52c6e1f7a27b172 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 15 Oct 2025 20:36:16 +0200 Subject: [PATCH 01/17] Fix accessing of overridden private property in get_object_vars() Fixes GH-20177 Closes GH-20182 --- NEWS | 2 ++ Zend/tests/gh20177.phpt | 25 +++++++++++++++++++++++++ Zend/zend_object_handlers.c | 5 +++++ 3 files changed, 32 insertions(+) create mode 100644 Zend/tests/gh20177.phpt diff --git a/NEWS b/NEWS index 4b2a4777fef5..3bc4e05761f8 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ PHP NEWS . Fixed bug GH-20073 (Assertion failure in WeakMap offset operations on reference). (nielsdos) . Fixed bug GH-19844 (Don't bail when closing resources on shutdown). (ilutov) + . Fixed bug GH-20177 (Accessing overridden private property in + get_object_vars() triggers assertion error). (ilutov) - DOM: . Partially fixed bug GH-16317 (DOM classes do not allow diff --git a/Zend/tests/gh20177.phpt b/Zend/tests/gh20177.phpt new file mode 100644 index 000000000000..fd69460067f9 --- /dev/null +++ b/Zend/tests/gh20177.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-20177: Access overridden private property in get_object_vars() +--FILE-- + +--EXPECT-- +array(1) { + ["prop"]=> + string(8) "A::$prop" +} diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 2eacb614f97c..2fe02e222418 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -528,6 +528,11 @@ ZEND_API zend_result zend_check_property_access(const zend_object *zobj, zend_st return FAILURE; } } else { + /* We were looking for a protected property but found a private one + * belonging to the parent class. */ + if (property_info->flags & ZEND_ACC_PRIVATE) { + return FAILURE; + } ZEND_ASSERT(property_info->flags & ZEND_ACC_PROTECTED); } return SUCCESS; From 939b97219d4e4981dcf8b063ca983189328f6a03 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 17 Oct 2025 22:39:25 +0200 Subject: [PATCH 02/17] Improve bug60602.phpt portability On NixOS we need to inherit the PATH variable so we can actually resolve the location of ls. Closes GH-20206 --- ext/standard/tests/streams/bug60602.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/tests/streams/bug60602.phpt b/ext/standard/tests/streams/bug60602.phpt index b97f6f877a42..71df0408e2d7 100644 --- a/ext/standard/tests/streams/bug60602.phpt +++ b/ext/standard/tests/streams/bug60602.phpt @@ -9,7 +9,7 @@ $descs = array( 2 => array('pipe', 'w'), // strerr ); -$environment = array('test' => array(1, 2, 3)); +$environment = array('test' => array(1, 2, 3), 'PATH' => getenv('PATH')); $cmd = (substr(PHP_OS, 0, 3) == 'WIN') ? 'dir' : 'ls'; $p = proc_open($cmd, $descs, $pipes, '.', $environment); From e37a207ddd6fd3440afbfc647e72d5ae10195c85 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 20 Oct 2025 17:25:48 +0200 Subject: [PATCH 03/17] NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 85c4b9ecb222..4ce58de3a362 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,8 @@ PHP NEWS (Arnaud) . Fixed bug GH-20121 (JIT broken in ZTS builds on MacOS 15). (Arnaud, Shivam Mathur) + . Fixed bug GH-19875 (JIT 1205 segfault on large file compiled in subprocess). + (Arnaud) - PgSql: . Fix memory leak when first string conversion fails. (nielsdos) From 43549ea1483d626f81e1643f5915a7a8df9b39a0 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 20 Oct 2025 17:26:27 +0200 Subject: [PATCH 04/17] NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 467cfd318714..51d98a083e04 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,8 @@ PHP NEWS (Arnaud) . Fixed bug GH-20121 (JIT broken in ZTS builds on MacOS 15). (Arnaud, Shivam Mathur) + . Fixed bug GH-19875 (JIT 1205 segfault on large file compiled in subprocess). + (Arnaud) - OpenSSL: . Fixed bug GH-19994 (openssl_get_cipher_methods inconsistent with fetching). From 45a50d0d6889274d1ad4f94750572dc159ef7266 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:31:01 +0200 Subject: [PATCH 05/17] phar: Remove pointless efree() (#20232) `error` is NULL at this point, otherwise we couldn't have gotten and *shouldn't* have gotten here in the first place. --- ext/phar/dirstream.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index 81fa62c854df..71420c11e62d 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -450,7 +450,6 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo if (NULL == zend_hash_add_mem(&phar->manifest, entry.filename, &entry, sizeof(phar_entry_info))) { php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", adding to manifest failed", ZSTR_VAL(entry.filename), phar->fname); - efree(error); zend_string_efree(entry.filename); return 0; } From 40b3f884260b5b3a686d8bba569317be0e2d0e2d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:31:16 +0200 Subject: [PATCH 06/17] phar: Avoid string duplication just for error message, use truncation formatting string (#20229) --- ext/phar/phar_object.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 9c09417e077b..fba332aa21a0 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -4315,10 +4315,7 @@ PHP_METHOD(Phar, extractTo) } if (ZSTR_LEN(path_to) >= MAXPATHLEN) { - char *tmp = estrndup(ZSTR_VAL(path_to), 50); - /* truncate for error message */ - zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Cannot extract to \"%s...\", destination directory is too long for filesystem", tmp); - efree(tmp); + zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Cannot extract to \"%.50s...\", destination directory is too long for filesystem", ZSTR_VAL(path_to)); RETURN_THROWS(); } From 0805953bb0b7b2a7bc646442a49953071556a344 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 22:55:24 +0200 Subject: [PATCH 07/17] phar: Restructure code to get rid of a goto --- ext/phar/zip.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index dff893c221b9..10301fc48e7d 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -234,7 +234,7 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch uint16_t i; phar_archive_data *mydata = NULL; phar_entry_info entry = {0}; - char *p = buf, *ext, *actual_alias = NULL; + char *ext, *actual_alias = NULL; char *metadata = NULL; size = php_stream_tell(fp); @@ -261,7 +261,16 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch return FAILURE; } - if ((p = phar_find_eocd(buf, size)) != NULL) { + char *p = phar_find_eocd(buf, size); + if (!p) { + php_stream_close(fp); + if (error) { + spprintf(error, 4096, "phar error: end of central directory not found in zip-based phar \"%s\"", fname); + } + return FAILURE; + } + + { memcpy((void *)&locator, (void *) p, sizeof(locator)); if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { /* split archives not handled */ @@ -301,18 +310,8 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch } else { ZVAL_UNDEF(&mydata->metadata_tracker.val); } - - goto foundit; - } - - php_stream_close(fp); - - if (error) { - spprintf(error, 4096, "phar error: end of central directory not found in zip-based phar \"%s\"", fname); } - return FAILURE; -foundit: mydata->fname = pestrndup(fname, fname_len, mydata->is_persistent); #ifdef PHP_WIN32 phar_unixify_path_separators(mydata->fname, fname_len); From 0098f3efbce2311027ad558ce4f5eb5f02634101 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 22:55:35 +0200 Subject: [PATCH 08/17] phar: De-indent code --- ext/phar/zip.c | 60 ++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 10301fc48e7d..5f025d75e341 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -270,46 +270,44 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch return FAILURE; } - { - memcpy((void *)&locator, (void *) p, sizeof(locator)); - if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { - /* split archives not handled */ - php_stream_close(fp); - if (error) { - spprintf(error, 4096, "phar error: split archives spanning multiple zips cannot be processed in zip-based phar \"%s\"", fname); - } - return FAILURE; + memcpy((void *)&locator, (void *) p, sizeof(locator)); + if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { + /* split archives not handled */ + php_stream_close(fp); + if (error) { + spprintf(error, 4096, "phar error: split archives spanning multiple zips cannot be processed in zip-based phar \"%s\"", fname); } + return FAILURE; + } - if (PHAR_GET_16(locator.counthere) != PHAR_GET_16(locator.count)) { - if (error) { - spprintf(error, 4096, "phar error: corrupt zip archive, conflicting file count in end of central directory record in zip-based phar \"%s\"", fname); - } - php_stream_close(fp); - return FAILURE; + if (PHAR_GET_16(locator.counthere) != PHAR_GET_16(locator.count)) { + if (error) { + spprintf(error, 4096, "phar error: corrupt zip archive, conflicting file count in end of central directory record in zip-based phar \"%s\"", fname); } + php_stream_close(fp); + return FAILURE; + } - mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist)); - mydata->is_persistent = PHAR_G(persist); + mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist)); + mydata->is_persistent = PHAR_G(persist); - /* read in archive comment, if any */ - if (PHAR_GET_16(locator.comment_len)) { + /* read in archive comment, if any */ + if (PHAR_GET_16(locator.comment_len)) { - metadata = p + sizeof(locator); + metadata = p + sizeof(locator); - if (PHAR_GET_16(locator.comment_len) != size - (metadata - buf)) { - if (error) { - spprintf(error, 4096, "phar error: corrupt zip archive, zip file comment truncated in zip-based phar \"%s\"", fname); - } - php_stream_close(fp); - pefree(mydata, mydata->is_persistent); - return FAILURE; + if (PHAR_GET_16(locator.comment_len) != size - (metadata - buf)) { + if (error) { + spprintf(error, 4096, "phar error: corrupt zip archive, zip file comment truncated in zip-based phar \"%s\"", fname); } - - phar_parse_metadata_lazy(metadata, &mydata->metadata_tracker, PHAR_GET_16(locator.comment_len), mydata->is_persistent); - } else { - ZVAL_UNDEF(&mydata->metadata_tracker.val); + php_stream_close(fp); + pefree(mydata, mydata->is_persistent); + return FAILURE; } + + phar_parse_metadata_lazy(metadata, &mydata->metadata_tracker, PHAR_GET_16(locator.comment_len), mydata->is_persistent); + } else { + ZVAL_UNDEF(&mydata->metadata_tracker.val); } mydata->fname = pestrndup(fname, fname_len, mydata->is_persistent); From 6291f97c149677ce9a9bb249ca16e5ffec00cafe Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 23:03:51 +0200 Subject: [PATCH 09/17] phar: Move code to avoid goto --- ext/phar/dirstream.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index 71420c11e62d..6b5033659ce3 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -188,8 +188,6 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has entry = safe_emalloc(keylen, 1, 1); memcpy(entry, ZSTR_VAL(str_key), keylen); entry[keylen] = '\0'; - - goto PHAR_ADD_ENTRY; } else { if (0 != memcmp(ZSTR_VAL(str_key), dir, dirlen)) { /* entry in directory not found */ @@ -199,7 +197,6 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has continue; } } - } const char *save = ZSTR_VAL(str_key); save += dirlen + 1; /* seek to just past the path separator */ @@ -220,7 +217,8 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has entry[keylen - dirlen - 1] = '\0'; keylen = keylen - dirlen - 1; } -PHAR_ADD_ENTRY: + } + if (keylen) { /** * Add an empty element to avoid duplicates From a69b35328d02282f18c843b10df4d719f7465c08 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 23:03:59 +0200 Subject: [PATCH 10/17] phar: Re-indent code --- ext/phar/dirstream.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index 6b5033659ce3..f37599e7db11 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -198,25 +198,25 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has } } - const char *save = ZSTR_VAL(str_key); - save += dirlen + 1; /* seek to just past the path separator */ - - const char *has_slash = memchr(save, '/', keylen - dirlen - 1); - if (has_slash) { - /* is subdirectory */ - save -= dirlen + 1; - entry = safe_emalloc(has_slash - save + dirlen, 1, 1); - memcpy(entry, save + dirlen + 1, has_slash - save - dirlen - 1); - keylen = has_slash - save - dirlen - 1; - entry[keylen] = '\0'; - } else { - /* is file */ - save -= dirlen + 1; - entry = safe_emalloc(keylen - dirlen, 1, 1); - memcpy(entry, save + dirlen + 1, keylen - dirlen - 1); - entry[keylen - dirlen - 1] = '\0'; - keylen = keylen - dirlen - 1; - } + const char *save = ZSTR_VAL(str_key); + save += dirlen + 1; /* seek to just past the path separator */ + + const char *has_slash = memchr(save, '/', keylen - dirlen - 1); + if (has_slash) { + /* is subdirectory */ + save -= dirlen + 1; + entry = safe_emalloc(has_slash - save + dirlen, 1, 1); + memcpy(entry, save + dirlen + 1, has_slash - save - dirlen - 1); + keylen = has_slash - save - dirlen - 1; + entry[keylen] = '\0'; + } else { + /* is file */ + save -= dirlen + 1; + entry = safe_emalloc(keylen - dirlen, 1, 1); + memcpy(entry, save + dirlen + 1, keylen - dirlen - 1); + entry[keylen - dirlen - 1] = '\0'; + keylen = keylen - dirlen - 1; + } } if (keylen) { From 9c90e16e188d5fd8cca8cd77fd8c65d29308428e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:34:04 +0200 Subject: [PATCH 11/17] phar: Avoid an error goto in phar_zip_flush() (#20233) --- ext/phar/zip.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 5f025d75e341..5abbc49d3c2c 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -1392,8 +1392,8 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_zip_flush(phar_archive_data *phar, z zend_hash_apply_with_argument(&phar->manifest, phar_zip_changed_apply, (void *) &pass); phar_metadata_tracker_try_ensure_has_serialized_data(&phar->metadata_tracker, phar->is_persistent); - if (pass_error) { -has_pass_error: + if (pass_error + || FAILURE == phar_zip_applysignature(phar, &pass)) { spprintf(error, 4096, "phar zip flush of \"%s\" failed: %s", phar->fname, pass_error); efree(pass_error); nopasserror: @@ -1406,11 +1406,6 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_zip_flush(phar_archive_data *phar, z return; } - if (FAILURE == phar_zip_applysignature(phar, &pass)) { - ZEND_ASSERT(pass_error != NULL); - goto has_pass_error; - } - /* save zip */ cdir_size = php_stream_tell(pass.centralfp); cdir_offset = php_stream_tell(pass.filefp); From df8ce6ddbd713d7e9163dd60d19648e2e4a8bb80 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 00:10:40 +0200 Subject: [PATCH 12/17] Zend: remove zval_dtor() compatibility macro This is an alias for zval_ptr_dtor_nogc(). I've seen people make mistakes against this and use zval_dtor() instead of zval_ptr_dtor(). The crucial detail here is that the former won't root possible GC cycles while the latter will. We can avoid the confusion by just retiring this compatibility macro. Closes GH-20235. --- UPGRADING.INTERNALS | 2 ++ Zend/zend_variables.h | 3 --- ext/pdo/pdo_stmt.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index d269bd107885..42e52480bbb9 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -32,6 +32,8 @@ PHP 8.6 INTERNALS UPGRADE NOTES . ZEND_LTOA() (and ZEND_LTOA_BUF_LEN) has been removed, as it was unsafe. Directly use ZEND_LONG_FMT with a function from the printf family. + . The zval_dtor() alias of zval_ptr_dtor_nogc() has been removed. + Call zval_ptr_dtor_nogc() directly instead. ======================== 2. Build system changes diff --git a/Zend/zend_variables.h b/Zend/zend_variables.h index 1cb745ca1b1d..d90ad9951782 100644 --- a/Zend/zend_variables.h +++ b/Zend/zend_variables.h @@ -81,9 +81,6 @@ ZEND_API void zval_ptr_dtor(zval *zval_ptr); ZEND_API void zval_ptr_safe_dtor(zval *zval_ptr); ZEND_API void zval_internal_ptr_dtor(zval *zvalue); -/* Kept for compatibility */ -#define zval_dtor(zvalue) zval_ptr_dtor_nogc(zvalue) - ZEND_API void zval_add_ref(zval *p); END_EXTERN_C() diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index e2723c703f0a..e87af66c7586 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1052,7 +1052,7 @@ PHP_METHOD(PDOStatement, fetch) array_init_size(return_value, 1); bool success = pdo_do_key_pair_fetch(stmt, ori, off, Z_ARRVAL_P(return_value)); if (!success) { - zval_dtor(return_value); + zval_ptr_dtor_nogc(return_value); PDO_HANDLE_STMT_ERR(); RETURN_FALSE; } From 8e84e9a5514e6566ed8bbcaebfacbd32dc42f74b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:16:55 +0200 Subject: [PATCH 13/17] pgsql: Use cheaper string conversion functions --- ext/pgsql/pgsql.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 3f511d21456b..b59c9ef8f3c8 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3409,7 +3409,8 @@ PHP_FUNCTION(pg_copy_to) static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, zval *value) { - zend_string *tmp = zval_try_get_string(value); + zend_string *tmp_tmp; + zend_string *tmp = zval_try_get_tmp_string(value, &tmp_tmp); if (UNEXPECTED(!tmp)) { return FAILURE; } @@ -3423,11 +3424,11 @@ static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, } if (PQputCopyData(pgsql, ZSTR_VAL(zquery), ZSTR_LEN(zquery)) != 1) { zend_string_release_ex(zquery, false); - zend_string_release(tmp); + zend_tmp_string_release(tmp_tmp); return FAILURE; } zend_string_release_ex(zquery, false); - zend_string_release(tmp); + zend_tmp_string_release(tmp_tmp); return SUCCESS; } From e2396a6ffb60ff3d426f3b66182f60ab4dffdc7c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:19:34 +0200 Subject: [PATCH 14/17] pgsql: Simplify pgsql_copy_from_query() by using character buffers This also avoids some unnecessary zend_string overhead. --- ext/pgsql/pgsql.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index b59c9ef8f3c8..b0e2f53cc8d9 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3414,21 +3414,20 @@ static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, if (UNEXPECTED(!tmp)) { return FAILURE; } - zend_string *zquery = zend_string_alloc(ZSTR_LEN(tmp) + 2, false); - memcpy(ZSTR_VAL(zquery), ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); - ZSTR_LEN(zquery) = ZSTR_LEN(tmp); - if (ZSTR_LEN(tmp) > 0 && ZSTR_VAL(zquery)[ZSTR_LEN(tmp) - 1] != '\n') { - ZSTR_VAL(zquery)[ZSTR_LEN(tmp)] = '\n'; - ZSTR_VAL(zquery)[ZSTR_LEN(tmp) + 1] = '\0'; - ZSTR_LEN(zquery) ++; - } - if (PQputCopyData(pgsql, ZSTR_VAL(zquery), ZSTR_LEN(zquery)) != 1) { - zend_string_release_ex(zquery, false); - zend_tmp_string_release(tmp_tmp); - return FAILURE; + char *zquery = emalloc(ZSTR_LEN(tmp) + 2); + memcpy(zquery, ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); + size_t zquery_len = ZSTR_LEN(tmp); + if (ZSTR_LEN(tmp) > 0 && zquery[ZSTR_LEN(tmp) - 1] != '\n') { + zquery[ZSTR_LEN(tmp)] = '\n'; + zquery[ZSTR_LEN(tmp) + 1] = '\0'; + zquery_len++; } - zend_string_release_ex(zquery, false); zend_tmp_string_release(tmp_tmp); + if (PQputCopyData(pgsql, zquery, zquery_len) != 1) { + efree(zquery); + return FAILURE; + } + efree(zquery); return SUCCESS; } From da9638298c8a578970286106e0e0ee8a031492ab Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:23:47 +0200 Subject: [PATCH 15/17] pgsql: Avoid unnecessary work in pgsql_copy_from_query() if the input already ends in a newline --- ext/pgsql/pgsql.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index b0e2f53cc8d9..1212acdd1476 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3414,21 +3414,22 @@ static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, if (UNEXPECTED(!tmp)) { return FAILURE; } - char *zquery = emalloc(ZSTR_LEN(tmp) + 2); - memcpy(zquery, ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); - size_t zquery_len = ZSTR_LEN(tmp); - if (ZSTR_LEN(tmp) > 0 && zquery[ZSTR_LEN(tmp) - 1] != '\n') { + + int result; + if (ZSTR_LEN(tmp) > 0 && ZSTR_VAL(tmp)[ZSTR_LEN(tmp) - 1] != '\n') { + char *zquery = emalloc(ZSTR_LEN(tmp) + 2); + memcpy(zquery, ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); zquery[ZSTR_LEN(tmp)] = '\n'; zquery[ZSTR_LEN(tmp) + 1] = '\0'; - zquery_len++; - } - zend_tmp_string_release(tmp_tmp); - if (PQputCopyData(pgsql, zquery, zquery_len) != 1) { + result = PQputCopyData(pgsql, zquery, ZSTR_LEN(tmp) + 1); efree(zquery); - return FAILURE; + } else { + result = PQputCopyData(pgsql, ZSTR_VAL(tmp), ZSTR_LEN(tmp)); } - efree(zquery); - return SUCCESS; + + zend_tmp_string_release(tmp_tmp); + + return result != 1 ? FAILURE : SUCCESS; } /* {{{ Copy table from array */ From 5eec4d8001ca97f54ce52e7be4bd131e0f4ef0d6 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:36:08 +0200 Subject: [PATCH 16/17] pgsql: Avoid duplicating strings and factor out parameter building code --- ext/pgsql/pgsql.c | 144 +++++++++++++++++----------------------------- 1 file changed, 53 insertions(+), 91 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 1212acdd1476..89c99cd84132 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -1223,26 +1223,55 @@ PHP_FUNCTION(pg_query) } } -static void _php_pgsql_free_params(char **params, int num_params) +/* The char pointer MUST refer to the char* of a zend_string struct */ +static void php_pgsql_zend_string_release_from_char_pointer(char *ptr) { + zend_string_release((zend_string*) (ptr - XtOffsetOf(zend_string, val))); +} + +static void _php_pgsql_free_params(char **params, uint32_t num_params) { - int i; - for (i = 0; i < num_params; i++) { + for (uint32_t i = 0; i < num_params; i++) { if (params[i]) { - efree(params[i]); + php_pgsql_zend_string_release_from_char_pointer(params[i]); } } efree(params); } +static char **php_pgsql_make_arguments(const HashTable *param_arr, int *num_params) +{ + /* This conversion is safe because of the limit of number of elements in a table. */ + *num_params = (int) zend_hash_num_elements(param_arr); + char **params = safe_emalloc(sizeof(char *), *num_params, 0); + uint32_t i = 0; + + ZEND_HASH_FOREACH_VAL(param_arr, zval *tmp) { + ZVAL_DEREF(tmp); + if (Z_TYPE_P(tmp) == IS_NULL) { + params[i] = NULL; + } else { + zend_string *param_str = zval_try_get_string(tmp); + if (!param_str) { + _php_pgsql_free_params(params, i); + return NULL; + } + params[i] = ZSTR_VAL(param_str); + } + i++; + } ZEND_HASH_FOREACH_END(); + + return params; +} + /* Execute a query */ PHP_FUNCTION(pg_query_params) { zval *pgsql_link = NULL; - zval *pv_param_arr, *tmp; + zval *pv_param_arr; char *query; size_t query_len; bool leftover = false; - int num_params = 0; + int num_params; char **params = NULL; pgsql_link_handle *link; PGconn *pgsql; @@ -1286,26 +1315,9 @@ PHP_FUNCTION(pg_query_params) php_error_docref(NULL, E_NOTICE, "Found results on this connection. Use pg_get_result() to get these results first"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *param_str = zval_try_get_string(tmp); - if (!param_str) { - _php_pgsql_free_params(params, i); - RETURN_THROWS(); - } - params[i] = estrndup(ZSTR_VAL(param_str), ZSTR_LEN(param_str)); - zend_string_release(param_str); - } - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } pgsql_result = PQexecParams(pgsql, query, num_params, @@ -1440,11 +1452,11 @@ PHP_FUNCTION(pg_prepare) PHP_FUNCTION(pg_execute) { zval *pgsql_link = NULL; - zval *pv_param_arr, *tmp; + zval *pv_param_arr; char *stmtname; size_t stmtname_len; bool leftover = false; - int num_params = 0; + int num_params; char **params = NULL; PGconn *pgsql; pgsql_link_handle *link; @@ -1488,25 +1500,9 @@ PHP_FUNCTION(pg_execute) php_error_docref(NULL, E_NOTICE, "Found results on this connection. Use pg_get_result() to get these results first"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *tmp_str; - zend_string *str = zval_get_tmp_string(tmp, &tmp_str); - - params[i] = estrndup(ZSTR_VAL(str), ZSTR_LEN(str)); - zend_tmp_string_release(tmp_str); - } - - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } pgsql_result = PQexecPrepared(pgsql, stmtname, num_params, @@ -4034,9 +4030,9 @@ PHP_FUNCTION(pg_send_query) /* {{{ Send asynchronous parameterized query */ PHP_FUNCTION(pg_send_query_params) { - zval *pgsql_link, *pv_param_arr, *tmp; + zval *pgsql_link, *pv_param_arr; pgsql_link_handle *link; - int num_params = 0; + int num_params; char **params = NULL; char *query; size_t query_len; @@ -4066,25 +4062,9 @@ PHP_FUNCTION(pg_send_query_params) "There are results on this connection. Call pg_get_result() until it returns FALSE"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *tmp_str; - zend_string *str = zval_get_tmp_string(tmp, &tmp_str); - - params[i] = estrndup(ZSTR_VAL(str), ZSTR_LEN(str)); - zend_tmp_string_release(tmp_str); - } - - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } if (PQsendQueryParams(pgsql, query, num_params, NULL, (const char * const *)params, NULL, NULL, 0)) { @@ -4206,8 +4186,8 @@ PHP_FUNCTION(pg_send_execute) { zval *pgsql_link; pgsql_link_handle *link; - zval *pv_param_arr, *tmp; - int num_params = 0; + zval *pv_param_arr; + int num_params; char **params = NULL; char *stmtname; size_t stmtname_len; @@ -4237,27 +4217,9 @@ PHP_FUNCTION(pg_send_execute) "There are results on this connection. Call pg_get_result() until it returns FALSE"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *tmp_str = zval_try_get_string(tmp); - if (UNEXPECTED(!tmp_str)) { - _php_pgsql_free_params(params, i); - return; - } - params[i] = estrndup(ZSTR_VAL(tmp_str), ZSTR_LEN(tmp_str)); - zend_string_release(tmp_str); - } - - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } if (PQsendQueryPrepared(pgsql, stmtname, num_params, (const char * const *)params, NULL, NULL, 0)) { From 020bbea8fde8b83b0a6c3c0416d0a648466acb68 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 17 Oct 2025 23:18:04 +0200 Subject: [PATCH 17/17] phar: Fix memory leak when openssl polyfill returns garbage Closes GH-20210. --- NEWS | 1 + ...sl_sign_invalid_polyfill_return_value.phpt | 34 +++++++++++++++++++ ext/phar/util.c | 4 ++- 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt diff --git a/NEWS b/NEWS index 3bc4e05761f8..020927bb5ff6 100644 --- a/NEWS +++ b/NEWS @@ -55,6 +55,7 @@ PHP NEWS (nielsdos) . Fix potential buffer length truncation due to usage of type int instead of type size_t. (Girgias) + . Fix memory leak when openssl polyfill returns garbage. (nielsdos) - Random: . Fix Randomizer::__serialize() w.r.t. INDIRECTs. (nielsdos) diff --git a/ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt b/ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt new file mode 100644 index 000000000000..37c14188edf9 --- /dev/null +++ b/ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt @@ -0,0 +1,34 @@ +--TEST-- +openssl_sign() polyfill with wrong return value +--EXTENSIONS-- +phar +--SKIPIF-- + +--INI-- +phar.require_hash=0 +--FILE-- +setSignatureAlgorithm(Phar::OPENSSL, "randomcrap"); +try { + $phar->addEmptyDir('blah'); +} catch (PharException $e) { + echo $e->getMessage(); +} + +?> +--CLEAN-- + +--EXPECTF-- +phar error: unable to write signature to tar-based phar: unable to write phar "%s" with requested openssl signature diff --git a/ext/phar/util.c b/ext/phar/util.c index 416aa1dcd7b0..c69f830e6280 100644 --- a/ext/phar/util.c +++ b/ext/phar/util.c @@ -1471,7 +1471,6 @@ static int phar_call_openssl_signverify(int is_sign, php_stream *fp, zend_off_t zval_ptr_dtor_str(&zp[2]); switch (Z_TYPE(retval)) { - default: case IS_LONG: zval_ptr_dtor(&zp[1]); if (1 == Z_LVAL(retval)) { @@ -1483,6 +1482,9 @@ static int phar_call_openssl_signverify(int is_sign, php_stream *fp, zend_off_t *signature_len = Z_STRLEN(zp[1]); zval_ptr_dtor(&zp[1]); return SUCCESS; + default: + zval_ptr_dtor(&retval); + ZEND_FALLTHROUGH; case IS_FALSE: zval_ptr_dtor(&zp[1]); return FAILURE;