Fix GH-21700: undefined behavior in _php_stream_seek with user stream returning negative position#21914
Open
lacatoire wants to merge 1 commit intophp:masterfrom
Open
Conversation
…am returning negative position When a user stream's stream_tell() returns a negative offset after a stream_seek() reported success, _php_stream_seek would store that value into stream->position and then trip ZEND_ASSERT(stream->position >= 0) on the next SEEK_CUR. Three changes: - After the SEEK_CUR -> SEEK_SET conversion, fall through to the existing 'offset < 0' rejection so a SEEK_CUR whose absolute target would be negative is refused before invoking the user seek hook. - After stream->ops->seek returns success, validate stream->position is non-negative. If a user stream lied (e.g. stream_tell() returns -42), restore the prior position and return -1, matching POSIX fseek()'s contract that the file-position indicator is unchanged on failure. - In php_stream_open_wrapper_ex's append-mode position fixup, ignore a negative newpos returned by stream->ops->seek (defense in depth for the only other direct caller of the seek hook in this layer). Includes phpt regression tests covering both _php_stream_seek paths and the append-mode open path.
lacatoire
force-pushed
the
fix/21700-stream-seek-negative-position
branch
from
471818b to
225fa9e
Compare
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
1 participant
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.
Fixes GH-21700.
When a user stream's
stream_tell()returns a negative offset after astream_seek()reported success,_php_stream_seekwould store that value intostream->positionand then tripZEND_ASSERT(stream->position >= 0)on the nextSEEK_CUR(or any subsequent SEEK_CUR-based op).Two changes:
offset < 0rejection so a SEEK_CUR whose absolute target is negative is refused before invoking the user seek hook.stream->ops->seekreturns success, validatestream->positionis non-negative. If a user stream lied (e.g.stream_tell()returns -42), restore the prior position and return -1, matching POSIXfseek()'s contract that the file-position indicator is unchanged on failure.Includes a regression phpt covering both paths.