Fix GH-20302: Freeing a phar alias may invalidate PharFileInfo objects

Closes GH-20345.

Author: ndossche

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)

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;
 }
 /* }}} */
 

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--
+<?php
+$fname = __DIR__.'/gh20302.phar';
+$pname = 'phar://' . $fname;
+$file = "<?php
+__HALT_COMPILER(); ?>";
+$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--
+<?php
+@unlink(__DIR__.'/gh20302/gh20302.phar');
+@unlink(__DIR__.'/gh20302.phar');
+@rmdir(__DIR__.'/gh20302');
+?>
+--EXPECTF--
+Cannot open archive "%sgh20302.phar", alias is already in use by existing archive