diff --git a/NEWS b/NEWS index 5c6f6e8db9e3..405909644a8c 100644 --- a/NEWS +++ b/NEWS @@ -81,6 +81,10 @@ PHP NEWS . Added support for the libsodium 1.0.22 KEM APIs (X-Wing and ML-KEM768). (Zachary DuBois) +- SPL: + . Fixed SplFileObject::fseek() accepting $whence values that do not fit in an + int, which were silently truncated onto a valid seek constant. (lacatoire) + - Standard: . Fixed a segfault when a stream filter callback unsets StreamBucket::$data before re-attaching the bucket. (iliaal) @@ -93,6 +97,10 @@ PHP NEWS argument number for $timeout). (lacatoire) . Fixed bug GH-23576 (Next index for array returned from array_keys() is wrong). (Lazizbek Ergashev) + . Fixed fseek() accepting $whence values that do not fit in an int, which + were silently truncated onto a valid seek constant. (lacatoire) + . Fixed a failed seek discarding the read buffer, which desynchronized the + stream from its reported position. (lacatoire) - SimpleXML: . Fixed writing to a dimension of the object returned by attributes() not diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 16571158610d..4f9c5198a87a 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -2458,8 +2458,17 @@ PHP_METHOD(SplFileObject, fseek) CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern); - spl_filesystem_file_free_line(intern); - RETURN_LONG(php_stream_seek(intern->u.file.stream, pos, (int)whence)); + if (ZEND_LONG_EXCEEDS_INT(whence)) { + RETURN_LONG(-1); + } + + int ret = php_stream_seek(intern->u.file.stream, pos, (int)whence); + + if (ret == 0) { + spl_filesystem_file_free_line(intern); + } + + RETURN_LONG(ret); } /* }}} */ /* {{{ Get a character from the file */ diff --git a/ext/spl/tests/SplFileObject/SplFileObject_fseek_whence_invalid_inrange.phpt b/ext/spl/tests/SplFileObject/SplFileObject_fseek_whence_invalid_inrange.phpt new file mode 100644 index 000000000000..7eafd8d977b8 --- /dev/null +++ b/ext/spl/tests/SplFileObject/SplFileObject_fseek_whence_invalid_inrange.phpt @@ -0,0 +1,48 @@ +--TEST-- +SplFileObject::fseek(): a failed seek must not discard the current line +--FILE-- +current()); + var_dump($file->fseek(3, $whence)); + var_dump($file->ftell()); + var_dump($file->current()); + unset($file); + + echo PHP_EOL; +} +?> +--CLEAN-- + +--EXPECT-- +whence=99 +string(4) "aaa +" +int(-1) +int(4) +string(4) "aaa +" + +whence=-2147483648 +string(4) "aaa +" +int(-1) +int(4) +string(4) "aaa +" + +whence=2147483647 +string(4) "aaa +" +int(-1) +int(4) +string(4) "aaa +" diff --git a/ext/spl/tests/SplFileObject/SplFileObject_fseek_whence_overflow.phpt b/ext/spl/tests/SplFileObject/SplFileObject_fseek_whence_overflow.phpt new file mode 100644 index 000000000000..0d757cd2dd29 --- /dev/null +++ b/ext/spl/tests/SplFileObject/SplFileObject_fseek_whence_overflow.phpt @@ -0,0 +1,49 @@ +--TEST-- +SplFileObject::fseek(): $whence values that overflow int must return -1, not alias onto a valid constant +--SKIPIF-- + +--FILE-- +fseek(4); +var_dump($file->fseek(3, SEEK_CUR + $bias)); // -1 +var_dump($file->ftell()); // 4 (unchanged) + +// SEEK_END + 2**32 must not alias onto SEEK_END (2) +$file->fseek(4); +var_dump($file->fseek(3, SEEK_END + $bias)); // -1 +var_dump($file->ftell()); // 4 (unchanged) + +// PHP_INT_MIN must not alias onto SEEK_SET (0) +$file->fseek(4); +var_dump($file->fseek(3, PHP_INT_MIN)); // -1 +var_dump($file->ftell()); // 4 (unchanged) + +// Sanity: normal SEEK_CUR still works +$file->fseek(4); +var_dump($file->fseek(3, SEEK_CUR)); // 0 +var_dump($file->ftell()); // 7 + +unset($file); +?> +--CLEAN-- + +--EXPECT-- +int(-1) +int(4) +int(-1) +int(4) +int(-1) +int(4) +int(0) +int(7) diff --git a/ext/standard/file.c b/ext/standard/file.c index b52e5ba9525f..58ab662aa01e 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -1103,6 +1103,10 @@ PHPAPI PHP_FUNCTION(fseek) Z_PARAM_LONG(whence) ZEND_PARSE_PARAMETERS_END(); + if (ZEND_LONG_EXCEEDS_INT(whence)) { + RETURN_LONG(-1); + } + php_stream_error_operation_begin(); RETVAL_LONG(php_stream_seek(stream, offset, (int) whence)); php_stream_error_operation_end_for_stream(stream); diff --git a/ext/standard/tests/file/fseek_whence_invalid_inrange.phpt b/ext/standard/tests/file/fseek_whence_invalid_inrange.phpt new file mode 100644 index 000000000000..c10fbcfbeb73 --- /dev/null +++ b/ext/standard/tests/file/fseek_whence_invalid_inrange.phpt @@ -0,0 +1,67 @@ +--TEST-- +fseek(): an invalid $whence that fits in an int must not desynchronize the stream +--FILE-- + $tmp, 'memory' => 'php://memory'] as $label => $target) { + echo $label, PHP_EOL; + $h = fopen($target, $label === 'file' ? 'r' : 'w+'); + if ($label === 'memory') { + fwrite($h, "0123456789"); + rewind($h); + } + var_dump(fread($h, 4)); + var_dump(fseek($h, 3, $whence)); + var_dump(ftell($h)); + var_dump(fread($h, 6)); + fclose($h); + } + + echo PHP_EOL; +} +?> +--CLEAN-- + +--EXPECT-- +whence=99 +file +string(4) "0123" +int(-1) +int(4) +string(6) "456789" +memory +string(4) "0123" +int(-1) +int(4) +string(6) "456789" + +whence=-2147483648 +file +string(4) "0123" +int(-1) +int(4) +string(6) "456789" +memory +string(4) "0123" +int(-1) +int(4) +string(6) "456789" + +whence=2147483647 +file +string(4) "0123" +int(-1) +int(4) +string(6) "456789" +memory +string(4) "0123" +int(-1) +int(4) +string(6) "456789" diff --git a/ext/standard/tests/file/fseek_whence_overflow.phpt b/ext/standard/tests/file/fseek_whence_overflow.phpt new file mode 100644 index 000000000000..46c5a0292ef1 --- /dev/null +++ b/ext/standard/tests/file/fseek_whence_overflow.phpt @@ -0,0 +1,49 @@ +--TEST-- +fseek(): $whence values that overflow int must return -1, not alias onto a valid constant +--SKIPIF-- + +--FILE-- + +--CLEAN-- + +--EXPECT-- +int(-1) +int(4) +int(-1) +int(4) +int(-1) +int(4) +int(0) +int(7) diff --git a/main/streams/streams.c b/main/streams/streams.c index a09a2180921d..bbc5047ab357 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -1367,6 +1367,7 @@ PHPAPI int php_stream_seek(php_stream *stream, zend_off_t offset, int whence) if (stream->ops->seek && (stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) { + zend_off_t old_position = stream->position; int ret; switch(whence) { case SEEK_CUR: @@ -1386,6 +1387,13 @@ PHPAPI int php_stream_seek(php_stream *stream, zend_off_t offset, int whence) ret = stream->ops->seek(stream, offset, whence, &stream->position); if (((stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) || ret == 0) { + if (ret != 0 && stream->position == old_position) { + /* the seek failed without moving the stream, so the buffered + * data and the filter state still describe the current + * position and must be left alone */ + return ret; + } + if (ret == 0) { stream->eof = 0; stream->fatal_error = 0;