镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Fix fseek undefined behavior - #20965

Closed
arshidkv12 wants to merge 4 commits into
php:masterfrom
arshidkv12:fseek
Closed

arshidkv12 wants to merge 4 commits into
php:masterfrom
arshidkv12:fseek

Conversation

@arshidkv12

Copy link
Copy Markdown
Contributor

@arshidkv12
arshidkv12 requested a review from bukka as a code owner January 18, 2026 03:58
Comment thread main/streams/memory.c Outdated
case SEEK_CUR:
if (offset < 0) {
if (ms->fpos < (size_t)(-offset)) {
if (ms->fpos < -(size_t)offset) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error goes away, but it's not really correct. The - effectively becomes a no-op. However, the intent here is to compare ms->fpos to a positive offset.

I'm not sure if we need this check at all? For positive offset, we're not checking for overflows and rely on eof checks when reading from the stream, so do we need to check for underflows for negative offset? It would simply wrap around the same way positive offsets do. Though I suppose removing this check is a subtle breaking change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it as before.

case SEEK_CUR:
  if (offset < 0) {
-  if (ms->fpos < -(size_t)offset) {
+  if (ms->fpos < (size_t)(-offset)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but my comment was referring to both cases. I suppose the fix is ok to avoid the undefined behavior, though it's probably not the optimal fix either.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Adding the following code in the first part of the php_stream_memory_seek function, what do you think?

	if (offset == ZEND_LONG_MIN) {
		zend_argument_value_error(2, "must be greater than " ZEND_LONG_FMT, ZEND_LONG_MIN);
		return FAILURE;
	}  

@bukka

bukka commented Oct 6, 2026

Copy link
Copy Markdown
Member

This was already addressed by #21433

@bukka bukka closed this Oct 6, 2026
@arshidkv12
arshidkv12 deleted the fseek branch October 6, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants