Skip to content

Commit b9aaa05

Browse files
iliaalGirgias
authored andcommitted
phar: free is_temp_dir entry before rejecting .phar/* paths in offsetGet
Phar::offsetGet() calls phar_get_entry_info_dir with allow_dir=1, which may return a heap-allocated temporary directory entry (is_temp_dir=1) for paths that resolve to a virtual directory in the manifest. Three early-exit paths for .phar/stub.php, .phar/alias.txt, and the generic .phar/* prefix all called RETURN_THROWS() before the is_temp_dir cleanup block, leaking the entry and its filename buffer on every rejection. Move the is_temp_dir cleanup before the .phar/* guards so all exit paths release the temporary entry regardless of which rejection fires. Closes GH-21798
1 parent 36c4195 commit b9aaa05

2 files changed

Lines changed: 44 additions & 5 deletions

File tree

ext/phar/phar_object.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3594,6 +3594,11 @@ PHP_METHOD(Phar, offsetGet)
35943594
if (!(entry = phar_get_entry_info_dir(phar_obj->archive, ZSTR_VAL(file_name), ZSTR_LEN(file_name), 1, &error, 0))) {
35953595
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s does not exist%s%s", ZSTR_VAL(file_name), error?", ":"", error?error:"");
35963596
} else {
3597+
if (entry->is_temp_dir) {
3598+
efree(entry->filename);
3599+
efree(entry);
3600+
}
3601+
35973602
if (zend_string_equals_literal(file_name, ".phar/stub.php")) {
35983603
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot get stub \".phar/stub.php\" directly in phar \"%s\", use getStub", phar_obj->archive->fname);
35993604
RETURN_THROWS();
@@ -3609,11 +3614,6 @@ PHP_METHOD(Phar, offsetGet)
36093614
RETURN_THROWS();
36103615
}
36113616

3612-
if (entry->is_temp_dir) {
3613-
efree(entry->filename);
3614-
efree(entry);
3615-
}
3616-
36173617
zend_string *sfname = strpprintf(0, "phar://%s/%s", phar_obj->archive->fname, ZSTR_VAL(file_name));
36183618
zval zfname;
36193619
ZVAL_NEW_STR(&zfname, sfname);
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
GH-21798: Phar::offsetGet() must free is_temp_dir entry before rejecting .phar/* paths
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.readonly=0
7+
phar.require_hash=0
8+
--FILE--
9+
<?php
10+
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar';
11+
$phar = new Phar($fname);
12+
$phar->addFromString('index.php', '<?php echo "ok"; ?>');
13+
unset($phar);
14+
15+
$phar = new Phar($fname);
16+
try {
17+
$phar->offsetGet('.phar/stub.php');
18+
} catch (BadMethodCallException $e) {
19+
echo $e->getMessage() . "\n";
20+
}
21+
try {
22+
$phar->offsetGet('.phar/alias.txt');
23+
} catch (BadMethodCallException $e) {
24+
echo $e->getMessage() . "\n";
25+
}
26+
try {
27+
$phar->offsetGet('.phar/internal');
28+
} catch (BadMethodCallException $e) {
29+
echo $e->getMessage() . "\n";
30+
}
31+
echo "no crash\n";
32+
?>
33+
--CLEAN--
34+
<?php @unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar'); ?>
35+
--EXPECT--
36+
Entry .phar/stub.php does not exist
37+
Entry .phar/alias.txt does not exist
38+
Entry .phar/internal does not exist
39+
no crash

0 commit comments

Comments
 (0)