From 405be1c940af576e18633add4774cfc5f4326657 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 5 Jul 2025 12:07:12 +0200 Subject: [PATCH] Fix phar crash and file corruption with SplFileObject There are two bugfixes here. The first was a crash that I discovered while working on GH-19035. The check for when a file pointer was still occupied was wrong, leading to a UAF. Strangely, zip got this right. The second issue was that even after fixing the first one, the file contents were garbage. This is because the file write offset for the phar stream was wrong. Closes GH-19038. --- NEWS | 1 + ext/phar/phar.c | 2 +- ext/phar/stream.c | 4 ++-- ext/phar/tar.c | 2 +- ext/phar/tests/gh19038.phpt | 25 +++++++++++++++++++++++++ 5 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 ext/phar/tests/gh19038.phpt diff --git a/NEWS b/NEWS index df26a2d9823..84add23a52e 100644 --- a/NEWS +++ b/NEWS @@ -39,6 +39,7 @@ PHP NEWS - Phar: . Fix stream double free in phar. (nielsdos, dixyes) + . Fix phar crash and file corruption with SplFileObject. (nielsdos) - SOAP: . Fixed bug GH-18990, bug #81029, bug #47314 (SOAP HTTP socket not closing diff --git a/ext/phar/phar.c b/ext/phar/phar.c index 125fc847036..ab5460887c3 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -2737,7 +2737,7 @@ int phar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int conv /* remove this from the new phar */ continue; } - if (!entry->is_modified && entry->fp_refcount) { + if (entry->fp_refcount) { /* open file pointers refer to this fp, do not free the stream */ switch (entry->fp_type) { case PHAR_FP: diff --git a/ext/phar/stream.c b/ext/phar/stream.c index fee100cc31a..e68f07bddd0 100644 --- a/ext/phar/stream.c +++ b/ext/phar/stream.c @@ -450,12 +450,12 @@ static ssize_t phar_stream_write(php_stream *stream, const char *buf, size_t cou { phar_entry_data *data = (phar_entry_data *) stream->abstract; - php_stream_seek(data->fp, data->position, SEEK_SET); + php_stream_seek(data->fp, data->position + data->zero, SEEK_SET); if (count != php_stream_write(data->fp, buf, count)) { php_stream_wrapper_log_error(stream->wrapper, stream->flags, "phar error: Could not write %d characters to \"%s\" in phar \"%s\"", (int) count, data->internal_file->filename, data->phar->fname); return -1; } - data->position = php_stream_tell(data->fp); + data->position = php_stream_tell(data->fp) - data->zero; if (data->position > (zend_off_t)data->internal_file->uncompressed_filesize) { data->internal_file->uncompressed_filesize = data->position; } diff --git a/ext/phar/tar.c b/ext/phar/tar.c index e259fb6f3de..38bd52c8609 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -832,7 +832,7 @@ static int phar_tar_writeheaders_int(phar_entry_info *entry, void *argument) /* php_stream_write(fp->new, padding, ((entry->uncompressed_filesize +511)&~511) - entry->uncompressed_filesize); } - if (!entry->is_modified && entry->fp_refcount) { + if (entry->fp_refcount) { /* open file pointers refer to this fp, do not free the stream */ switch (entry->fp_type) { case PHAR_FP: diff --git a/ext/phar/tests/gh19038.phpt b/ext/phar/tests/gh19038.phpt new file mode 100644 index 00000000000..34eba44c1dc --- /dev/null +++ b/ext/phar/tests/gh19038.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-19038 (Phar crash and data corruption with SplFileObject) +--EXTENSIONS-- +phar +--INI-- +phar.readonly=0 +--FILE-- +addFromString("file", "123"); +$file = $phar["file"]->openFile(); +$file->fseek(3); +var_dump($file->fwrite("456", 3)); +$file->fseek(0); +echo $file->fread(100); + +?> +--CLEAN-- + +--EXPECT-- +int(3) +123456