From 4923a3b147f4fcb87f7a70eb2c3366a41f51bc2f Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sun, 31 Mar 2024 01:17:49 +0100
Subject: [PATCH] Fix GH-13833: Applying zero offset to null pointer in
 zend_hash.c

MAPPHAR_FAIL will call the destructor of the manifest, mounted_dirs, and
virtual_dirs tables. When a new phar object is allocated using (p)ecalloc,
the bytes are zeroed, but the flag for an uninitialized table is
non-zero. So we have to manually set the flag in case that we have a
code path that can destroy the tables without first initializing them at
least once.
---
 ext/phar/phar.c             |  3 +++
 ext/phar/tests/gh13833.phpt | 52 +++++++++++++++++++++++--------------
 ext/phar/tests/gh13836.phpt | 33 +++++++++++++++++++++++
 3 files changed, 68 insertions(+), 20 deletions(-)
 create mode 100644 ext/phar/tests/gh13836.phpt

diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index f60b0d6a7c04b..b774f22e2d565 100644
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -1090,6 +1090,9 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, size_t fname_len, ch
 
 	mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist));
 	mydata->is_persistent = PHAR_G(persist);
+	HT_INVALIDATE(&mydata->manifest);
+	HT_INVALIDATE(&mydata->mounted_dirs);
+	HT_INVALIDATE(&mydata->virtual_dirs);
 
 	/* check whether we have meta data, zero check works regardless of byte order */
 	SAFE_PHAR_GET_32(buffer, endbuffer, len);
diff --git a/ext/phar/tests/gh13833.phpt b/ext/phar/tests/gh13833.phpt
index d1b79d033e0e8..026735deac14e 100644
--- a/ext/phar/tests/gh13833.phpt
+++ b/ext/phar/tests/gh13833.phpt
@@ -1,33 +1,45 @@
 --TEST--
-GH-13836 (Renaming a file in a Phar to an already existing filename causes a NULL pointer dereference)
---EXTENSIONS--
-phar
+GH-13833 (Applying zero offset to null pointer in zend_hash.c)
 --INI--
 phar.require_hash=0
 phar.readonly=0
 --FILE--
 <?php
-$fname = __DIR__ . '/gh13836.phar';
+$fname = __DIR__ . '/gh13833.phar';
+$pname = 'phar://' . $fname;
+$file = "<?php
+Phar::mapPhar('hio');
+__HALT_COMPILER(); ?>";
+$files = array();
+$files['a'] = 'a';
+include 'files/phar_test.inc';
+include $fname;
 
-$phar = new Phar($fname, 0, 'a.phar');
-$phar['x'] = 'hi1';
-$phar['y'] = 'hi2';
+$file = "<?php __HALT_COMPILER(); ?>";
+$files['a'] = array('cont' => 'a');
+include 'files/phar_test.inc';
 
-var_dump(rename("phar://a.phar/x", "phar://a.phar/y"));
-
-var_dump(isset($phar['x']));
-var_dump($phar['y']);
+$phar = new Phar($fname);
+$phar->setMetadata((object) ['my' => 'friend']);
+// NOTE: Phar will use the cached value of metadata if setMetaData was called on that Phar path before.
+// Save the writes to the phar and use a different file path.
+$fname_new = "$fname.copy.phar";
+copy($fname, $fname_new);
+try {
+  new Phar($fname_new);
+} catch (UnexpectedValueException $e) {
+  echo $e->getMessage(), "\n";
+}
 ?>
+--EXTENSIONS--
+phar
 --CLEAN--
 <?php
-unlink(__DIR__ . '/gh13836.phar');
+unlink(__DIR__ . '/gh13833.phar');
+unlink(__DIR__ . '/gh13833.phar.copy.phar');
 ?>
+--CREDITS--
+Yuancheng Jiang <yuancheng@comp.nus.edu.sg>
+Felix De Vliegher <felix.devliegher@gmail.com>
 --EXPECTF--
-bool(true)
-bool(false)
-object(PharFileInfo)#2 (2) {
-  ["pathName":"SplFileInfo":private]=>
-  string(%d) "phar://%sgh13836.phar/y"
-  ["fileName":"SplFileInfo":private]=>
-  string(1) "y"
-}
+internal corruption of phar "%sgh13833.phar.copy.phar" (trying to read past buffer end)
diff --git a/ext/phar/tests/gh13836.phpt b/ext/phar/tests/gh13836.phpt
new file mode 100644
index 0000000000000..d1b79d033e0e8
--- /dev/null
+++ b/ext/phar/tests/gh13836.phpt
@@ -0,0 +1,33 @@
+--TEST--
+GH-13836 (Renaming a file in a Phar to an already existing filename causes a NULL pointer dereference)
+--EXTENSIONS--
+phar
+--INI--
+phar.require_hash=0
+phar.readonly=0
+--FILE--
+<?php
+$fname = __DIR__ . '/gh13836.phar';
+
+$phar = new Phar($fname, 0, 'a.phar');
+$phar['x'] = 'hi1';
+$phar['y'] = 'hi2';
+
+var_dump(rename("phar://a.phar/x", "phar://a.phar/y"));
+
+var_dump(isset($phar['x']));
+var_dump($phar['y']);
+?>
+--CLEAN--
+<?php
+unlink(__DIR__ . '/gh13836.phar');
+?>
+--EXPECTF--
+bool(true)
+bool(false)
+object(PharFileInfo)#2 (2) {
+  ["pathName":"SplFileInfo":private]=>
+  string(%d) "phar://%sgh13836.phar/y"
+  ["fileName":"SplFileInfo":private]=>
+  string(1) "y"
+}