Commit 75cea65c997 for php.net
commit 75cea65c997a7d580abc41e61db2b1de15ee66c3
Author: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat May 31 12:14:27 2025 +0200
Fix reference type confusion and leak in user random engine
Closes GH-18718.
Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
diff --git a/NEWS b/NEWS
index 4088849616b..02b90ff3dc7 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,10 @@ PHP NEWS
. Fix warning not being emitted when failure to cancel a query with
pg_cancel_query(). (Girgias)
+- Random:
+ . Fix reference type confusion and leak in user random engine.
+ (nielsdos, timwolla)
+
- Readline:
. Fix memory leak when calloc() fails in php_readline_completion_cb().
(nielsdos)
diff --git a/ext/random/engine_user.c b/ext/random/engine_user.c
index b45924d3bb7..ce68521c129 100644
--- a/ext/random/engine_user.c
+++ b/ext/random/engine_user.c
@@ -27,6 +27,7 @@ static uint64_t generate(php_random_status *status)
uint64_t result = 0;
size_t size;
zval retval;
+ zend_string *zstr;
zend_call_known_instance_method_with_0_params(s->generate_method, s->object, &retval);
@@ -34,8 +35,14 @@ static uint64_t generate(php_random_status *status)
return 0;
}
+ if (UNEXPECTED(Z_ISREF(retval))) {
+ zstr = Z_STR_P(Z_REFVAL(retval));
+ } else {
+ zstr = Z_STR(retval);
+ }
+
/* Store generated size in a state */
- size = Z_STRLEN(retval);
+ size = ZSTR_LEN(zstr);
/* Guard for over 64-bit results */
if (size > sizeof(uint64_t)) {
@@ -46,11 +53,10 @@ static uint64_t generate(php_random_status *status)
if (size > 0) {
/* Endianness safe copy */
for (size_t i = 0; i < size; i++) {
- result += ((uint64_t) (unsigned char) Z_STRVAL(retval)[i]) << (8 * i);
+ result += ((uint64_t) (unsigned char) ZSTR_VAL(zstr)[i]) << (8 * i);
}
} else {
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "A random engine must return a non-empty string");
- return 0;
}
zval_ptr_dtor(&retval);
diff --git a/ext/random/tests/02_engine/user_reference_return.phpt b/ext/random/tests/02_engine/user_reference_return.phpt
new file mode 100644
index 00000000000..f54e65bed75
--- /dev/null
+++ b/ext/random/tests/02_engine/user_reference_return.phpt
@@ -0,0 +1,25 @@
+--TEST--
+Random: Engine: User: Returning by reference works
+--FILE--
+<?php
+
+use Random\Engine;
+use Random\Randomizer;
+
+final class ReferenceEngine implements Engine
+{
+ private $field = 'abcdef';
+
+ public function &generate(): string
+ {
+ return $this->field;
+ }
+}
+
+$randomizer = new Randomizer(new ReferenceEngine());
+
+var_dump($randomizer->getBytes(64));
+
+?>
+--EXPECT--
+string(64) "abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcd"
diff --git a/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt b/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt
index 6aec7180b1e..8333df88d64 100644
--- a/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt
+++ b/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt
@@ -10,7 +10,8 @@ final class EmptyStringEngine implements Engine
{
public function generate(): string
{
- return '';
+ // Create a non-interned empty string.
+ return preg_replace('/./', '', random_bytes(4));
}
}