Skip to content

Commit c36c50e

Browse files
GirgiasGirgias
committed
Merge branch 'PHP-8.5'
* PHP-8.5: Update NEWS for recent bug fixes ext/phar: Fix memory leak in phar_verify_signature() when md_ctx is invalid phar: propagate phar_stream_flush return value from phar_stream_close phar: call phar_entry_delref before goto finish in phar_add_file error paths phar: free is_temp_dir entry before rejecting .phar/* paths in offsetGet phar: fix NULL dereference in Phar::webPhar() when SCRIPT_NAME is absent phar: restore is_link handler in phar_intercept_functions_shutdown
2 parents 383ff8c + 786356d commit c36c50e

File tree

9 files changed

+173
-7
lines changed

9 files changed

+173
-7
lines changed

‎ext/phar/func_interceptors.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,7 @@ void phar_intercept_functions_shutdown(void)
893893
PHAR_RELEASE(fopen);
894894
PHAR_RELEASE(file_get_contents);
895895
PHAR_RELEASE(is_file);
896+
PHAR_RELEASE(is_link);
896897
PHAR_RELEASE(is_dir);
897898
PHAR_RELEASE(opendir);
898899
PHAR_RELEASE(file_exists);

‎ext/phar/phar_object.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,9 @@ PHP_METHOD(Phar, webPhar)
639639

640640
} else {
641641
char *testit = sapi_getenv("SCRIPT_NAME", sizeof("SCRIPT_NAME")-1);
642+
if (!testit) {
643+
goto finish;
644+
}
642645
if (!(pt = strstr(testit, basename))) {
643646
efree(testit);
644647
goto finish;
@@ -3513,6 +3516,11 @@ PHP_METHOD(Phar, offsetGet)
35133516
if (!(entry = phar_get_entry_info_dir(phar_obj->archive, ZSTR_VAL(file_name), ZSTR_LEN(file_name), 1, &error, false))) {
35143517
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s does not exist%s%s", ZSTR_VAL(file_name), error?", ":"", error?error:"");
35153518
} else {
3519+
if (entry->is_temp_dir) {
3520+
zend_string_efree(entry->filename);
3521+
efree(entry);
3522+
}
3523+
35163524
if (zend_string_equals_literal(file_name, ".phar/stub.php")) {
35173525
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot get stub \".phar/stub.php\" directly in phar \"%s\", use getStub", phar_obj->archive->fname);
35183526
RETURN_THROWS();
@@ -3528,11 +3536,6 @@ PHP_METHOD(Phar, offsetGet)
35283536
RETURN_THROWS();
35293537
}
35303538

3531-
if (entry->is_temp_dir) {
3532-
zend_string_efree(entry->filename);
3533-
efree(entry);
3534-
}
3535-
35363539
zend_string *sfname = strpprintf(0, "phar://%s/%s", phar_obj->archive->fname, ZSTR_VAL(file_name));
35373540
zval zfname;
35383541
ZVAL_NEW_STR(&zfname, sfname);
@@ -3603,11 +3606,13 @@ static void phar_add_file(phar_archive_data **pphar, zend_string *file_name, con
36033606
size_t written_len = php_stream_write(data->fp, ZSTR_VAL(content), ZSTR_LEN(content));
36043607
if (written_len != contents_len) {
36053608
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
3609+
phar_entry_delref(data);
36063610
goto finish;
36073611
}
36083612
} else {
36093613
if (!(php_stream_from_zval_no_verify(contents_file, zresource))) {
36103614
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
3615+
phar_entry_delref(data);
36113616
goto finish;
36123617
}
36133618
php_stream_copy_to_stream_ex(contents_file, data->fp, PHP_STREAM_COPY_ALL, &contents_len);

‎ext/phar/stream.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,11 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
351351
static int phar_stream_close(php_stream *stream, int close_handle) /* {{{ */
352352
{
353353
/* for some reasons phar needs to be flushed even if there is no write going on */
354-
phar_stream_flush(stream);
354+
int ret = phar_stream_flush(stream);
355355

356356
phar_entry_delref((phar_entry_data *)stream->abstract);
357357

358-
return 0;
358+
return ret;
359359
}
360360
/* }}} */
361361

