Fix GH-17650: realloc with size 0 in user_filters.c

[nielsdos]

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.

[devnexen]

LCTM

[cmb69]

Quick thought: what if we treat empty data like missing data?

[nielsdos]

Can you be a bit more specific please?

[cmb69]

I haven't really checked that, but the idea is

 ext/standard/user_filters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c
index 737237f663..8997e9c038 100644
--- a/ext/standard/user_filters.c
+++ b/ext/standard/user_filters.c
@@ -393,7 +393,7 @@ static void php_stream_bucket_attach(int append, INTERNAL_FUNCTION_PARAMETERS)
 		RETURN_THROWS();
 	}
 
-	if (NULL != (pzdata = zend_hash_str_find_deref(Z_OBJPROP_P(zobject), "data", sizeof("data")-1)) && Z_TYPE_P(pzdata) == IS_STRING) {
+	if (NULL != (pzdata = zend_hash_str_find_deref(Z_OBJPROP_P(zobject), "data", sizeof("data")-1)) && Z_TYPE_P(pzdata) == IS_STRING && Z_STRLEN_P(data) > 0) {
 		if (!bucket->own_buf) {
 			bucket = php_stream_bucket_make_writeable(bucket);
 		}

[nielsdos]

@cmb69 That's wrong, but the reason why it's wrong is not revealed by the test. I've updated the test so that would now be revealed. The difference with my approach and yours is that my approach won't output anything while yours will output the input data. Given that the data field of the bucket is overwritten by the code to an empty string, I think the output created by my approach is better.

[cmb69]

Thanks for the explanation (makes perfect sense), and amending the test.