From 32344c4dc463affe588496976eadf55ba6658178 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 27 Jun 2025 11:05:33 +0800
Subject: [PATCH] Fix stream double free in phar

The copy function does two things wrong:
- The error recovery logic is a hack that temporarily moves the fp
  pointer to cfp, even though it's not compressed. The respective error
  recovery it talks about is not present in the code, nor is it
  necessary. This is the direct cause of the double free in the original
  reproducer. Fixing this makes it crash in another location though.
- The link following logic is inconsistent and illogical. It cannot be a
  link at this point.

The root cause, after fixing the above issues, is that the file pointers
are not reset properly for the copy. The file pointer need to be the
original ones to perform the copy from the right source, but after that
they need to be set properly to NULL (because fp_type == PHAR_FP).

Closes GH-19035.

Co-authored-by: Yun Dou <dixyes@gmail.com>
---
 NEWS                        |  3 +++
 ext/phar/phar_object.c      | 23 ++++++++-------------
 ext/phar/tests/gh18953.phpt | 40 +++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 15 deletions(-)
 create mode 100644 ext/phar/tests/gh18953.phpt

diff --git a/NEWS b/NEWS
index 11b5146bc33..df26a2d9823 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,9 @@ PHP                                                                        NEWS
   . Fixed bug GH-18958 (Fatal error during shutdown after pcntl_rfork() or
     pcntl_forkx() with zend-max-execution-timers). (Arnaud)
 
+- Phar:
+  . Fix stream double free in phar. (nielsdos, dixyes)
+
 - SOAP:
   . Fixed bug GH-18990, bug #81029, bug #47314 (SOAP HTTP socket not closing
     on object destruction). (nielsdos)
diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c
index bd724b10369..cb032095c39 100644
--- a/ext/phar/phar_object.c
+++ b/ext/phar/phar_object.c
@@ -1926,7 +1926,8 @@ static int phar_copy_file_contents(phar_entry_info *entry, php_stream *fp) /* {{
 {
 	char *error;
 	zend_off_t offset;
-	phar_entry_info *link;
+
+	ZEND_ASSERT(!entry->link);
 
 	if (FAILURE == phar_open_entry_fp(entry, &error, 1)) {
 		if (error) {
@@ -1941,26 +1942,14 @@ static int phar_copy_file_contents(phar_entry_info *entry, php_stream *fp) /* {{
 	}
 
 	/* copy old contents in entirety */
-	phar_seek_efp(entry, 0, SEEK_SET, 0, 1);
+	phar_seek_efp(entry, 0, SEEK_SET, 0, 0);
 	offset = php_stream_tell(fp);
-	link = phar_get_link_source(entry);
-
-	if (!link) {
-		link = entry;
-	}
-
-	if (SUCCESS != php_stream_copy_to_stream_ex(phar_get_efp(link, 0), fp, link->uncompressed_filesize, NULL)) {
+	if (SUCCESS != php_stream_copy_to_stream_ex(phar_get_efp(entry, 0), fp, entry->uncompressed_filesize, NULL)) {
 		zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0,
 			"Cannot convert phar archive \"%s\", unable to copy entry \"%s\" contents", entry->phar->fname, entry->filename);
 		return FAILURE;
 	}
 
-	if (entry->fp_type == PHAR_MOD) {
-		/* save for potential restore on error */
-		entry->cfp = entry->fp;
-		entry->fp = NULL;
-	}
-
 	/* set new location of file contents */
 	entry->fp_type = PHAR_FP;
 	entry->offset = offset;
@@ -2299,6 +2288,10 @@ static zend_object *phar_convert_to_other(phar_archive_data *source, int convert
 			return NULL;
 		}
 no_copy:
+		/* Reset file pointers, they have to be reset here such that if a copy happens the original
+		 * source fp can be accessed. */
+		newentry.fp = NULL;
+		newentry.cfp = NULL;
 		newentry.filename = estrndup(newentry.filename, newentry.filename_len);
 
 		phar_metadata_tracker_clone(&newentry.metadata_tracker);
diff --git a/ext/phar/tests/gh18953.phpt b/ext/phar/tests/gh18953.phpt
new file mode 100644
index 00000000000..52bbace2406
--- /dev/null
+++ b/ext/phar/tests/gh18953.phpt
@@ -0,0 +1,40 @@
+--TEST--
+GH-18953 (Phar: Stream double free)
+--EXTENSIONS--
+phar
+--INI--
+phar.readonly=0
+--FILE--
+<?php
+
+$phar = new Phar(__DIR__ . "/gh18953.phar");
+$phar->addFromString("file", str_repeat("123", random_int(1, 1)));
+$phar->addEmptyDir("dir");
+$phar2 = $phar->compress(Phar::GZ);
+
+var_dump($phar["dir"]);
+var_dump($phar2["dir"]);
+var_dump($phar["file"]->openFile()->fread(100));
+var_dump($phar2["file"]->openFile()->fread(100));
+
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . "/gh18953.phar");
+@unlink(__DIR__ . "/gh18953.phar.gz");
+?>
+--EXPECTF--
+object(PharFileInfo)#%d (2) {
+  ["pathName":"SplFileInfo":private]=>
+  string(%d) "%sphar%sdir"
+  ["fileName":"SplFileInfo":private]=>
+  string(3) "dir"
+}
+object(PharFileInfo)#%d (2) {
+  ["pathName":"SplFileInfo":private]=>
+  string(%d) "%sphar.gz%sdir"
+  ["fileName":"SplFileInfo":private]=>
+  string(3) "dir"
+}
+string(3) "123"
+string(3) "123"