Skip to content

Commit 62dce97

Browse files
committed
Require non-negative length in stream_get_contents()
If the length is not -1, require it to be non-negative. Using such lengths doesn't make sense (as only -1 is special-case to read in chunks, anything else will end up doing a huge upfront allocation) and can lead to string allocation overflow. A similar check is already in place for file_get_contents(). That one does not allow -1 (and uses null instead), but this function is explicitly specified to accept -1, so stick to that behavior.
1 parent 1b7ee6d commit 62dce97

File tree

2 files changed

+21
-0
lines changed

2 files changed

+21
-0
lines changed

ext/standard/streamsfuncs.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,11 @@ PHP_FUNCTION(stream_get_contents)
442442
Z_PARAM_LONG(desiredpos)
443443
ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE);
444444

445+
if (maxlen < 0 && maxlen != PHP_STREAM_COPY_ALL) {
446+
php_error_docref(NULL, E_WARNING, "Length must be greater than or equal to zero, or -1");
447+
RETURN_FALSE;
448+
}
449+
445450
php_stream_from_zval(stream, zsrc);
446451

447452
if (desiredpos >= 0) {
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
stream_get_contents() with negative max length
3+
--FILE--
4+
<?php
5+
6+
$tmp = tmpfile();
7+
fwrite($tmp, "abcd");
8+
var_dump(stream_get_contents($tmp, 2, 1));
9+
var_dump(stream_get_contents($tmp, -2));
10+
11+
?>
12+
--EXPECTF--
13+
string(2) "bc"
14+
15+
Warning: stream_get_contents(): Length must be greater than or equal to zero, or -1 in %s on line %d
16+
bool(false)

0 commit comments

Comments
 (0)