Skip to content

Commit ae4ee72

Browse files
bukkabukka
committed
Fix GH-81475: 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.
1 parent 444cc78 commit ae4ee72

File tree

5 files changed

+66
-34
lines changed

5 files changed

+66
-34
lines changed

‎ext/sqlite3/sqlite3.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh
11651165
sqlite3_stream->position = sqlite3_stream->position + offset;
11661166
*newoffs = sqlite3_stream->position;
11671167
stream->eof = 0;
1168+
stream->fatal_error = 0;
11681169
return 0;
11691170
}
11701171
} else {
@@ -1176,6 +1177,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh
11761177
sqlite3_stream->position = sqlite3_stream->position + offset;
11771178
*newoffs = sqlite3_stream->position;
11781179
stream->eof = 0;
1180+
stream->fatal_error = 0;
11791181
return 0;
11801182
}
11811183
}
@@ -1188,6 +1190,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh
11881190
sqlite3_stream->position = offset;
11891191
*newoffs = sqlite3_stream->position;
11901192
stream->eof = 0;
1193+
stream->fatal_error = 0;
11911194
return 0;
11921195
}
11931196
case SEEK_END:
@@ -1203,6 +1206,7 @@ static int php_sqlite3_stream_seek(php_stream *stream, zend_off_t offset, int wh
12031206
sqlite3_stream->position = sqlite3_stream->size + offset;
12041207
*newoffs = sqlite3_stream->position;
12051208
stream->eof = 0;
1209+
stream->fatal_error = 0;
12061210
return 0;
12071211
}
12081212
default:

‎ext/standard/tests/filters/gh13264.phpt

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,57 @@
11
--TEST--
2-
GH-81475: Memory leak during stream filter failure
2+
GH-81475: fgets() or stream_get_line() does not return false on filter fatal error
33
--SKIPIF--
44
<?php require 'filter_errors.inc'; filter_errors_skipif('zlib.inflate'); ?>
55
--FILE--
66
<?php
7-
// Prepare a big enough input so that it is not entirely buffered
8-
$stream = fopen('php://memory', 'r+');
9-
$content = '';
10-
for ($i = 0; $i < 10000; $i++) {
11-
$content .= "Hello $i\n";
12-
}
13-
fwrite($stream, gzcompress($content));
7+
function create_stream()
8+
{
9+
// Prepare a big enough input so that it is not entirely buffered
10+
$stream = fopen('php://memory', 'r+');
11+
$content = '';
12+
for ($i = 0; $i < 10000; $i++) {
13+
$content .= "Hello $i\n";
14+
}
15+
fwrite($stream, gzcompress($content));
16+
17+
// Mess up the checksum
18+
fseek($stream, -1, SEEK_CUR);
19+
fwrite($stream, '1');
1420

15-
// Mess up the checksum
16-
fseek($stream, -1, SEEK_CUR);
17-
fwrite($stream, '1');
21+
// Rewind and add the zlib filter
22+
rewind($stream);
23+
stream_filter_append($stream, 'zlib.inflate', STREAM_FILTER_READ, ['window' => 15]);
1824

19-
// Rewind and add the zlib filter
20-
rewind($stream);
21-
stream_filter_append($stream, 'zlib.inflate', STREAM_FILTER_READ, ['window' => 15]);
25+
return $stream;
26+
}
2227

23-
// Read the filtered stream line by line.
28+
// Read the filtered stream line by line using fgets.
29+
$stream = create_stream();
2430
while (($line = fgets($stream)) !== false) {
25-
$error = error_get_last();
26-
if ($error !== null) {
27-
// An error is thrown but fgets didn't return false
28-
var_dump(error_get_last());
29-
var_dump($line);
30-
}
31+
$error = error_get_last();
32+
if ($error !== null) {
33+
// An error is thrown but fgets didn't return false
34+
var_dump(error_get_last());
35+
var_dump($line);
36+
}
3137
}
38+
fclose($stream);
39+
error_clear_last();
3240

41+
// Read the filtered stream line by line using stream_get_line.
42+
$stream = create_stream();
43+
while (($line = stream_get_line($stream, 0, "\n")) !== false) {
44+
$error = error_get_last();
45+
if ($error !== null) {
46+
// An error is thrown but fgets didn't return false
47+
var_dump(error_get_last());
48+
var_dump($line);
49+
}
50+
}
3351
fclose($stream);
3452
?>
3553
--EXPECTF--
3654

