Renaming a file in a Phar to an already existing filename causes a NULL pointer dereference

[YuanchengJiang]

Description

This case (https://github.com/php/php-src/blob/master/ext/phar/tests/bug47085.phpt) failed in the latest commit, not seen before.

--TEST--
Phar: PHP bug #47085: "rename() returns true even if the file in PHAR does not exist"
--EXTENSIONS--
phar
--INI--
phar.require_hash=0
phar.readonly=0
--FILE--
<?php
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar';

$phar = new Phar($fname, 0, 'a.phar');
$phar['x'] = 'hi';
unset($phar);
rename("phar://a.phar/x", "phar://a.phar/y");
var_dump(rename("phar://a.phar/x", "phar://a.phar/y"));
?>
--CLEAN--
<?php unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar');?>
--EXPECTF--
Warning: rename(): phar error: cannot rename "phar://a.phar/x" to "phar://a.phar/y" from extracted phar archive, source does not exist in %sbug47085.php on line %d
bool(false)

Output:

/php-src/ext/phar/stream.c:864:10: runtime error: member access within null pointer of type 'phar_entry_info' (aka 'struct _phar_entry_info')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /php-src/ext/phar/stream.c:864:10 in 

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

[ndossche]

Tested with ASAN, UBSAN, and Valgrind with both USE_ZEND_ALLOC=0 and USE_ZEND_ALLOC=1, also tested with debug and without debug; but I can't reproduce this...
Please give more information about how you ran this test, info about your php.ini, and what configuration options you used. I'm testing on commit f2b2b7f.

[YuanchengJiang]

scripts to reproduce:

/php-src/sapi/cli/php  -n -c '/php-src/tmp-php.ini'   -d "opcache.cache_id=worker22" -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "opcache.jit_max_root_traces=100000" -d "opcache.jit_max_side_traces=100000" -d "opcache.jit_max_exit_counters=100000" -d "opcache.protect_memory=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -d "extension_dir=/php-src/modules/" -d "zend_extension=/php-src/modules/opcache.so" -d "session.auto_start=0" -d "phar.require_hash=0" -d "phar.readonly=0" -f "/php-src/ext/phar/tests/test.php"  2>&1

[ndossche]

Thanks, it reproduces now; what was missing was running under opcache, and running it outside run-tests.php.

[ndossche]

It seems it reproduces only starting from the second execution because the clean section doesn't actually clean it outside run-tests.php.
What's actually going on is that if you rename a file to a filename that already exists, it crashes. This is because it uses the add api to add a new entry for the new filename, but that returns NULL because the entry exists already. That's why it only works after loading the existing phar that has the y file.
Since this never worked before, we can just error out properly, similar to what copy does. EDIT: actually Phar::copy throws while copy succeeds so we should follow the latter.