ext/phar: Fix .phar-prefixed non-magic directory handling (#22372)

Use a shared helper for checking whether a path refers to the magic .phar
directory. Treat .phar itself and paths below it as magic, while allowing
non-magic entries that merely use a .phar-prefixed name such as .pharx.

Apply the same check across creation, copying, ArrayAccess, stream lookup,
directory iteration, extraction, and mounts so these paths are handled
consistently.

Co-authored-by: Gina Peter Banyard <girgias@php.net>

Closes #22372

Author: LamentXU123

NEWS

@@ -8,6 +8,13 @@ PHP                                                                        NEWS
   . Fixed memory leaks when calling Collator::__construct() or
     Spoofchecker::__construct() twice. (Weilin Du)
 
+- Phar:
+  . Fixed inconsistent handling of the magic ".phar" directory. Paths such as
+    "/.phar" remain protected, while non-magic paths that merely start with
+    ".phar" are handled consistently across file and directory creation,
+    copying, ArrayAccess, stream lookup, directory iteration and extraction.
+    (Weilin Du)
+
 - Reflection:
   . Fixed bug GH-22324 (Ignore leading namespace separator in
     ReflectionParameter::__construct()). (jorgsowa)

ext/phar/dirstream.c

@@ -178,7 +178,7 @@ static php_stream *phar_make_dirstream(char *dir, HashTable *manifest) /* {{{ */
 	ALLOC_HASHTABLE(data);
 	zend_hash_init(data, 64, NULL, NULL, 0);
 
-	if ((*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) || (dirlen >= sizeof(".phar")-1 && !memcmp(dir, ".phar", sizeof(".phar")-1))) {
+	if ((*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) || phar_path_is_magic_phar_ex(dir, dirlen)) {
 		/* make empty root directory for empty phar */
 		/* make empty directory for .phar magic directory */
 		efree(dir);
@@ -204,7 +204,7 @@ static php_stream *phar_make_dirstream(char *dir, HashTable *manifest) /* {{{ */
 
 		if (*dir == '/') {
 			/* root directory */
-			if (keylen >= sizeof(".phar")-1 && !memcmp(ZSTR_VAL(str_key), ".phar", sizeof(".phar")-1)) {
+			if (phar_is_magic_phar(str_key)) {
 				/* do not add any magic entries to this directory */
 				if (SUCCESS != zend_hash_move_forward(manifest)) {
 					break;

ext/phar/phar_internal.h

@@ -381,6 +381,31 @@ static inline bool phar_validate_alias(const char *alias, size_t alias_len) /* {
 }
 /* }}} */
 
+static inline bool phar_path_is_magic_phar_ex(const char *path, size_t path_len) /* {{{ */
+{
+	if (path_len > 0 && path[0] == '/') {
+		path++;
+		path_len--;
+	}
+
+	if (path_len < sizeof(".phar") - 1 || memcmp(path, ".phar", sizeof(".phar") - 1) != 0) {
+		return false;
+	}
+
+	if (path_len == sizeof(".phar") - 1) {
+		return true;
+	}
+
+	return path[sizeof(".phar") - 1] == '/' || path[sizeof(".phar") - 1] == '\\';
+}
+/* }}} */
+
+static inline bool phar_is_magic_phar(const zend_string *path) /* {{{ */
+{
+	return phar_path_is_magic_phar_ex(ZSTR_VAL(path), ZSTR_LEN(path));
+}
+/* }}} */
+
 static inline void phar_set_inode(phar_entry_info *entry) /* {{{ */
 {
 	char tmp[MAXPATHLEN];

ext/phar/phar_object.c

@@ -1619,7 +1619,7 @@ static int phar_build(zend_object_iterator *iter, void *puser) /* {{{ */
 		return ZEND_HASH_APPLY_STOP;
 	}
 after_open_fp:
-	if (str_key_len >= sizeof(".phar")-1 && !memcmp(str_key, ".phar", sizeof(".phar")-1)) {
+	if (phar_path_is_magic_phar_ex(str_key, str_key_len)) {
 		/* silently skip any files that would be added to the magic .phar directory */
 		if (save) {
 			efree(save);
@@ -3468,14 +3468,14 @@ PHP_METHOD(Phar, copy)
 		RETURN_THROWS();
 	}
 
-	if (zend_string_starts_with_literal(old_file, ".phar")) {
+	if (phar_is_magic_phar(old_file)) {
 		/* can't copy a meta file */
 		zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0,
 			"file \"%s\" cannot be copied to file \"%s\", cannot copy Phar meta-file in %s", ZSTR_VAL(old_file), ZSTR_VAL(new_file), phar_obj->archive->fname);
 		RETURN_THROWS();
 	}
 
-	if (zend_string_starts_with_literal(new_file, ".phar")) {
+	if (phar_is_magic_phar(new_file)) {
 		/* can't copy a meta file */
 		zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0,
 			"file \"%s\" cannot be copied to file \"%s\", cannot copy to Phar meta-file in %s", ZSTR_VAL(old_file), ZSTR_VAL(new_file), phar_obj->archive->fname);
@@ -3562,11 +3562,8 @@ PHP_METHOD(Phar, offsetExists)
 			}
 		}
 
-		if (zend_string_starts_with_literal(file_name, ".phar")) {
-			/* none of these are real files, so they don't exist */
-			RETURN_FALSE;
-		}
-		RETURN_TRUE;
+		/* none of these are real files, so they don't exist */
+		RETURN_BOOL(!phar_is_magic_phar(file_name));
 	} else {
 		/* If the info class is not based on PharFileInfo, directories are not directly instantiable */
 		if (UNEXPECTED(!instanceof_function(phar_obj->spl.info_class, phar_ce_entry))) {
@@ -3609,7 +3606,7 @@ PHP_METHOD(Phar, offsetGet)
 			RETURN_THROWS();
 		}
 
-		if (zend_string_starts_with_literal(file_name, ".phar")) {
+		if (phar_is_magic_phar(file_name)) {
 			zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot directly get any files or directories in magic \".phar\" directory");
 			RETURN_THROWS();
 		}
@@ -3640,16 +3637,9 @@ static void phar_add_file(phar_archive_data **pphar, zend_string *file_name, con
 	ALLOCA_FLAG(filename_use_heap)
 #endif
 
-	if (
-		zend_string_starts_with_literal(file_name, ".phar")
-		|| zend_string_starts_with_literal(file_name, "/.phar")
-	) {
-		size_t prefix_len = (ZSTR_VAL(file_name)[0] == '/') + sizeof(".phar")-1;
-		char next_char = ZSTR_VAL(file_name)[prefix_len];
-		if (next_char == '/' || next_char == '\\' || next_char == '\0') {
-			zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot create any files in magic \".phar\" directory");
-			return;
-		}
+	if (phar_is_magic_phar(file_name)) {
+		zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot create any files in magic \".phar\" directory");
+		return;
 	}
 
 	/* TODO How to handle Windows path normalisation with zend_string ? */
@@ -3796,7 +3786,7 @@ PHP_METHOD(Phar, offsetSet)
 		RETURN_THROWS();
 	}
 
-	if (zend_string_starts_with_literal(file_name, ".phar")) {
+	if (phar_is_magic_phar(file_name)) {
 		zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot set any files or directories in magic \".phar\" directory");
 		RETURN_THROWS();
 	}
@@ -3863,16 +3853,9 @@ PHP_METHOD(Phar, addEmptyDir)
 
 	PHAR_ARCHIVE_OBJECT();
 
-	if (
-		zend_string_starts_with_literal(dir_name, ".phar")
-		|| zend_string_starts_with_literal(dir_name, "/.phar")
-	) {
-		size_t prefix_len = (ZSTR_VAL(dir_name)[0] == '/') + sizeof(".phar")-1;
-		char next_char = ZSTR_VAL(dir_name)[prefix_len];
-		if (next_char == '/' || next_char == '\\' || next_char == '\0') {
-			zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot create a directory in magic \".phar\" directory");
-			RETURN_THROWS();
-		}
+	if (phar_is_magic_phar(dir_name)) {
+		zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot create a directory in magic \".phar\" directory");
+		RETURN_THROWS();
 	}
 
 	phar_mkdir(&phar_obj->archive, dir_name);
@@ -4178,7 +4161,7 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 		return SUCCESS;
 	}
 
-	if (entry->filename_len >= sizeof(".phar")-1 && !memcmp(entry->filename, ".phar", sizeof(".phar")-1)) {
+	if (phar_path_is_magic_phar_ex(entry->filename, entry->filename_len)) {
 		return SUCCESS;
 	}
 	/* strip .. from path and restrict it to be under dest directory */

ext/phar/tests/phar_magic_dir_prefix.phpt

@@ -0,0 +1,80 @@
+--TEST--
+Phar: .phar-prefixed non-magic directories are accessible
+--EXTENSIONS--
+phar
+--INI--
+phar.readonly=0
+phar.require_hash=0
+--FILE--
+<?php
+$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar.php';
+$pname = 'phar://' . $fname;
+
+$phar = new Phar($fname);
+$phar['.pharx/array.txt'] = 'array';
+$phar->addFromString('.pharx/from-string.txt', 'from-string');
+$phar->addFromString('/.phary/leading.txt', 'leading');
+$phar->copy('.pharx/array.txt', '.pharx/copy.txt');
+
+var_dump(isset($phar['.pharx/array.txt']));
+echo $phar['.pharx/array.txt']->getContent(), "\n";
+echo file_get_contents($pname . '/.pharx/from-string.txt'), "\n";
+echo file_get_contents($pname . '/.phary/leading.txt'), "\n";
+echo file_get_contents($pname . '/.pharx/copy.txt'), "\n";
+
+$root = [];
+$dh = opendir($pname . '/');
+while (false !== ($entry = readdir($dh))) {
+    $root[] = $entry;
+}
+closedir($dh);
+sort($root);
+var_dump($root);
+
+$subdir = [];
+$dh = opendir($pname . '/.pharx');
+while (false !== ($entry = readdir($dh))) {
+    $subdir[] = $entry;
+}
+closedir($dh);
+sort($subdir);
+var_dump($subdir);
+
+try {
+    $phar->addFromString('.phar/still-magic.txt', 'no');
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $phar->addEmptyDir('/.phar');
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.php');
+?>
+--EXPECT--
+bool(true)
+array
+from-string
+leading
+array
+array(2) {
+  [0]=>
+  string(6) ".pharx"
+  [1]=>
+  string(6) ".phary"
+}
+array(3) {
+  [0]=>
+  string(9) "array.txt"
+  [1]=>
+  string(8) "copy.txt"
+  [2]=>
+  string(15) "from-string.txt"
+}
+Cannot create any files in magic ".phar" directory
+Cannot create a directory in magic ".phar" directory

ext/phar/util.c

@@ -208,7 +208,7 @@ zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t fil
 		return FAILURE;
 	}
 
-	if (path_len >= sizeof(".phar")-1 && !memcmp(path, ".phar", sizeof(".phar")-1)) {
+	if (phar_path_is_magic_phar_ex(path, path_len)) {
 		/* no creating magic phar files by mounting them */
 		return FAILURE;
 	}
@@ -1290,7 +1290,7 @@ phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, si
 		*error = NULL;
 	}
 
-	if (security && path_len >= sizeof(".phar")-1 && !memcmp(path, ".phar", sizeof(".phar")-1)) {
+	if (security && phar_path_is_magic_phar_ex(path, path_len)) {
 		if (error) {
 			spprintf(error, 4096, "phar error: cannot directly access magic \".phar\" directory or files within it");
 		}