From b85429e1c74f216655b79a8f5b44587384bdb20b Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 5 Jun 2025 19:30:37 +0200 Subject: [PATCH] Fix GH-13264: fgets() and stream_get_line() do not return false on filter fatal error This happens because there are no checks in php_stream_fill_read_buffer calls. This should not fail always but only on fatal error so special flag is needed for that. Closes GH-18778 --- ext/sqlite3/sqlite3.c | 4 ++ ext/standard/tests/filters/gh13264.phpt | 72 ++++++++++++++----------- main/php_streams.h | 3 ++ main/streams/memory.c | 5 ++ main/streams/streams.c | 16 +++++- 5 files changed, 66 insertions(+), 34 deletions(-) diff --git a/ext/sqlite3/sqlite3.c b/ext/sqlite3/sqlite3.c index 09cb57410c87..9b3286b70220 100644 --- a/ext/sqlite3/sqlite3.c +++ b/ext/sqlite3/sqlite3.c @@ -1165,6 +1165,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh sqlite3_stream->position = sqlite3_stream->position + offset; *newoffs = sqlite3_stream->position; stream->eof = 0; + stream->fatal_error = 0; return 0; } } else { @@ -1176,6 +1177,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh sqlite3_stream->position = sqlite3_stream->position + offset; *newoffs = sqlite3_stream->position; stream->eof = 0; + stream->fatal_error = 0; return 0; } } @@ -1188,6 +1190,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh sqlite3_stream->position = offset; *newoffs = sqlite3_stream->position; stream->eof = 0; + stream->fatal_error = 0; return 0; } case SEEK_END: @@ -1203,6 +1206,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh sqlite3_stream->position = sqlite3_stream->size + offset; *newoffs = sqlite3_stream->position; stream->eof = 0; + stream->fatal_error = 0; return 0; } default: diff --git a/ext/standard/tests/filters/gh13264.phpt b/ext/standard/tests/filters/gh13264.phpt index e992d0868898..b31a0d98c34d 100644 --- a/ext/standard/tests/filters/gh13264.phpt +++ b/ext/standard/tests/filters/gh13264.phpt @@ -1,49 +1,57 @@ --TEST-- -GH-81475: Memory leak during stream filter failure +GH-13264: fgets() and stream_get_line() do not return false on filter fatal error --SKIPIF-- --FILE-- 15]); -// Rewind and add the zlib filter -rewind($stream); -stream_filter_append($stream, 'zlib.inflate', STREAM_FILTER_READ, ['window' => 15]); + return $stream; +} -// Read the filtered stream line by line. +// Read the filtered stream line by line using fgets. +$stream = create_stream(); while (($line = fgets($stream)) !== false) { - $error = error_get_last(); - if ($error !== null) { - // An error is thrown but fgets didn't return false - var_dump(error_get_last()); - var_dump($line); - } + $error = error_get_last(); + if ($error !== null) { + // An error is thrown but fgets didn't return false + var_dump(error_get_last()); + var_dump($line); + } } +fclose($stream); +error_clear_last(); +// Read the filtered stream line by line using stream_get_line. +$stream = create_stream(); +while (($line = stream_get_line($stream, 0, "\n")) !== false) { + $error = error_get_last(); + if ($error !== null) { + // An error is thrown but fgets didn't return false + var_dump(error_get_last()); + var_dump($line); + } +} fclose($stream); ?> --EXPECTF-- Notice: fgets(): zlib: data error in %s on line %d -array(4) { - ["type"]=> - int(8) - ["message"]=> - string(25) "fgets(): zlib: data error" - ["file"]=> - string(%d) "%s" - ["line"]=> - int(%d) -} -string(%d) "Hello%s" +Notice: stream_get_line(): zlib: data error in %s on line %d diff --git a/main/php_streams.h b/main/php_streams.h index 31b80de98605..8996ee8bcb07 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -223,6 +223,9 @@ struct _php_stream { /* whether stdio cast flushing is in progress */ uint16_t fclose_stdiocast_flush_in_progress:1; + /* whether fatal error happened and all operations should terminates as soon as possible */ + uint16_t fatal_error:1; + char mode[16]; /* "rwb" etc. ala stdio */ uint32_t flags; /* PHP_STREAM_FLAG_XXX */ diff --git a/main/streams/memory.c b/main/streams/memory.c index ce11aec382bf..785109db6582 100644 --- a/main/streams/memory.c +++ b/main/streams/memory.c @@ -136,10 +136,12 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe ms->fpos = ms->fpos + offset; *newoffs = ms->fpos; stream->eof = 0; + stream->fatal_error = 0; return 0; } } else { stream->eof = 0; + stream->fatal_error = 0; ms->fpos = ms->fpos + offset; *newoffs = ms->fpos; return 0; @@ -153,6 +155,7 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe ms->fpos = offset; *newoffs = ms->fpos; stream->eof = 0; + stream->fatal_error = 0; return 0; } case SEEK_END: @@ -160,6 +163,7 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe ms->fpos = ZSTR_LEN(ms->data) + offset; *newoffs = ms->fpos; stream->eof = 0; + stream->fatal_error = 0; return 0; } else if (ZSTR_LEN(ms->data) < (size_t)(-offset)) { ms->fpos = 0; @@ -169,6 +173,7 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe ms->fpos = ZSTR_LEN(ms->data) + offset; *newoffs = ms->fpos; stream->eof = 0; + stream->fatal_error = 0; return 0; } default: diff --git a/main/streams/streams.c b/main/streams/streams.c index 4f9c88e4774c..1471c98558e1 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -636,6 +636,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size) /* some fatal error. Theoretically, the stream is borked, so all * further reads should fail. */ stream->eof = 1; + stream->fatal_error = 1; /* free all data left in brigades */ while ((bucket = brig_inp->head)) { /* Remove unconsumed buckets from the input brigade */ @@ -1009,7 +1010,12 @@ PHPAPI char *_php_stream_get_line(php_stream *stream, char *buf, size_t maxlen, } } - php_stream_fill_read_buffer(stream, toread); + if (php_stream_fill_read_buffer(stream, toread) == FAILURE && stream->fatal_error) { + if (grow_mode) { + efree(bufstart); + } + return NULL; + } if (stream->writepos - stream->readpos == 0) { break; @@ -1084,7 +1090,9 @@ PHPAPI zend_string *php_stream_get_record(php_stream *stream, size_t maxlen, con to_read_now = MIN(maxlen - buffered_len, stream->chunk_size); - php_stream_fill_read_buffer(stream, buffered_len + to_read_now); + if (php_stream_fill_read_buffer(stream, buffered_len + to_read_now) == FAILURE && stream->fatal_error) { + return NULL; + } just_read = STREAM_BUFFERED_AMOUNT(stream) - buffered_len; @@ -1357,6 +1365,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence) stream->readpos += offset; /* if offset = ..., then readpos = writepos */ stream->position += offset; stream->eof = 0; + stream->fatal_error = 0; return 0; } break; @@ -1366,6 +1375,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence) stream->readpos += offset - stream->position; stream->position = offset; stream->eof = 0; + stream->fatal_error = 0; return 0; } break; @@ -1400,6 +1410,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence) if (((stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) || ret == 0) { if (ret == 0) { stream->eof = 0; + stream->fatal_error = 0; } /* invalidate the buffer contents */ @@ -1422,6 +1433,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence) offset -= didread; } stream->eof = 0; + stream->fatal_error = 0; return 0; }