From f47bf6407f33579dd09aa570b9b32ba8bc0c2495 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. --- ext/phar/phar.c | 2 +- ext/phar/stream.c | 4 ++-- ext/phar/tar.c | 2 +- ext/phar/tests/gh19038.phpt | 25 +++++++++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 ext/phar/tests/gh19038.phpt diff --git a/ext/phar/phar.c b/ext/phar/phar.c index 125fc84703612..ab5460887c3fe 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 fee100cc31a10..e68f07bddd0ca 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 e259fb6f3dece..38bd52c86093b 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 0000000000000..34eba44c1dcd6 --- /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