From f35702b215ddd4c05a70999571a41a1c6c37a999 Mon Sep 17 00:00:00 2001 From: Alessio Attilio Date: Sun, 25 Oct 2026 16:11:02 +0100 Subject: [PATCH 1/3] Fix GH-24170: read concatenated bzip2 streams --- NEWS | 4 + UPGRADING | 5 ++ ext/bz2/bz2.c | 113 ++++++++++++++++++++++++-- ext/bz2/tests/gh24170.phpt | 89 ++++++++++++++++++++ ext/bz2/tests/gh24170_boundaries.phpt | 41 ++++++++++ ext/bz2/tests/gh24170_errors.phpt | 53 ++++++++++++ 6 files changed, 298 insertions(+), 7 deletions(-) create mode 100644 ext/bz2/tests/gh24170.phpt create mode 100644 ext/bz2/tests/gh24170_boundaries.phpt create mode 100644 ext/bz2/tests/gh24170_errors.phpt diff --git a/NEWS b/NEWS index 86cb13963862..80f2bed14fdd 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.7.0alpha1 +- Bz2: + . Fixed GH-24170 (compress.bzip2:// truncates concatenated bzip2 streams). + (Alessio Attilio) + - Core: . A flat unkeyed list assignment from an array literal with one value per target, such as [$a, $b] = [$b, $a], no longer builds the array when the diff --git a/UPGRADING b/UPGRADING index 49d1ebc8258c..1cea53fb9439 100644 --- a/UPGRADING +++ b/UPGRADING @@ -19,6 +19,11 @@ PHP 8.7 UPGRADE NOTES 1. Backward Incompatible Changes ======================================== +- Bz2: + . The compress.bzip2:// wrapper and bzopen() now read all concatenated bzip2 + streams instead of stopping after the first stream. Trailing data without + a bzip2 header is still ignored. + - Curl: . curl_multi_exec() now throws a TypeError when still_running is not an int. . Nested arrays in CURLOPT_POSTFIELDS now throw a TypeError, instead of being diff --git a/ext/bz2/bz2.c b/ext/bz2/bz2.c index dd1b87d2bf61..6f761626ece3 100644 --- a/ext/bz2/bz2.c +++ b/ext/bz2/bz2.c @@ -57,11 +57,54 @@ ZEND_GET_MODULE(bz2) struct php_bz2_stream_data_t { BZFILE *bz_file; + FILE *file; + bool stream_end; + bool read_failed; zend_resource *stream_res; }; /* {{{ BZip2 stream implementation */ +static bool php_bz2iop_next_stream(php_stream *stream) +{ + struct php_bz2_stream_data_t *self = stream->abstract; + char buffer[BZ_MAX_UNUSED]; + void *unused; + int unused_len, error; + + BZ2_bzReadGetUnused(&error, self->bz_file, &unused, &unused_len); + if (error != BZ_OK) { + self->read_failed = true; + stream->eof = 1; + return false; + } + memcpy(buffer, unused, unused_len); + while (unused_len < 4) { + int ch = fgetc(self->file); + if (ch == EOF) { + self->read_failed = ferror(self->file) != 0; + break; + } + buffer[unused_len++] = ch; + } + if (unused_len < 4 || memcmp(buffer, "BZh", 3) != 0 + || buffer[3] < '1' || buffer[3] > '9') { + stream->eof = 1; + return false; + } + + BZFILE *next = BZ2_bzReadOpen(&error, self->file, 0, strchr(stream->mode, 's') != NULL, buffer, unused_len); + if (error != BZ_OK) { + self->read_failed = true; + stream->eof = 1; + return false; + } + BZ2_bzReadClose(&error, self->bz_file); + self->bz_file = next; + self->stream_end = false; + return true; +} + static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count) { struct php_bz2_stream_data_t *self = (struct php_bz2_stream_data_t *)stream->abstract; @@ -72,12 +115,30 @@ static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count) size_t remain = count - ret; int to_read = (int)(remain <= INT_MAX ? remain : INT_MAX); - just_read = BZ2_bzread(self->bz_file, buf, to_read); + if (stream->eof) { + return ret ? (ssize_t) ret : (self->read_failed ? -1 : 0); + } + if (self->file) { + int error; + if (self->stream_end && !php_bz2iop_next_stream(stream)) { + return ret ? (ssize_t) ret : (self->read_failed ? -1 : 0); + } + just_read = BZ2_bzRead(&error, self->bz_file, buf + ret, to_read); + self->stream_end = error == BZ_STREAM_END; + if (error != BZ_OK && error != BZ_STREAM_END) { + just_read = -1; + } else if (just_read == 0 && self->stream_end) { + continue; + } + } else { + just_read = BZ2_bzread(self->bz_file, buf + ret, to_read); + } if (just_read < 1) { /* it is not safe to keep reading after an error, see #72613 */ stream->eof = 1; if (just_read < 0) { + self->read_failed = true; if (ret) { return ret; } @@ -126,7 +187,13 @@ static int php_bz2iop_close(php_stream *stream, int close_handle) int ret = EOF; if (close_handle) { - BZ2_bzclose(self->bz_file); + if (self->file) { + int error; + BZ2_bzReadClose(&error, self->bz_file); + fclose(self->file); + } else { + BZ2_bzclose(self->bz_file); + } } /* The inner stream may have been closed by the user, only its resource is held */ @@ -164,14 +231,32 @@ const php_stream_ops php_stream_bz2io_ops = { /* {{{ Bzip2 stream openers */ -/* bzlib closes the descriptor it is given, so it gets a duplicate */ -static BZFILE *php_bz2_bzdopen(php_socket_t fd, const char *mode) +static BZFILE *php_bz2_readopen(FILE *file, const char *mode) +{ + int error; + BZFILE *bz = BZ2_bzReadOpen(&error, file, 0, strchr(mode, 's') != NULL, NULL, 0); + if (!bz) { + fclose(file); + } + return bz; +} + +static BZFILE *php_bz2_bzdopen(php_socket_t fd, const char *mode, FILE **file) { int dup_fd = dup((int) fd); if (dup_fd == -1) { return NULL; } + if (mode[0] == 'r') { + *file = fdopen(dup_fd, "rb"); + if (!*file) { + close(dup_fd); + return NULL; + } + return php_bz2_readopen(*file, mode); + } + BZFILE *bz = BZ2_bzdopen(dup_fd, mode); if (!bz) { close(dup_fd); @@ -193,6 +278,9 @@ PHP_BZ2_API php_stream *_php_stream_bz2open_from_BZFILE(BZFILE *bz, GC_ADDREF(innerstream->res); } self->bz_file = bz; + self->file = NULL; + self->stream_end = false; + self->read_failed = false; return php_stream_alloc_rel(&php_stream_bz2io_ops, self, 0, mode); } @@ -207,6 +295,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, php_stream *retstream = NULL, *stream = NULL; char *path_copy = NULL; BZFILE *bz_file = NULL; + FILE *file = NULL; if (strncasecmp("compress.bzip2://", path, 17) == 0) { path += 17; @@ -229,7 +318,14 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, } /* try and open it directly first */ - bz_file = BZ2_bzopen(path_copy, mode); + if (mode[0] == 'r') { + file = VCWD_FOPEN(path_copy, "rb"); + if (file) { + bz_file = php_bz2_readopen(file, mode); + } + } else { + bz_file = BZ2_bzopen(path_copy, mode); + } if (opened_path && bz_file) { *opened_path = zend_string_init(path_copy, strlen(path_copy), 0); @@ -246,7 +342,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, if (stream) { php_socket_t fd; if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD, (void **) &fd, REPORT_ERRORS)) { - bz_file = php_bz2_bzdopen(fd, mode); + bz_file = php_bz2_bzdopen(fd, mode, &file); } } @@ -261,6 +357,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, if (bz_file) { retstream = _php_stream_bz2open_from_BZFILE(bz_file, mode, stream STREAMS_REL_CC); if (retstream) { + ((struct php_bz2_stream_data_t *) retstream->abstract)->file = file; return retstream; } @@ -358,6 +455,7 @@ PHP_FUNCTION(bzopen) BZFILE *bz; /* The compressed file stream */ php_stream *stream = NULL; + FILE *handle = NULL; if (zend_parse_parameters(ZEND_NUM_ARGS(), "zs", &file, &mode, &mode_len) == FAILURE) { RETURN_THROWS(); @@ -432,12 +530,13 @@ PHP_FUNCTION(bzopen) RETURN_FALSE; } - bz = php_bz2_bzdopen(fd, mode); + bz = php_bz2_bzdopen(fd, mode, &handle); if (!bz) { RETURN_FALSE; } stream = php_stream_bz2open_from_BZFILE(bz, mode, stream); + ((struct php_bz2_stream_data_t *) stream->abstract)->file = handle; } else { zend_argument_type_error(1, "must be of type string or file-resource, %s given", zend_zval_value_name(file)); RETURN_THROWS(); diff --git a/ext/bz2/tests/gh24170.phpt b/ext/bz2/tests/gh24170.phpt new file mode 100644 index 000000000000..05578696e5c9 --- /dev/null +++ b/ext/bz2/tests/gh24170.phpt @@ -0,0 +1,89 @@ +--TEST-- +GH-24170 (bzip2 streams read all concatenated members) +--EXTENSIONS-- +bz2 +--FILE-- + $part) { + $compressed .= bzcompress($part, $i + 1); +} +file_put_contents($file, $compressed); +$expected = implode('', $parts); +var_dump(file_get_contents('compress.bzip2://' . $file) === $expected); +$copy = $file . '.out'; +var_dump(copy('compress.bzip2://' . $file, $copy)); +var_dump(file_get_contents($copy) === $expected); +unlink($copy); + +foreach ([1, 17, 8192, 900000, 1000000] as $size) { + $fp = fopen('compress.bzip2://' . $file, 'rb'); + $offset = 0; + $matches = true; + while (!feof($fp)) { + $chunk = fread($fp, $size); + $matches = $matches && $chunk === substr($expected, $offset, strlen($chunk)); + $offset += strlen($chunk); + } + var_dump($matches && $offset === strlen($expected)); + var_dump(fread($fp, 1)); + fclose($fp); +} + +foreach ([false, true] as $resource) { + $inner = $resource ? fopen($file, 'rb') : null; + $fp = bzopen($inner ?? $file, 'r'); + if ($inner) { + fclose($inner); + } + $result = ''; + while (!feof($fp)) { + $result .= bzread($fp, 8192); + } + var_dump($result === $expected); + var_dump(bzerrno($fp)); + var_dump(bzread($fp, 1)); + bzclose($fp); +} + +file_put_contents($file, str_repeat(bzcompress(''), 400) . bzcompress('end')); +var_dump(file_get_contents('compress.bzip2://' . $file)); +file_put_contents($file, str_repeat(bzcompress(''), 400)); +var_dump(file_get_contents('compress.bzip2://' . $file)); +unlink($file); +?> +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +int(0) +string(0) "" +bool(true) +int(0) +string(0) "" +string(3) "end" +string(0) "" diff --git a/ext/bz2/tests/gh24170_boundaries.phpt b/ext/bz2/tests/gh24170_boundaries.phpt new file mode 100644 index 000000000000..9d6b79dc14ac --- /dev/null +++ b/ext/bz2/tests/gh24170_boundaries.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-24170 (concatenated bzip2 headers split across the compressed input buffer) +--EXTENSIONS-- +bz2 +--FILE-- + +--CLEAN-- + +--EXPECT-- +int(4997) +bool(true) +bool(true) +int(4998) +bool(true) +bool(true) +int(4999) +bool(true) +bool(true) +int(5000) +bool(true) +bool(true) diff --git a/ext/bz2/tests/gh24170_errors.phpt b/ext/bz2/tests/gh24170_errors.phpt new file mode 100644 index 000000000000..b17f1d3dc447 --- /dev/null +++ b/ext/bz2/tests/gh24170_errors.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-24170 (concatenated bzip2 streams handle trailing data and damaged members) +--EXTENSIONS-- +bz2 +--FILE-- + +--CLEAN-- + +--EXPECT-- +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +bool(false) +bool(true) +bool(true) +bool(false) +string(4) "data" +bool(false) +bool(true) +bool(true) +bool(false) +string(4) "data" +bool(false) +bool(true) +bool(true) +bool(false) From 915999cee9b705d4cb8a05bb3761d63fe6a1ebd2 Mon Sep 17 00:00:00 2001 From: Alessio Attilio Date: Fri, 30 Oct 2026 21:45:54 +0100 Subject: [PATCH 2/3] Clarify bzdecompress concatenated stream behavior --- UPGRADING | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/UPGRADING b/UPGRADING index 1cea53fb9439..dbcd02184c18 100644 --- a/UPGRADING +++ b/UPGRADING @@ -21,8 +21,9 @@ PHP 8.7 UPGRADE NOTES - Bz2: . The compress.bzip2:// wrapper and bzopen() now read all concatenated bzip2 - streams instead of stopping after the first stream. Trailing data without - a bzip2 header is still ignored. + streams instead of stopping after the first stream. bzdecompress() still + decompresses only the first stream. Trailing data without a bzip2 header is + still ignored. - Curl: . curl_multi_exec() now throws a TypeError when still_running is not an int. From e8dba59a0bfa22e8f46cbd18f826d8a6f878dc76 Mon Sep 17 00:00:00 2001 From: Alessio Attilio Date: Mon, 2 Nov 2026 22:09:59 +0100 Subject: [PATCH 3/3] Report errors reading damaged bzip2 members --- ext/bz2/bz2.c | 37 +++++++++++++++++++++++++------ ext/bz2/tests/gh24170_errors.phpt | 13 +++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/ext/bz2/bz2.c b/ext/bz2/bz2.c index 6f761626ece3..71d1495370f4 100644 --- a/ext/bz2/bz2.c +++ b/ext/bz2/bz2.c @@ -60,11 +60,25 @@ struct php_bz2_stream_data_t { FILE *file; bool stream_end; bool read_failed; + bool report_read_errors; + bool read_error_reported; zend_resource *stream_res; }; /* {{{ BZip2 stream implementation */ +static void php_bz2iop_report_read_error(php_stream *stream) +{ + struct php_bz2_stream_data_t *self = stream->abstract; + + self->read_failed = true; + stream->eof = 1; + if (self->report_read_errors && !self->read_error_reported) { + self->read_error_reported = true; + php_error_docref(NULL, E_WARNING, "Failed to read bzip2 stream"); + } +} + static bool php_bz2iop_next_stream(php_stream *stream) { struct php_bz2_stream_data_t *self = stream->abstract; @@ -74,8 +88,7 @@ static bool php_bz2iop_next_stream(php_stream *stream) BZ2_bzReadGetUnused(&error, self->bz_file, &unused, &unused_len); if (error != BZ_OK) { - self->read_failed = true; - stream->eof = 1; + php_bz2iop_report_read_error(stream); return false; } memcpy(buffer, unused, unused_len); @@ -87,6 +100,10 @@ static bool php_bz2iop_next_stream(php_stream *stream) } buffer[unused_len++] = ch; } + if (self->read_failed) { + php_bz2iop_report_read_error(stream); + return false; + } if (unused_len < 4 || memcmp(buffer, "BZh", 3) != 0 || buffer[3] < '1' || buffer[3] > '9') { stream->eof = 1; @@ -95,8 +112,7 @@ static bool php_bz2iop_next_stream(php_stream *stream) BZFILE *next = BZ2_bzReadOpen(&error, self->file, 0, strchr(stream->mode, 's') != NULL, buffer, unused_len); if (error != BZ_OK) { - self->read_failed = true; - stream->eof = 1; + php_bz2iop_report_read_error(stream); return false; } BZ2_bzReadClose(&error, self->bz_file); @@ -115,6 +131,9 @@ static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count) size_t remain = count - ret; int to_read = (int)(remain <= INT_MAX ? remain : INT_MAX); + if (self->read_failed) { + return ret ? (ssize_t) ret : -1; + } if (stream->eof) { return ret ? (ssize_t) ret : (self->read_failed ? -1 : 0); } @@ -136,14 +155,14 @@ static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count) if (just_read < 1) { /* it is not safe to keep reading after an error, see #72613 */ - stream->eof = 1; if (just_read < 0) { - self->read_failed = true; + php_bz2iop_report_read_error(stream); if (ret) { return ret; } return -1; } + stream->eof = 1; break; } @@ -281,6 +300,8 @@ PHP_BZ2_API php_stream *_php_stream_bz2open_from_BZFILE(BZFILE *bz, self->file = NULL; self->stream_end = false; self->read_failed = false; + self->report_read_errors = false; + self->read_error_reported = false; return php_stream_alloc_rel(&php_stream_bz2io_ops, self, 0, mode); } @@ -357,7 +378,9 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, if (bz_file) { retstream = _php_stream_bz2open_from_BZFILE(bz_file, mode, stream STREAMS_REL_CC); if (retstream) { - ((struct php_bz2_stream_data_t *) retstream->abstract)->file = file; + struct php_bz2_stream_data_t *self = retstream->abstract; + self->file = file; + self->report_read_errors = wrapper != NULL; return retstream; } diff --git a/ext/bz2/tests/gh24170_errors.phpt b/ext/bz2/tests/gh24170_errors.phpt index b17f1d3dc447..f1380d370b06 100644 --- a/ext/bz2/tests/gh24170_errors.phpt +++ b/ext/bz2/tests/gh24170_errors.phpt @@ -22,6 +22,17 @@ foreach (['BZh9', substr(bzcompress('broken'), 0, -1), $damaged] as $suffix) { var_dump(bzread($fp, 8192)); bzclose($fp); } + +file_put_contents($file, bzcompress('data') . $damaged); +$warning = false; +set_error_handler(function () use (&$warning) { + $warning = true; + return true; +}); +$result = file_get_contents('compress.bzip2://' . $file); +restore_error_handler(); +var_dump($result); +var_dump($warning); unlink($file); ?> --CLEAN-- @@ -51,3 +62,5 @@ bool(false) bool(true) bool(true) bool(false) +string(4) "data" +bool(true)