Skip to content

Signed integer overflow in main/streams/streams.c #15980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
YuanchengJiang opened this issue Sep 22, 2024 · 2 comments
Closed

Signed integer overflow in main/streams/streams.c #15980

YuanchengJiang opened this issue Sep 22, 2024 · 2 comments

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
$file_modes = array( "w","wb","wt","w+","w+b","w+t",
"x","xb","xt","x+","x+b","x+t");
$file_content_types = array( "text_with_new_line","alphanumeric");
$offset = array(-1,0,1,PHP_INT_MAX,600); // different offsets
$filename = __DIR__."/fseek_ftell_rewind_variation6.tmp"; // this is name of the file created by create_files()
foreach($file_content_types as $file_content_type){
foreach($file_modes as $file_mode) {
$file_handle = fopen($filename, $file_mode);
foreach($offset as $count){
var_dump( fseek($file_handle,$count,SEEK_CUR) );
} //end of offset loop
} //end of file_mode loop
} //end of file_content_types loop

Resulted in this output:

/php-src/main/streams/streams.c:1385:31: runtime error: signed integer overflow: 1 + 9223372036854775807 cannot be represented in type 'long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /php-src/main/streams/streams.c:1385:31 in

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

@cmb69
Copy link
Member

cmb69 commented Sep 22, 2024

Simple reproducer:

<?php
$s = fopen(__FILE__, "r");
fseek($s, 1);
fseek($s, PHP_INT_MAX, SEEK_CUR);

Possible fix:

 main/streams/streams.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/main/streams/streams.c b/main/streams/streams.c
index f3fb5e3cda..0fbb1a13fc 100644
--- a/main/streams/streams.c
+++ b/main/streams/streams.c
@@ -1382,7 +1382,12 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
 
 		switch(whence) {
 			case SEEK_CUR:
-				offset = stream->position + offset;
+				ZEND_ASSERT(stream->position >= 0);
+				if (offset > ZEND_LONG_MAX - stream->position) {
+					offset = ZEND_LONG_MAX;
+				} else {
+					offset = stream->position + offset;
+				}
 				whence = SEEK_SET;
 				break;
 		}

Older PHP versions are likely affected as well.

@cmb69 cmb69 self-assigned this Sep 22, 2024
cmb69 added a commit to cmb69/php-src that referenced this issue Sep 22, 2024
We need to avoid signed integer overflows which are undefined behavior.
We catch that, and set `offset` to `ZEND_LONG_MAX` (which is also the
largest value of `zend_off_t` on all platforms).  Of course, after such
a seek a stream is no longer readable, but that matches the current
behavior for offsets near `ZEND_LONG_MAX`.
cmb69 added a commit that referenced this issue Sep 22, 2024
* PHP-8.2:
  Fix GH-15980: Signed integer overflow in main/streams/streams.c
@cmb69 cmb69 closed this as completed in 6a04c79 Sep 22, 2024
cmb69 added a commit that referenced this issue Sep 22, 2024
* PHP-8.3:
  Fix GH-15980: Signed integer overflow in main/streams/streams.c
cmb69 added a commit that referenced this issue Sep 22, 2024
This reverts commit 6a04c79, since the
new test case apparently fails on 64bit Linux, so this needs closer
investigation.
cmb69 added a commit that referenced this issue Sep 22, 2024
* PHP-8.2:
  Revert "Fix GH-15980: Signed integer overflow in main/streams/streams.c"
cmb69 added a commit that referenced this issue Sep 22, 2024
* PHP-8.3:
  Revert "Fix GH-15980: Signed integer overflow in main/streams/streams.c"
@cmb69
Copy link
Member

cmb69 commented Sep 22, 2024

I had to revert, so reopening.

@cmb69 cmb69 reopened this Sep 22, 2024
@cmb69 cmb69 closed this as completed in 8191675 Sep 24, 2024
cmb69 added a commit that referenced this issue Sep 24, 2024
* PHP-8.2:
  Fix GH-15980: Signed integer overflow in main/streams/streams.c
cmb69 added a commit that referenced this issue Sep 24, 2024
* PHP-8.3:
  Fix GH-15980: Signed integer overflow in main/streams/streams.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants