Skip to content

Commit bf6cba4

Browse files
committed
Fix GH-16695: phar:// tar parser and zero-length file header blocks
There are two issues: 1) There's an off-by-one in the check for the minimum file size for a tar (i.e. `>` instead of `>=`). 2) The loop in the tar parsing parses a header, and then unconditionally reads the next one. However, that doesn't necessarily exist. Instead, we remove the loop condition and check for the end of the file before reading the next header. Note that we can't use php_stream_eof as the flag may not be set yet when we're already at the end.
1 parent 5ddb756 commit bf6cba4

File tree

5 files changed

+88
-4
lines changed

5 files changed

+88
-4
lines changed

ext/phar/phar.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1774,7 +1774,7 @@ static int phar_open_from_fp(php_stream* fp, char *fname, size_t fname_len, char
17741774
return phar_parse_zipfile(fp, fname, fname_len, alias, alias_len, pphar, error);
17751775
}
17761776

1777-
if (got > 512) {
1777+
if (got >= 512) {
17781778
if (phar_is_tar(pos, fname)) {
17791779
php_stream_rewind(fp);
17801780
return phar_parse_tarfile(fp, fname, fname_len, alias, alias_len, pphar, is_data, compression, error);

ext/phar/tar.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,8 @@ int phar_parse_tarfile(php_stream* fp, char *fname, size_t fname_len, char *alia
254254
entry.is_tar = 1;
255255
entry.is_crc_checked = 1;
256256
entry.phar = myphar;
257-
pos += sizeof(buf);
258257

259-
do {
258+
while (true) {
260259
phar_entry_info *newentry;
261260

262261
pos = php_stream_tell(fp);
@@ -597,6 +596,11 @@ int phar_parse_tarfile(php_stream* fp, char *fname, size_t fname_len, char *alia
597596
}
598597
}
599598

599+
/* Only read next header if we're not yet at the end */
600+
if (php_stream_tell(fp) == totalsize) {
601+
break;
602+
}
603+
600604
read = php_stream_read(fp, buf, sizeof(buf));
601605

602606
if (read != sizeof(buf)) {
@@ -607,7 +611,7 @@ int phar_parse_tarfile(php_stream* fp, char *fname, size_t fname_len, char *alia
607611
phar_destroy_phar_data(myphar);
608612
return FAILURE;
609613
}
610-
} while (!php_stream_eof(fp));
614+
}
611615

612616
if (zend_hash_str_exists(&(myphar->manifest), ".phar/stub.php", sizeof(".phar/stub.php")-1)) {
613617
myphar->is_data = 0;

ext/phar/tests/tar/gh16695_1.phpt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
GH-16695 (phar:// tar parser and zero-length file header blocks)
3+
--CREDITS--
4+
hakre
5+
--EXTENSIONS--
6+
phar
7+
--INI--
8+
phar.require_hash=0
9+
--FILE--
10+
<?php
11+
12+
$reportTar = __DIR__.'/gh16695_1.tmp';
13+
14+
$length = file_put_contents($reportTar, base64_decode('dGxzAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAwMDA3MDAAMDAwMDAwMAAwMDAwMDAwADAwMDAwMDAwMDAwADAwMDAwMDAwMDAwADAwNzcxNgAgNQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwADAwMDAwMDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='));
15+
var_dump($length);
16+
$buffer = file_get_contents("phar://$reportTar/tls");
17+
var_dump($buffer);
18+
19+
?>
20+
--CLEAN--
21+
<?php
22+
@unlink(__DIR__.'/gh16695_1.tmp');
23+
?>
24+
--EXPECTF--
25+
int(512)
26+
27+
Warning: file_get_contents(%stls): Failed to open stream: phar error: path "tls" is a directory in %s on line %d
28+
bool(false)

ext/phar/tests/tar/gh16695_2.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-16695 (phar:// tar parser and zero-length file header blocks)
3+
--CREDITS--
4+
hakre
5+
--EXTENSIONS--
6+
phar
7+
--INI--
8+
phar.require_hash=0
9+
--FILE--
10+
<?php
11+
12+
$reportTar = __DIR__.'/gh16695_2.tmp';
13+
14+
$length = file_put_contents($reportTar, base64_decode('bWV0YS5qc29uAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAwMDA2NDQAMDAwMDAwMAAwMDAwMDAwADAwMDAwMDAwMTcyADAwMDAwMDAwMDAwADAxMTAyNgAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwADAwMDAwMDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB7Ik5hbWUiOiJkZWZhdWx0IiwiTWV0YWRhdGEiOnt9LCJFbmRwb2ludHMiOnsiZG9ja2VyIjp7Ikhvc3QiOiJ1bml4Oi8vL3J1bi91c2VyLzEwMDAvZG9ja2VyLnNvY2siLCJTa2lwVExTVmVyaWZ5IjpmYWxzZX19fQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=='));
15+
var_dump($length);
16+
$buffer = file_get_contents("phar://$reportTar/meta.json");
17+
var_dump($buffer);
18+
19+
?>
20+
--CLEAN--
21+
<?php
22+
@unlink(__DIR__.'/gh16695_2.tmp');
23+
?>
24+
--EXPECT--
25+
int(1024)
26+
string(122) "{"Name":"default","Metadata":{},"Endpoints":{"docker":{"Host":"unix:///run/user/1000/docker.sock","SkipTLSVerify":false}}}"

ext/phar/tests/tar/gh16695_3.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-16695 (phar:// tar parser and zero-length file header blocks)
3+
--CREDITS--
4+
hakre
5+
--EXTENSIONS--
6+
phar
7+
--INI--
8+
phar.require_hash=0
9+
--FILE--
10+
<?php
11+
12+
$reportTar = __DIR__.'/gh16695_3.tmp';
13+
14+
$length = file_put_contents($reportTar, base64_decode('dGxzAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAwMDA3MDAAMDAwMDAwMAAwMDAwMDAwADAwMDAwMDAwMDAwADAwMDAwMDAwMDAwADAwNzcxMQAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwADAwMDAwMDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='));
15+
var_dump($length);
16+
$buffer = file_get_contents("phar://$reportTar/tls");
17+
var_dump($buffer);
18+
19+
?>
20+
--CLEAN--
21+
<?php
22+
@unlink(__DIR__.'/gh16695_3.tmp');
23+
?>
24+
--EXPECT--
25+
int(512)
26+
string(0) ""

0 commit comments

Comments
 (0)