Fix GH-20921: Avoid signed negation of PHP_INT_MIN in memory stream seek#22163
Open
wadakatu wants to merge 1 commit into
Open
Fix GH-20921: Avoid signed negation of PHP_INT_MIN in memory stream seek#22163wadakatu wants to merge 1 commit into
wadakatu wants to merge 1 commit into
Conversation
…m seek
In php_stream_memory_seek(), the SEEK_END handler computed
`(size_t)(-offset)` to obtain the absolute value of a negative offset.
When `offset` is `LONG_MIN` (PHP_INT_MIN), the unary minus is performed
in signed `zend_off_t` and overflows, which is undefined behavior. UBSan
reports:
runtime error: negation of -9223372036854775808 cannot be represented
in type 'zend_off_t' (aka 'long')
Cast to unsigned first so that the negation happens in unsigned
arithmetic (defined to wrap modulo 2^N), yielding the same absolute
value without invoking UB.
A matching pattern exists in the SEEK_CUR branch, but _php_stream_seek()
converts SEEK_CUR to SEEK_SET before invoking the memory ops, so that
path is unreachable via fseek() and is left for a separate cleanup.
Contributor
|
note: This is the same fix as in #20922 |
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
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.
Summary
Fixes GH-20921.
php_stream_memory_seek()evaluated(size_t)(-offset)for negative offsetsin the
SEEK_ENDbranch. Whenoffset == LONG_MIN, the unary minus isperformed in signed
zend_off_tand overflows — undefined behavior under C116.5p5 — which UBSan flags:
The fix casts to
size_tfirst so the negation happens in unsignedarithmetic, which is defined to wrap modulo 2^N and yields the same
absolute value without invoking UB.
Reproducer
Run under a UBSan-enabled build (
--enable-undefined-sanitizer) to observethe runtime error before the fix.
Note on the SEEK_CUR branch
A structurally identical
(size_t)(-offset)exists in theSEEK_CURbranchof the same function (line 131). However,
_php_stream_seek()rewritesSEEK_CURtoSEEK_SETbefore calling the ops vtable, so that path isunreachable via the userland
fseek()API. I left it untouched to keep thisPR focused on the reported, user-reachable case; happy to fold a follow-up
cleanup in if maintainers prefer.
Test plan
ext/standard/tests/streams/gh20921.phptmake test TESTS=ext/standard/tests/streams— 130 pass, 24 skip, 0 failmake test TESTS=ext/spl/tests— 758 pass, 9 skip, 1 XFAIL, 0 fail