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

Closes phpGH-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