Skip to content

Commit

Permalink
Fix phpGH-17650: realloc with size 0 in user_filters.c
Browse files Browse the repository at this point in the history
If the returned buffer string is of length 0, then a realloc can happen
with length 0. However, the behaviour is implementation-defined.
From 7.20.3.1 of C11 spec:

> If the size of the space requested is zero, the behavior is
> implementation-defined: either a null pointer is returned,
> or the behavior is as if the size were some nonzero value,
> except that the returned pointer shall not be used to access an object

This is problematic for the test case on my system as it returns NULL,
causing a memleak and later using it in memcpy causing UB.
The bucket code is not prepared to handle a NULL pointer.
To solve this, we use MAX to clamp the size to 1 at the least.
  • Loading branch information
nielsdos committed Jan 31, 2025
1 parent f8b57ff commit 8e8b5df
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
33 changes: 33 additions & 0 deletions ext/standard/tests/streams/gh17650.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
GH-17650 (realloc with size 0 in user_filters.c)
--FILE--
<?php
class testfilter extends php_user_filter {
function filter($in, $out, &$consumed, $closing): int {
while ($bucket = stream_bucket_make_writeable($in)) {
$bucket->data = '';
$consumed += strlen($bucket->data);
stream_bucket_append($out, $bucket);
}
return PSFS_PASS_ON;
}
}

stream_filter_register('testfilter','testfilter');

$text = "Hello There!";

$fp = fopen('php://memory', 'w+');
fwrite($fp, $text);

rewind($fp);
stream_filter_append($fp, 'testfilter', STREAM_FILTER_READ, 'testuserfilter');

while (fgets($fp));

fclose($fp);

echo "Done\n";
?>
--EXPECT--
Done
2 changes: 1 addition & 1 deletion ext/standard/user_filters.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ static void php_stream_bucket_attach(int append, INTERNAL_FUNCTION_PARAMETERS)
bucket = php_stream_bucket_make_writeable(bucket);
}
if (bucket->buflen != Z_STRLEN_P(pzdata)) {
bucket->buf = perealloc(bucket->buf, Z_STRLEN_P(pzdata), bucket->is_persistent);
bucket->buf = perealloc(bucket->buf, MAX(Z_STRLEN_P(pzdata), 1), bucket->is_persistent);
bucket->buflen = Z_STRLEN_P(pzdata);
}
memcpy(bucket->buf, Z_STRVAL_P(pzdata), bucket->buflen);
Expand Down

0 comments on commit 8e8b5df

Please sign in to comment.