From 6423adbca90db694315d2e7032445b6bb3a7391b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 8 Sep 2020 15:09:30 +0200 Subject: [PATCH 1/3] Fix #72941: Modifying bucket->data by-ref has no effect any longer To match the PHP 5 behavior, we have to explicitly cater to `data` being a reference. --- ext/standard/tests/filters/bug72941.phpt | 35 ++++++++++++++++++++++++ ext/standard/user_filters.c | 20 ++++++++------ 2 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 ext/standard/tests/filters/bug72941.phpt diff --git a/ext/standard/tests/filters/bug72941.phpt b/ext/standard/tests/filters/bug72941.phpt new file mode 100644 index 0000000000000..464b0050793c4 --- /dev/null +++ b/ext/standard/tests/filters/bug72941.phpt @@ -0,0 +1,35 @@ +--TEST-- +Bug #72941 (Modifying bucket->data by-ref has no effect any longer) +--FILE-- +rotate($bucket->data); + $consumed += $bucket->datalen; + stream_bucket_prepend($out, $bucket); + } + + return PSFS_PASS_ON; + } + + function rotate(&$data) + { + $n = strlen($data); + for ($i = 0; $i < $n - 1; ++$i) { + $data[$i] = $data[$i + 1]; + } + } +} + +stream_filter_register("rotator_notWorking", rotate_filter_nw::class); +$stream = fopen('php://memory', 'w+'); +fwrite($stream, 'hello, world'); +rewind($stream); +stream_filter_append($stream, "rotator_notWorking"); +var_dump(stream_get_contents($stream)); +?> +--EXPECT-- +string(12) "ello, worldd" diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index 584d055145eba..d5c5eeff1a0fd 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -448,15 +448,19 @@ static void php_stream_bucket_attach(int append, INTERNAL_FUNCTION_PARAMETERS) RETURN_FALSE; } - if (NULL != (pzdata = zend_hash_str_find(Z_OBJPROP_P(zobject), "data", sizeof("data")-1)) && Z_TYPE_P(pzdata) == IS_STRING) { - if (!bucket->own_buf) { - 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->buflen = Z_STRLEN_P(pzdata); + if (NULL != (pzdata = zend_hash_str_find(Z_OBJPROP_P(zobject), "data", sizeof("data")-1))) { + zval zdata; + ZVAL_COPY_DEREF(&zdata, pzdata); + if (Z_TYPE_P(&zdata) == IS_STRING) { + if (!bucket->own_buf) { + bucket = php_stream_bucket_make_writeable(bucket); + } + if (bucket->buflen != Z_STRLEN_P(&zdata)) { + bucket->buf = perealloc(bucket->buf, Z_STRLEN_P(&zdata), bucket->is_persistent); + bucket->buflen = Z_STRLEN_P(&zdata); + } + memcpy(bucket->buf, Z_STRVAL_P(&zdata), bucket->buflen); } - memcpy(bucket->buf, Z_STRVAL_P(pzdata), bucket->buflen); } if (append) { From 23f9fe806d9761d2177b909f262f97cc2e54e632 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 8 Sep 2020 15:41:45 +0200 Subject: [PATCH 2/3] Don't copy_deref since it would leak; actually no copy required at all --- ext/standard/user_filters.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index d5c5eeff1a0fd..bcfd5d0cc5230 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -448,19 +448,15 @@ static void php_stream_bucket_attach(int append, INTERNAL_FUNCTION_PARAMETERS) RETURN_FALSE; } - if (NULL != (pzdata = zend_hash_str_find(Z_OBJPROP_P(zobject), "data", sizeof("data")-1))) { - zval zdata; - ZVAL_COPY_DEREF(&zdata, pzdata); - if (Z_TYPE_P(&zdata) == IS_STRING) { - if (!bucket->own_buf) { - bucket = php_stream_bucket_make_writeable(bucket); - } - if (bucket->buflen != Z_STRLEN_P(&zdata)) { - bucket->buf = perealloc(bucket->buf, Z_STRLEN_P(&zdata), bucket->is_persistent); - bucket->buflen = Z_STRLEN_P(&zdata); - } - memcpy(bucket->buf, Z_STRVAL_P(&zdata), bucket->buflen); + if (NULL != (pzdata = zend_hash_str_find_deref(Z_OBJPROP_P(zobject), "data", sizeof("data")-1)) && Z_TYPE_P(pzdata) == IS_STRING) { + if (!bucket->own_buf) { + 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->buflen = Z_STRLEN_P(pzdata); } + memcpy(bucket->buf, Z_STRVAL_P(pzdata), bucket->buflen); } if (append) { From ebc2134d54b838bf6bf897379bc9325dc7e38beb Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 8 Sep 2020 16:39:16 +0200 Subject: [PATCH 3/3] zend_hash_str_find_deref() the bucket as well --- 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 bcfd5d0cc5230..030591d36a379 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -434,7 +434,7 @@ static void php_stream_bucket_attach(int append, INTERNAL_FUNCTION_PARAMETERS) Z_PARAM_OBJECT(zobject) ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE); - if (NULL == (pzbucket = zend_hash_str_find(Z_OBJPROP_P(zobject), "bucket", sizeof("bucket")-1))) { + if (NULL == (pzbucket = zend_hash_str_find_deref(Z_OBJPROP_P(zobject), "bucket", sizeof("bucket")-1))) { php_error_docref(NULL, E_WARNING, "Object has no bucket property"); RETURN_FALSE; }