From d9e40372fcd62be0ee1c954a1a2172ab054815da Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Thu, 13 Nov 2025 23:19:04 +0100 Subject: [PATCH] Fix assertion failure when fseeking a phar file out of bounds In 61884c3b52 I added these FIXME comments after I noticed that this would cause an assertion failure. At that time I did not yet know what to do here. I took a look at the code now and other streams return -1 and leave the file position untouched. So we do the same for phar. This fixes the assertion failure and subsequent crashes, but also changes one test output. However, I believe the new test output is correct. Closes GH-20475. --- NEWS | 1 + ext/phar/stream.c | 2 -- ext/phar/tests/022.phpt | 14 ++++++------- ext/phar/tests/fseek_outside_bounds.phpt | 26 ++++++++++++++++++++++++ 4 files changed, 34 insertions(+), 9 deletions(-) create mode 100644 ext/phar/tests/fseek_outside_bounds.phpt diff --git a/NEWS b/NEWS index a7b5e126e3a..c9ff9ac461d 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,7 @@ PHP NEWS . Fixed bug GH-20442 (Phar does not respect case-insensitiveness of __halt_compiler() when reading stub). (ndossche, TimWolla) . Fix broken return value of fflush() for phar file entries. (ndossche) + . Fix assertion failure when fseeking a phar file out of bounds. (ndossche) - PHPDBG: . Fixed ZPP type violation in phpdbg_get_executable() and phpdbg_end_oplog(). diff --git a/ext/phar/stream.c b/ext/phar/stream.c index a0049f9a6b1..1151b520bba 100644 --- a/ext/phar/stream.c +++ b/ext/phar/stream.c @@ -429,11 +429,9 @@ static int phar_stream_seek(php_stream *stream, zend_off_t offset, int whence, z zend_off_t temp_signed = (zend_off_t) temp; if (temp_signed > data->zero + (zend_off_t) entry->uncompressed_filesize) { - *newoffset = -1; /* FIXME: this will invalidate the ZEND_ASSERT(stream->position >= 0); assertion in streams.c */ return -1; } if (temp_signed < data->zero) { - *newoffset = -1; /* FIXME: this will invalidate the ZEND_ASSERT(stream->position >= 0); assertion in streams.c */ return -1; } res = php_stream_seek(data->fp, temp_signed, SEEK_SET); diff --git a/ext/phar/tests/022.phpt b/ext/phar/tests/022.phpt index 5363a65be94..c484c4d3c06 100644 --- a/ext/phar/tests/022.phpt +++ b/ext/phar/tests/022.phpt @@ -80,28 +80,28 @@ int(1) fseek($fp, -1, SEEK_END)int(0) int(6) fseek($fp, -8, SEEK_END)int(-1) -bool(false) +int(6) fseek($fp, -7, SEEK_END)int(0) int(0) fseek($fp, 0, SEEK_END)int(0) int(7) fseek($fp, 1, SEEK_END)int(-1) -bool(false) +int(7) fseek($fp, -8, SEEK_END)int(-1) -bool(false) +int(7) fseek($fp, 6)int(0) int(6) fseek($fp, 8)int(-1) -bool(false) +int(6) fseek($fp, -1)int(-1) -bool(false) +int(6) next int(4) fseek($fp, -5, SEEK_CUR)int(-1) -bool(false) +int(4) int(4) fseek($fp, 5, SEEK_CUR)int(-1) -bool(false) +int(4) int(4) fseek($fp, -4, SEEK_CUR)int(0) int(0) diff --git a/ext/phar/tests/fseek_outside_bounds.phpt b/ext/phar/tests/fseek_outside_bounds.phpt new file mode 100644 index 00000000000..0a35abb360d --- /dev/null +++ b/ext/phar/tests/fseek_outside_bounds.phpt @@ -0,0 +1,26 @@ +--TEST-- +Assertion failure when fseeking outside of bounds of phar file +--EXTENSIONS-- +phar +--INI-- +phar.require_hash=0 +--FILE-- +setInfoClass('SplFileObject'); +$f = $phar['a.php']; +var_dump($f->fseek(1, SEEK_SET)); +var_dump($f->fseek(999999, SEEK_SET)); +var_dump($f->fseek(999999, SEEK_CUR)); +var_dump($f->ftell()); +var_dump($f->fseek(1, SEEK_CUR)); +var_dump($f->fread(3)); +?> +--EXPECT-- +int(0) +int(-1) +int(-1) +int(1) +int(0) +string(3) "php"