Skip to content

Fix GH-20921: Avoid signed negation of PHP_INT_MIN in memory stream seek#22163

Open
wadakatu wants to merge 1 commit into
php:PHP-8.4from
wadakatu:fix/gh-20921
Open

Fix GH-20921: Avoid signed negation of PHP_INT_MIN in memory stream seek#22163
wadakatu wants to merge 1 commit into
php:PHP-8.4from
wadakatu:fix/gh-20921

Conversation

@wadakatu
Copy link
Copy Markdown

Summary

Fixes GH-20921.

php_stream_memory_seek() evaluated (size_t)(-offset) for negative offsets
in the SEEK_END branch. When offset == LONG_MIN, the unary minus is
performed in signed zend_off_t and overflows — undefined behavior under C11
6.5p5 — which UBSan flags:

main/streams/memory.c:168:45: runtime error: negation of -9223372036854775808
cannot be represented in type 'zend_off_t' (aka 'long'); cast to an unsigned
type to negate this value to itself

The fix casts to size_t first so the negation happens in unsigned
arithmetic, which is defined to wrap modulo 2^N and yields the same
absolute value without invoking UB.

Reproducer

<?php
$f = new SplTempFileObject();
$f->fwrite("abcdef");
var_dump($f->fseek(PHP_INT_MIN, SEEK_END));

Run under a UBSan-enabled build (--enable-undefined-sanitizer) to observe
the runtime error before the fix.

Note on the SEEK_CUR branch

A structurally identical (size_t)(-offset) exists in the SEEK_CUR branch
of the same function (line 131). However, _php_stream_seek() rewrites
SEEK_CUR to SEEK_SET before calling the ops vtable, so that path is
unreachable via the userland fseek() API. I left it untouched to keep this
PR focused on the reported, user-reachable case; happy to fold a follow-up
cleanup in if maintainers prefer.

Test plan

  • New regression test ext/standard/tests/streams/gh20921.phpt
  • make test TESTS=ext/standard/tests/streams — 130 pass, 24 skip, 0 fail
  • make test TESTS=ext/spl/tests — 758 pass, 9 skip, 1 XFAIL, 0 fail
  • CI UBSan job — relies on PR pipeline to verify the UB no longer fires

…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.
@jorgsowa
Copy link
Copy Markdown
Contributor

note: This is the same fix as in #20922

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants