From 75cea65c997a7d580abc41e61db2b1de15ee66c3 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 31 May 2025 12:14:27 +0200
Subject: [PATCH] Fix reference type confusion and leak in user random engine
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes GH-18718.

Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
---
 NEWS                                          |  4 +++
 ext/random/engine_user.c                      | 12 ++++++---
 .../02_engine/user_reference_return.phpt      | 25 +++++++++++++++++++
 .../engine_unsafe_empty_string.phpt           |  3 ++-
 4 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100644 ext/random/tests/02_engine/user_reference_return.phpt

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));
     }
 }