Fix GH-21333: use-after-free when unlinking entries during iteration of a compressed phar.

close GH-21334

Author: devnexen

NEWS

@@ -13,6 +13,10 @@ PHP                                                                        NEWS
   . Fixed re-entrancy issue on php_pcre_match_impl, php_pcre_replace_impl,
     php_pcre_split_impl, and php_pcre_grep_impl. (David Carlier)
 
+- Phar:
+  . Fixed bug GH-21333 (use after free when unlinking entries during iteration
+    of a compressed phar). (David Carlier)
+
 - SNMP:
   . Fixed bug GH-21336 (SNMP::setSecurity() undefined behavior with
     NULL arguments). (David Carlier)

ext/phar/phar.c

@@ -2461,7 +2461,7 @@ static int phar_flush_clean_deleted_apply(zval *zv) /* {{{ */
 {
 	phar_entry_info *entry = (phar_entry_info *)Z_PTR_P(zv);
 
-	if (entry->fp_refcount <= 0 && entry->is_deleted) {
+	if (entry->is_deleted && phar_entry_can_remove(entry)) {
 		return ZEND_HASH_APPLY_REMOVE;
 	} else {
 		return ZEND_HASH_APPLY_KEEP;

ext/phar/phar_internal.h

@@ -400,6 +400,11 @@ static inline void phar_set_inode(phar_entry_info *entry) /* {{{ */
 }
 /* }}} */
 
+static inline bool phar_entry_can_remove(phar_entry_info *entry)
+{
+	return entry->fp_refcount == 0 && entry->fileinfo_lock_count == 0;
+}
+
 void phar_request_initialize(void);
 
 void phar_object_init(void);

ext/phar/tar.c

@@ -730,7 +730,7 @@ static int phar_tar_writeheaders_int(phar_entry_info *entry, void *argument) /*
 	}
 
 	if (entry->is_deleted) {
-		if (entry->fp_refcount <= 0 && entry->fileinfo_lock_count == 0) {
+		if (phar_entry_can_remove(entry)) {
 			return ZEND_HASH_APPLY_REMOVE;
 		} else {
 			/* we can't delete this in-memory until it is closed */

ext/phar/tests/gh21333.phpt

@@ -0,0 +1,38 @@
+--TEST--
+GH-21333 (UAF when unlinking entries during iteration of a compressed phar)
+--CREDITS--
+YuanchengJiang
+--EXTENSIONS--
+phar
+zlib
+--INI--
+phar.readonly=0
+--FILE--
+<?php
+$phar_path = __DIR__ . "/gh21333.phar";
+$phar = new Phar($phar_path);
+$phar->addFromString("file", "initial_content");
+$phar->addEmptyDir("dir");
+
+$phar2 = $phar->compress(Phar::GZ);
+
+$tmp_src = __DIR__ . "/gh21333.tmp";
+file_put_contents($tmp_src, str_repeat("A", 100));
+
+foreach ($phar2 as $item) {
+    @copy($tmp_src, $item);
+    @unlink($item);
+}
+
+$garbage = get_defined_vars();
+
+echo "Done\n";
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . "/gh21333.phar");
+@unlink(__DIR__ . "/gh21333.phar.gz");
+@unlink(__DIR__ . "/gh21333.tmp");
+?>
+--EXPECT--
+Done

ext/phar/zip.c

@@ -865,7 +865,7 @@ static int phar_zip_changed_apply_int(phar_entry_info *entry, void *arg) /* {{{
 	}
 
 	if (entry->is_deleted) {
-		if (entry->fp_refcount <= 0 && entry->fileinfo_lock_count == 0) {
+		if (phar_entry_can_remove(entry)) {
 			return ZEND_HASH_APPLY_REMOVE;
 		} else {
 			/* we can't delete this in-memory until it is closed */