Fix fseek() accepting $whence values that do not fit in an int - #23483
Open
lacatoire wants to merge 2 commits into
Open
Fix fseek() accepting $whence values that do not fit in an int#23483lacatoire wants to merge 2 commits into
lacatoire wants to merge 2 commits into
Conversation
devnexen
reviewed
Aug 28, 2026
$whence is parsed as a zend_long but cast to a C int when handed to php_stream_seek(). Values whose low 32 bits alias onto a valid seek constant were accepted and acted upon: on a 64-bit build, SEEK_CUR + 2**32 seeks relative to the current position, SEEK_END + 2**32 relative to the end, and PHP_INT_MIN relative to the start, each returning 0 for success. Reject values outside the int range before the cast and return -1, which is already what fseek() returns for an invalid $whence that does fit in an int. Platform constants such as SEEK_DATA and SEEK_HOLE fit in an int and keep working.
lacatoire
force-pushed
the
fix/fseek-whence-int-narrowing
branch
from
August 28, 2026 12:33
7c44b7a to
df3278d
Compare
php_stream_seek() invalidated the read buffer after calling ops->seek()
even when that call reported a failure. On a plain file, an invalid
$whence makes lseek() fail with EINVAL without moving the descriptor,
so the buffered data was still valid for the reported position; dropping
it desynchronized the stream:
$h = fopen($f, 'r'); // 10-byte file
fread($h, 4); // "0123"
fseek($h, 3, 99); // -1, position left at 4
fread($h, 6); // "" instead of "456789"
The buffer and the filter state are now left alone when ops->seek()
fails without moving the stream. Implementations that do move on failure,
such as php_stream_memory_seek() resetting fpos, still invalidate it.
devnexen
reviewed
Aug 28, 2026
| Z_PARAM_LONG(whence) | ||
| ZEND_PARSE_PARAMETERS_END(); | ||
|
|
||
| if (whence < INT_MIN || whence > INT_MAX) { |
Member
There was a problem hiding this comment.
I think you can just use ZEND_LONG_EXCEEDS_INT macro here.
| @@ -0,0 +1,40 @@ | |||
| --TEST-- | |||
| fseek(): an invalid $whence that fits in an int must not desynchronize the stream | |||
Member
There was a problem hiding this comment.
would also test for non file based streams, eg. memory.
| ?> | ||
| --FILE-- | ||
| <?php | ||
| $tmp = tempnam(sys_get_temp_dir(), 'fseek'); |
Member
There was a problem hiding this comment.
also you need a CLEAN section for this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fseek()parses$whenceas azend_long, but casts it to a Cintwhen callingphp_stream_seek(). Values whose low 32 bits alias onto a valid seek constant are accepted and acted upon.On a 64-bit build, with a 10-byte file and the cursor at 4:
A return value of
0means success, so the call reports that a seek nobody asked for went through.The fix rejects values outside the
intrange before the cast:-1is not a new convention: it is already whatfseek()returns for an invalid$whencethat happens to fit in anint, and the position is left untouched in both cases.Platform constants such as
SEEK_DATA(3) andSEEK_HOLE(4) fit in anintand are unaffected.ext/standard/tests/file/fseek_whence_overflow.phptcovers the three aliasing forms and a plainSEEK_CURas a regression guard. It skips on 32-bit builds, where nozend_longcan exceed anint.Tested on a 64-bit Linux build: the new test passes, and
ext/standard/tests/file,ext/standard/tests/streamsandext/standard/tests/filtersshow no regression.