From 72c0222926ab81076e2256fd461462152988e7ca Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 4 Nov 2024 21:18:37 +0100 Subject: [PATCH] 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. Closes GH-16700. --- NEWS | 4 ++++ ext/phar/phar.c | 2 +- ext/phar/tar.c | 10 +++++++--- ext/phar/tests/tar/gh16695_1.phpt | 28 ++++++++++++++++++++++++++++ ext/phar/tests/tar/gh16695_2.phpt | 26 ++++++++++++++++++++++++++ ext/phar/tests/tar/gh16695_3.phpt | 26 ++++++++++++++++++++++++++ 6 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 ext/phar/tests/tar/gh16695_1.phpt create mode 100644 ext/phar/tests/tar/gh16695_2.phpt create mode 100644 ext/phar/tests/tar/gh16695_3.phpt diff --git a/NEWS b/NEWS index c441f4c25e3..6e9e23670a1 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,10 @@ PHP NEWS - PDO: . Fixed memory leak of `setFetchMode()`. (SakiTakamachi) +- Phar: + . Fixed bug GH-16695 (phar:// tar parser and zero-length file header blocks). + (nielsdos) + 21 Nov 2024, PHP 8.2.26 - Cli: diff --git a/ext/phar/phar.c b/ext/phar/phar.c index e3d6ea74c61..01a3d546254 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -1774,7 +1774,7 @@ static int phar_open_from_fp(php_stream* fp, char *fname, size_t fname_len, char return phar_parse_zipfile(fp, fname, fname_len, alias, alias_len, pphar, error); } - if (got > 512) { + if (got >= 512) { if (phar_is_tar(pos, fname)) { php_stream_rewind(fp); return phar_parse_tarfile(fp, fname, fname_len, alias, alias_len, pphar, is_data, compression, error); diff --git a/ext/phar/tar.c b/ext/phar/tar.c index 60e248c78df..652062679d7 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -254,9 +254,8 @@ int phar_parse_tarfile(php_stream* fp, char *fname, size_t fname_len, char *alia entry.is_tar = 1; entry.is_crc_checked = 1; entry.phar = myphar; - pos += sizeof(buf); - do { + while (true) { phar_entry_info *newentry; pos = php_stream_tell(fp); @@ -597,6 +596,11 @@ next: } } + /* Only read next header if we're not yet at the end */ + if (php_stream_tell(fp) == totalsize) { + break; + } + read = php_stream_read(fp, buf, sizeof(buf)); if (read != sizeof(buf)) { @@ -607,7 +611,7 @@ next: phar_destroy_phar_data(myphar); return FAILURE; } - } while (!php_stream_eof(fp)); + } if (zend_hash_str_exists(&(myphar->manifest), ".phar/stub.php", sizeof(".phar/stub.php")-1)) { myphar->is_data = 0; diff --git a/ext/phar/tests/tar/gh16695_1.phpt b/ext/phar/tests/tar/gh16695_1.phpt new file mode 100644 index 00000000000..8ce82bcf28d --- /dev/null +++ b/ext/phar/tests/tar/gh16695_1.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-16695 (phar:// tar parser and zero-length file header blocks) +--CREDITS-- +hakre +--EXTENSIONS-- +phar +--INI-- +phar.require_hash=0 +--FILE-- + +--CLEAN-- + +--EXPECTF-- +int(512) + +Warning: file_get_contents(%stls): Failed to open stream: phar error: path "tls" is a directory in %s on line %d +bool(false) diff --git a/ext/phar/tests/tar/gh16695_2.phpt b/ext/phar/tests/tar/gh16695_2.phpt new file mode 100644 index 00000000000..5b7200398c4 --- /dev/null +++ b/ext/phar/tests/tar/gh16695_2.phpt @@ -0,0 +1,26 @@ +--TEST-- +GH-16695 (phar:// tar parser and zero-length file header blocks) +--CREDITS-- +hakre +--EXTENSIONS-- +phar +--INI-- +phar.require_hash=0 +--FILE-- + +--CLEAN-- + +--EXPECT-- +int(1024) +string(122) "{"Name":"default","Metadata":{},"Endpoints":{"docker":{"Host":"unix:///run/user/1000/docker.sock","SkipTLSVerify":false}}}" diff --git a/ext/phar/tests/tar/gh16695_3.phpt b/ext/phar/tests/tar/gh16695_3.phpt new file mode 100644 index 00000000000..eddf697b013 --- /dev/null +++ b/ext/phar/tests/tar/gh16695_3.phpt @@ -0,0 +1,26 @@ +--TEST-- +GH-16695 (phar:// tar parser and zero-length file header blocks) +--CREDITS-- +hakre +--EXTENSIONS-- +phar +--INI-- +phar.require_hash=0 +--FILE-- + +--CLEAN-- + +--EXPECT-- +int(512) +string(0) ""