Skip to content

Commit 786356d

Browse files
GirgiasGirgias
committed
Merge branch 'PHP-8.4' into PHP-8.5
* PHP-8.4: 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 3bb8072 + 3df1fa7 commit 786356d

File tree

10 files changed

+184
-7
lines changed

10 files changed

+184
-7
lines changed

‎NEWS‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ PHP NEWS
4949
. Fixed bug GH-21683 (pdo_pgsql throws with ATTR_PREFETCH=0
5050
on empty result set). (thomasschiet)
5151

52+
- Phar:
53+
. Restore is_link handler in phar_intercept_functions_shutdown. (iliaal)
54+
. Fixed bug GH-21797 (phar: NULL dereference in Phar::webPhar() when
55+
SCRIPT_NAME is absent from SAPI environment). (iliaal)
56+
. Fix memory leak in Phar::offsetGet(). (iliaal)
57+
. Fix memory leak in phar_add_file(). (iliaal)
58+
. Fixed bug GH-21799 (phar: propagate phar_stream_flush return value from
59+
phar_stream_close). (iliaal)
60+
. Fix memory leak in phar_verify_signature() when md_ctx is invalid.
61+
(JarneClauw)
62+
5263
- Random:
5364
. Fixed bug GH-21731 (Random\Engine\Xoshiro256StarStar::__unserialize()
5465
accepts all-zero state). (iliaal)

‎ext/phar/func_interceptors.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,7 @@ void phar_intercept_functions_shutdown(void)
932932
PHAR_RELEASE(fopen);
933933
PHAR_RELEASE(file_get_contents);
934934
PHAR_RELEASE(is_file);
935+
PHAR_RELEASE(is_link);
935936
PHAR_RELEASE(is_dir);
936937
PHAR_RELEASE(opendir);
937938
PHAR_RELEASE(file_exists);

‎ext/phar/phar_object.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,9 @@ PHP_METHOD(Phar, webPhar)
649649
char *testit;
650650

651651
testit = sapi_getenv("SCRIPT_NAME", sizeof("SCRIPT_NAME")-1);
652+
if (!testit) {
653+
goto finish;
654+
}
652655
if (!(pt = strstr(testit, basename))) {
653656
efree(testit);
654657
goto finish;
@@ -3590,6 +3593,11 @@ PHP_METHOD(Phar, offsetGet)
35903593
if (!(entry = phar_get_entry_info_dir(phar_obj->archive, ZSTR_VAL(file_name), ZSTR_LEN(file_name), 1, &error, 0))) {
35913594
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s does not exist%s%s", ZSTR_VAL(file_name), error?", ":"", error?error:"");
35923595
} else {
3596+
if (entry->is_temp_dir) {
3597+
zend_string_efree(entry->filename);
3598+
efree(entry);
3599+
}
3600+
35933601
if (zend_string_equals_literal(file_name, ".phar/stub.php")) {
35943602
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot get stub \".phar/stub.php\" directly in phar \"%s\", use getStub", phar_obj->archive->fname);
35953603
RETURN_THROWS();
@@ -3605,11 +3613,6 @@ PHP_METHOD(Phar, offsetGet)
36053613
RETURN_THROWS();
36063614
}
36073615

3608-
if (entry->is_temp_dir) {
3609-
zend_string_efree(entry->filename);
3610-
efree(entry);
3611-
}
3612-
36133616
zend_string *sfname = strpprintf(0, "phar://%s/%s", phar_obj->archive->fname, ZSTR_VAL(file_name));
36143617
zval zfname;
36153618
ZVAL_NEW_STR(&zfname, sfname);
@@ -3680,11 +3683,13 @@ static void phar_add_file(phar_archive_data **pphar, zend_string *file_name, con
36803683
size_t written_len = php_stream_write(data->fp, ZSTR_VAL(content), ZSTR_LEN(content));
36813684
if (written_len != contents_len) {
36823685
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
3686+
phar_entry_delref(data);
36833687
goto finish;
36843688
}
36853689
} else {
36863690
if (!(php_stream_from_zval_no_verify(contents_file, zresource))) {
36873691
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
3692+
phar_entry_delref(data);
36883693
goto finish;
36893694
}
36903695
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
@@ -353,11 +353,11 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
353353
static int phar_stream_close(php_stream *stream, int close_handle) /* {{{ */
354354
{
355355
/* for some reasons phar needs to be flushed even if there is no write going on */
356-
phar_stream_flush(stream);
356+
int ret = phar_stream_flush(stream);
357357

358358
phar_entry_delref((phar_entry_data *)stream->abstract);
359359

360-
return 0;
360+
return ret;
361361
}
362362
/* }}} */
363363

‎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
@@ -1609,6 +1609,7 @@ zend_result phar_verify_signature(php_stream *fp, size_t end_of_phar, uint32_t s
16091609
if (md_ctx) {
16101610
EVP_MD_CTX_destroy(md_ctx);
16111611
}
1612+
EVP_PKEY_free(key);
16121613
if (error) {
16131614
spprintf(error, 0, "openssl signature could not be verified");
16141615
}

0 commit comments

Comments
 (0)