‎ext/phar/tests/gh21797.phpt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
GH-21797: Phar::webPhar() NULL dereference when SCRIPT_NAME absent from SAPI environment
3+
--CGI--
4+
--EXTENSIONS--
5+
phar
6+
--INI--
7+
phar.readonly=0
8+
phar.require_hash=0
9+
variables_order=EGPC
10+
register_argc_argv=0
11+
cgi.fix_pathinfo=0
12+
--ENV--
13+
REQUEST_METHOD=GET
14+
PATH_INFO=/gh21797.phar
15+
--FILE--
16+
<?php
17+
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar';
18+
$phar = new Phar($fname);
19+
$phar->addFromString('index.php', '<?php echo "ok\n"; ?>');
20+
$phar->setStub('<?php
21+
Phar::webPhar();
22+
echo "no crash\n";
23+
__HALT_COMPILER(); ?>');
24+
unset($phar);
25+
include $fname;
26+
?>
27+
--CLEAN--
28+
<?php @unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar'); ?>
29+
--EXPECT--
30+
no crash

‎ext/phar/tests/gh21798-add-file-delref.phpt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-21798: phar_add_file must call phar_entry_delref on write error paths
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.readonly=0
7+
phar.require_hash=0
8+
--FILE--
9+
<?php
10+
// Regression baseline: verify addFile and addFromString succeed and
11+
// phar_entry_delref is not skipped on the happy path.
12+
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar';
13+
14+
$phar = new Phar($fname);
15+
$phar->addFromString('hello.txt', 'hello world');
16+
$phar->addFromString('empty.txt', '');
17+
unset($phar);
18+
19+
$phar = new Phar($fname);
20+
echo $phar['hello.txt']->getContent() . "\n";
21+
echo ($phar->offsetExists('empty.txt') ? 'empty exists' : 'missing') . "\n";
22+
echo "no crash\n";
23+
?>
24+
--CLEAN--
25+
<?php @unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar'); ?>
26+
--EXPECT--
27+
hello world
28+
empty exists
29+
no crash

‎ext/phar/tests/gh21798-offsetget-temp-entry.phpt‎

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

‎ext/phar/tests/gh21799-stream-close-flush.phpt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-21799: phar_stream_close propagates phar_stream_flush return value
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.readonly=0
7+
phar.require_hash=0
8+
--FILE--
9+
<?php
10+
// Regression baseline: fclose() on a phar file handle succeeds on the
11+
// normal code path. phar_stream_close now returns the phar_stream_flush
12+
// result instead of always returning 0.
13+
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar';
14+
15+
$phar = new Phar($fname);
16+
$phar->addFromString('hello.txt', 'hello');
17+
unset($phar);
18+
19+
$fp = fopen('phar://' . $fname . '/hello.txt', 'rb');
20+
$content = fread($fp, 1024);
21+
$result = fclose($fp);
22+
23+
echo $content . "\n";
24+
var_dump($result);
25+
echo "no crash\n";
26+
?>
27+
--CLEAN--
28+
<?php @unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar'); ?>
29+
--EXPECT--
30+
hello
31+
bool(true)
32+
no crash

‎ext/phar/tests/phar-is-link-intercept.phpt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
phar: is_link() intercept correctly delegates for non-symlink phar entries
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('file.txt', 'hello');
13+
$phar->setStub('<?php
14+
Phar::interceptFileFuncs();
15+
echo "regular entry (not a symlink): ";
16+
var_dump(is_link("file.txt"));
17+
echo "missing entry: ";
18+
var_dump(is_link("nonexistent.txt"));
19+
echo "absolute phar:// path (bypasses intercept): ";
20+
var_dump(is_link("phar://" . __FILE__ . "/file.txt"));
21+
__HALT_COMPILER(); ?>');
22+
include $fname;
23+
?>
24+
--CLEAN--
25+
<?php @unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar'); ?>
26+
--EXPECT--
27+
regular entry (not a symlink): bool(false)
28+
missing entry: bool(false)
29+
absolute phar:// path (bypasses intercept): bool(false)

‎ext/phar/util.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,6 +1576,7 @@ zend_result phar_verify_signature(php_stream *fp, size_t end_of_phar, uint32_t s
15761576
if (md_ctx) {
15771577
EVP_MD_CTX_destroy(md_ctx);
15781578
}
1579+
EVP_PKEY_free(key);
15791580
if (error) {
15801581
*error = estrdup("openssl signature could not be verified");
15811582
}

0 commit comments

Comments
 (0)