3755
Notice: fgets(): zlib: data error in %s on line %d
38-
array(4) {
39-
["type"]=>
40-
int(8)
41-
["message"]=>
42-
string(25) "fgets(): zlib: data error"
43-
["file"]=>
44-
string(%d) "%s"
45-
["line"]=>
46-
int(%d)
47-
}
48-
string(%d) "Hello%s"
4956

57+
Notice: stream_get_line(): zlib: data error in %s on line %d

‎main/php_streams.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,9 @@ struct _php_stream {
223223
/* whether stdio cast flushing is in progress */
224224
uint16_t fclose_stdiocast_flush_in_progress:1;
225225

226+
/* whether fatal error happened and all operations should terminates as soon as possible */
227+
uint16_t fatal_error:1;
228+
226229
char mode[16]; /* "rwb" etc. ala stdio */
227230

228231
uint32_t flags; /* PHP_STREAM_FLAG_XXX */

‎main/streams/memory.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,12 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe
136136
ms->fpos = ms->fpos + offset;
137137
*newoffs = ms->fpos;
138138
stream->eof = 0;
139+
stream->fatal_error = 0;
139140
return 0;
140141
}
141142
} else {
142143
stream->eof = 0;
144+
stream->fatal_error = 0;
143145
ms->fpos = ms->fpos + offset;
144146
*newoffs = ms->fpos;
145147
return 0;
@@ -153,13 +155,15 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe
153155
ms->fpos = offset;
154156
*newoffs = ms->fpos;
155157
stream->eof = 0;
158+
stream->fatal_error = 0;
156159
return 0;
157160
}
158161
case SEEK_END:
159162
if (offset > 0) {
160163
ms->fpos = ZSTR_LEN(ms->data) + offset;
161164
*newoffs = ms->fpos;
162165
stream->eof = 0;
166+
stream->fatal_error = 0;
163167
return 0;
164168
} else if (ZSTR_LEN(ms->data) < (size_t)(-offset)) {
165169
ms->fpos = 0;
@@ -169,6 +173,7 @@ static int php_stream_memory_seek(php_stream *stream, zend_off_t offset, int whe
169173
ms->fpos = ZSTR_LEN(ms->data) + offset;
170174
*newoffs = ms->fpos;
171175
stream->eof = 0;
176+
stream->fatal_error = 0;
172177
return 0;
173178
}
174179
default:

‎main/streams/streams.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
636636
/* some fatal error. Theoretically, the stream is borked, so all
637637
* further reads should fail. */
638638
stream->eof = 1;
639+
stream->fatal_error = 1;
639640
/* free all data left in brigades */
640641
while ((bucket = brig_inp->head)) {
641642
/* 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,
10091010
}
10101011
}
10111012

1012-
php_stream_fill_read_buffer(stream, toread);
1013+
if (php_stream_fill_read_buffer(stream, toread) == FAILURE && stream->fatal_error) {
1014+
if (grow_mode) {
1015+
efree(bufstart);
1016+
}
1017+
return NULL;
1018+
}
10131019

10141020
if (stream->writepos - stream->readpos == 0) {
10151021
break;
@@ -1084,7 +1090,9 @@ PHPAPI zend_string *php_stream_get_record(php_stream *stream, size_t maxlen, con
10841090

10851091
to_read_now = MIN(maxlen - buffered_len, stream->chunk_size);
10861092

1087-
php_stream_fill_read_buffer(stream, buffered_len + to_read_now);
1093+
if (php_stream_fill_read_buffer(stream, buffered_len + to_read_now) == FAILURE && stream->fatal_error) {
1094+
return NULL;
1095+
}
10881096

10891097
just_read = STREAM_BUFFERED_AMOUNT(stream) - buffered_len;
10901098

@@ -1357,6 +1365,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
13571365
stream->readpos += offset; /* if offset = ..., then readpos = writepos */
13581366
stream->position += offset;
13591367
stream->eof = 0;
1368+
stream->fatal_error = 0;
13601369
return 0;
13611370
}
13621371
break;
@@ -1366,6 +1375,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
13661375
stream->readpos += offset - stream->position;
13671376
stream->position = offset;
13681377
stream->eof = 0;
1378+
stream->fatal_error = 0;
13691379
return 0;
13701380
}
13711381
break;
@@ -1400,6 +1410,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
14001410
if (((stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) || ret == 0) {
14011411
if (ret == 0) {
14021412
stream->eof = 0;
1413+
stream->fatal_error = 0;
14031414
}
14041415

14051416
/* invalidate the buffer contents */
@@ -1422,6 +1433,7 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
14221433
offset -= didread;
14231434
}
14241435
stream->eof = 0;
1436+
stream->fatal_error = 0;
14251437
return 0;
14261438
}
14271439

0 commit comments

Comments
 (0)