From 6fe40de6e3c230432b2cd9fd96ffe09e8984b0eb Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Thu, 30 Oct 2025 22:43:07 +0100 Subject: [PATCH] Fix GH-20302: Freeing a phar alias may invalidate PharFileInfo objects Closes GH-20345. --- NEWS | 2 ++ ext/phar/phar_object.c | 28 +++++++++++++++++----------- ext/phar/tests/gh20302.phpt | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 ext/phar/tests/gh20302.phpt diff --git a/NEWS b/NEWS index 786a9d23fa365..606f476bb96aa 100644 --- a/NEWS +++ b/NEWS @@ -65,6 +65,8 @@ PHP NEWS . Fix file descriptor leak in phar_zip_flush() on failure. (nielsdos) . Fix memory leak when opening temp file fails while trying to open gzip-compressed archive. (nielsdos) + . Fixed bug GH-20302 (Freeing a phar alias may invalidate + PharFileInfo objects). (nielsdos) - Random: . Fix Randomizer::__serialize() w.r.t. INDIRECTs. (nielsdos) diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index c5bd8da398c0c..7c1bb6293782e 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -4493,6 +4493,9 @@ PHP_METHOD(PharFileInfo, __construct) entry_obj->entry = entry_info; if (!entry_info->is_persistent && !entry_info->is_temp_dir) { ++entry_info->fp_refcount; + /* The phar data must exist to keep the alias locked. */ + ZEND_ASSERT(!phar_data->is_persistent); + ++phar_data->refcount; } ZVAL_STRINGL(&arg1, fname, fname_len); @@ -4523,23 +4526,26 @@ PHP_METHOD(PharFileInfo, __destruct) RETURN_THROWS(); } - if (!entry_obj->entry) { + phar_entry_info *entry = entry_obj->entry; + if (!entry) { return; } - if (entry_obj->entry->is_temp_dir) { - if (entry_obj->entry->filename) { - efree(entry_obj->entry->filename); - entry_obj->entry->filename = NULL; + if (entry->is_temp_dir) { + if (entry->filename) { + efree(entry->filename); + entry->filename = NULL; } - efree(entry_obj->entry); - } else if (!entry_obj->entry->is_persistent) { - --entry_obj->entry->fp_refcount; - /* It is necessarily still in the manifest, which will ultimately free this. */ + efree(entry); + entry_obj->entry = NULL; + } else if (!entry->is_persistent) { + --entry->fp_refcount; + /* The entry itself still lives in the manifest, + * which will either be freed here if the file info was the last reference; or freed later. */ + entry_obj->entry = NULL; + phar_archive_delref(entry->phar); } - - entry_obj->entry = NULL; } /* }}} */ diff --git a/ext/phar/tests/gh20302.phpt b/ext/phar/tests/gh20302.phpt new file mode 100644 index 0000000000000..0cbd253d54fc3 --- /dev/null +++ b/ext/phar/tests/gh20302.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-20302 (Freeing a phar alias may invalidate PharFileInfo objects) +--EXTENSIONS-- +phar +--INI-- +phar.require_hash=0 +--FILE-- +"; +$files = array(); +$files['here'] = 'a'; +include __DIR__.'/files/phar_test.inc'; +$b = new PharFileInfo($pname . '/here'); + +// Create new phar with same alias and open it +@mkdir(__DIR__.'/gh20302'); +$fname = __DIR__.'/gh20302/gh20302.phar'; +$pname = 'phar://' . $fname; +include __DIR__.'/files/phar_test.inc'; +try { + new Phar($fname); +} catch (UnexpectedValueException $e) { + echo $e->getMessage(), "\n"; +} +?> +--CLEAN-- + +--EXPECTF-- +Cannot open archive "%sgh20302.phar", alias is already in use by existing archive