Skip to content

Commit 0f93006

Browse files
committed
Fix GH-13836: Renaming a file in a Phar to an already existing filename causes a NULL pointer dereference
If the destination already exists, then the `add` function on the manifest will return NULL, resulting in a NULL entry and therefore a NULL deref. As `copy()` (not `Phar::copy`) chooses to succeed and overwrite the destination if it already exists, we should do the same. Therefore the fix is as simple as changing `add` to `update`.
1 parent 6f11cc4 commit 0f93006

File tree

2 files changed

+36
-2
lines changed

2 files changed

+36
-2
lines changed

ext/phar/stream.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,8 +858,9 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
858858
entry->link = entry->tmp = NULL;
859859
source = entry;
860860

861-
/* add to the manifest, and then store the pointer to the new guy in entry */
862-
entry = zend_hash_str_add_mem(&(phar->manifest), ZSTR_VAL(resource_to->path)+1, ZSTR_LEN(resource_to->path)-1, (void **)&new, sizeof(phar_entry_info));
861+
/* add to the manifest, and then store the pointer to the new guy in entry
862+
* if it already exists, we overwrite the destination like what copy('phar://...', 'phar://...') does. */
863+
entry = zend_hash_str_update_mem(&(phar->manifest), ZSTR_VAL(resource_to->path)+1, ZSTR_LEN(resource_to->path)-1, (void **)&new, sizeof(phar_entry_info));
863864

864865
entry->filename = estrndup(ZSTR_VAL(resource_to->path)+1, ZSTR_LEN(resource_to->path)-1);
865866
if (FAILURE == phar_copy_entry_fp(source, entry, &error)) {

ext/phar/tests/gh13833.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-13836 (Renaming a file in a Phar to an already existing filename causes a NULL pointer dereference)
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.require_hash=0
7+
phar.readonly=0
8+
--FILE--
9+
<?php
10+
$fname = __DIR__ . '/gh13836.phar';
11+
12+
$phar = new Phar($fname, 0, 'a.phar');
13+
$phar['x'] = 'hi1';
14+
$phar['y'] = 'hi2';
15+
16+
var_dump(rename("phar://a.phar/x", "phar://a.phar/y"));
17+
18+
var_dump(isset($phar['x']));
19+
var_dump($phar['y']);
20+
?>
21+
--CLEAN--
22+
<?php
23+
unlink(__DIR__ . '/gh13836.phar');
24+
?>
25+
--EXPECTF--
26+
bool(true)
27+
bool(false)
28+
object(PharFileInfo)#2 (2) {
29+
["pathName":"SplFileInfo":private]=>
30+
string(%d) "phar://%sgh13836.phar/y"
31+
["fileName":"SplFileInfo":private]=>
32+
string(1) "y"
33+
}

0 commit comments

Comments
 (0)