From e9a447f9b30b05f462252a5527eaab46b6e9963e Mon Sep 17 00:00:00 2001 From: Go Kudo Date: Fri, 22 Jul 2022 12:40:42 +0900 Subject: [PATCH 01/10] random: fix __construct() call twice --- ext/random/randomizer.c | 9 +++-- .../tests/03_randomizer/construct_twice.phpt | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 ext/random/tests/03_randomizer/construct_twice.phpt diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 722fbd0b5c4b0..0915309d03ae8 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -42,7 +42,7 @@ static inline void randomizer_common_init(php_random_randomizer *randomizer, zen zend_string *mname; zend_function *generate_method; - mname = zend_string_init("generate", strlen("generate"), 0); + mname = zend_string_init("generate", sizeof("generate") - 1, 0); generate_method = zend_hash_find_ptr(&engine_object->ce->function_table, mname); zend_string_release(mname); @@ -70,6 +70,11 @@ PHP_METHOD(Random_Randomizer, __construct) Z_PARAM_OBJ_OF_CLASS_OR_NULL(engine_object, random_ce_Random_Engine); ZEND_PARSE_PARAMETERS_END(); + if (randomizer->algo) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot call constructor twice"); + RETURN_THROWS(); + } + /* Create default RNG instance */ if (!engine_object) { engine_object = random_ce_Random_Engine_Secure->create_object(random_ce_Random_Engine_Secure); @@ -80,7 +85,7 @@ PHP_METHOD(Random_Randomizer, __construct) ZVAL_OBJ(&zengine_object, engine_object); - zend_update_property(random_ce_Random_Randomizer, Z_OBJ_P(ZEND_THIS), "engine", strlen("engine"), &zengine_object); + zend_update_property(random_ce_Random_Randomizer, Z_OBJ_P(ZEND_THIS), "engine", sizeof("engine") - 1, &zengine_object); randomizer_common_init(randomizer, engine_object); } diff --git a/ext/random/tests/03_randomizer/construct_twice.phpt b/ext/random/tests/03_randomizer/construct_twice.phpt new file mode 100644 index 0000000000000..97dca2662b35e --- /dev/null +++ b/ext/random/tests/03_randomizer/construct_twice.phpt @@ -0,0 +1,36 @@ +--TEST-- +Random: Randomizer: __construct() twice +--FILE-- +__construct(); +} catch (\BadMethodCallException $e) { + echo $e->getMessage() . PHP_EOL; +} + +try { + (new \Random\Randomizer( + new class () implements \Random\Engine { + public function generate(): string + { + return \random_bytes(4); /* 32-bit */ + } + } + ))->__construct( + new class () implements \Random\Engine { + public function generate(): string + { + return \random_bytes(4); /* 32-bit */ + } + } + ); +} catch (\BadMethodCallException $e) { + echo $e->getMessage() . PHP_EOL; +} + +die('success'); +?> +--EXPECT-- +Cannot call constructor twice +Cannot call constructor twice +success From 4adebfd183561de69edd2e404b7723d1b9899fc0 Mon Sep 17 00:00:00 2001 From: Go Kudo Date: Fri, 22 Jul 2022 12:42:25 +0900 Subject: [PATCH 02/10] update NEWS --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 637a848f90c7f..014cb8d950029 100644 --- a/NEWS +++ b/NEWS @@ -63,6 +63,7 @@ PHP NEWS - Random: . Added new random extension. (Go Kudo) . Fixed bug GH-9066 (signed integer overflow). (zeriyoshi) + . Fixed bug GH-9089 ()Memory leak when calling Randomizer::__construct() on an existing object). (zeriyoshi) - SPL: . Widen iterator_to_array() and iterator_count()'s $iterator parameter to From a095e45ab380681066c8fa20eec0adc1cd39f9a4 Mon Sep 17 00:00:00 2001 From: Go Kudo Date: Fri, 22 Jul 2022 12:59:57 +0900 Subject: [PATCH 03/10] [CI skip] fix typo --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 014cb8d950029..03866b88066af 100644 --- a/NEWS +++ b/NEWS @@ -63,7 +63,7 @@ PHP NEWS - Random: . Added new random extension. (Go Kudo) . Fixed bug GH-9066 (signed integer overflow). (zeriyoshi) - . Fixed bug GH-9089 ()Memory leak when calling Randomizer::__construct() on an existing object). (zeriyoshi) + . Fixed bug GH-9089 (Memory leak when calling Randomizer::__construct() on an existing object). (zeriyoshi) - SPL: . Widen iterator_to_array() and iterator_count()'s $iterator parameter to From 4afc9db8162b58051b271e2dd292aff09a6c3747 Mon Sep 17 00:00:00 2001 From: Go Kudo Date: Fri, 22 Jul 2022 18:31:50 +0900 Subject: [PATCH 04/10] fix: replace all strlen -> sizeof - 1 --- ext/random/engine_mt19937.c | 2 +- ext/random/randomizer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c index 63613e2fcc878..02fade6b6a0cb 100644 --- a/ext/random/engine_mt19937.c +++ b/ext/random/engine_mt19937.c @@ -394,7 +394,7 @@ PHP_METHOD(Random_Engine_Mt19937, __debugInfo) zend_throw_exception(NULL, "Engine serialize failed", 0); RETURN_THROWS(); } - zend_hash_str_add(Z_ARR_P(return_value), "__states", strlen("__states"), &t); + zend_hash_str_add(Z_ARR_P(return_value), "__states", sizeof("__states") - 1, &t); } } /* }}} */ diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 0915309d03ae8..3e7e37949ba6a 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -279,7 +279,7 @@ PHP_METHOD(Random_Randomizer, __unserialize) } object_properties_load(&randomizer->std, Z_ARRVAL_P(members_zv)); - zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", strlen("engine"), 0, NULL); + zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", sizeof("engine") - 1, 0, NULL); if (Z_TYPE_P(zengine) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(zengine), random_ce_Random_Engine)) { zend_throw_exception(NULL, "Incomplete or ill-formed serialization data", 0); RETURN_THROWS(); From e9b9118c2f7ca04b02b16afc4da69c3e7f2ed4dd Mon Sep 17 00:00:00 2001 From: Go Kudo Date: Sat, 23 Jul 2022 23:44:07 +0900 Subject: [PATCH 05/10] Update ext/random/tests/03_randomizer/construct_twice.phpt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Düsterhus --- ext/random/tests/03_randomizer/construct_twice.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/random/tests/03_randomizer/construct_twice.phpt b/ext/random/tests/03_randomizer/construct_twice.phpt index 97dca2662b35e..c52c12ffa4e8f 100644 --- a/ext/random/tests/03_randomizer/construct_twice.phpt +++ b/ext/random/tests/03_randomizer/construct_twice.phpt @@ -1,5 +1,5 @@ --TEST-- -Random: Randomizer: __construct() twice +Random: Randomizer: Disallow manually calling __construct --FILE-- Date: Sat, 23 Jul 2022 23:47:56 +0900 Subject: [PATCH 06/10] Revert "fix: replace all strlen -> sizeof - 1" This reverts commit 4afc9db8162b58051b271e2dd292aff09a6c3747. --- ext/random/engine_mt19937.c | 2 +- ext/random/randomizer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c index 02fade6b6a0cb..63613e2fcc878 100644 --- a/ext/random/engine_mt19937.c +++ b/ext/random/engine_mt19937.c @@ -394,7 +394,7 @@ PHP_METHOD(Random_Engine_Mt19937, __debugInfo) zend_throw_exception(NULL, "Engine serialize failed", 0); RETURN_THROWS(); } - zend_hash_str_add(Z_ARR_P(return_value), "__states", sizeof("__states") - 1, &t); + zend_hash_str_add(Z_ARR_P(return_value), "__states", strlen("__states"), &t); } } /* }}} */ diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 3e7e37949ba6a..0915309d03ae8 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -279,7 +279,7 @@ PHP_METHOD(Random_Randomizer, __unserialize) } object_properties_load(&randomizer->std, Z_ARRVAL_P(members_zv)); - zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", sizeof("engine") - 1, 0, NULL); + zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", strlen("engine"), 0, NULL); if (Z_TYPE_P(zengine) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(zengine), random_ce_Random_Engine)) { zend_throw_exception(NULL, "Incomplete or ill-formed serialization data", 0); RETURN_THROWS(); From 31b7f9a68670fae9a5329ab489ea8d6fb860a32c Mon Sep 17 00:00:00 2001 From: zeriyoshi Date: Sat, 23 Jul 2022 23:50:04 +0900 Subject: [PATCH 07/10] random: revert strlen -> sizeof - 1 --- ext/random/randomizer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 0915309d03ae8..a763d8e71c987 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -85,7 +85,7 @@ PHP_METHOD(Random_Randomizer, __construct) ZVAL_OBJ(&zengine_object, engine_object); - zend_update_property(random_ce_Random_Randomizer, Z_OBJ_P(ZEND_THIS), "engine", sizeof("engine") - 1, &zengine_object); + zend_update_property(random_ce_Random_Randomizer, Z_OBJ_P(ZEND_THIS), "engine", strlen("engine"), &zengine_object); randomizer_common_init(randomizer, engine_object); } From cb5f7aeb8040e04807ce975a42497a74b84f244c Mon Sep 17 00:00:00 2001 From: zeriyoshi Date: Sat, 23 Jul 2022 23:50:38 +0900 Subject: [PATCH 08/10] random: test: fix test readability --- .../tests/03_randomizer/construct_twice.phpt | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/ext/random/tests/03_randomizer/construct_twice.phpt b/ext/random/tests/03_randomizer/construct_twice.phpt index c52c12ffa4e8f..ccb831dfd4ab5 100644 --- a/ext/random/tests/03_randomizer/construct_twice.phpt +++ b/ext/random/tests/03_randomizer/construct_twice.phpt @@ -2,6 +2,15 @@ Random: Randomizer: Disallow manually calling __construct --FILE-- __construct(); } catch (\BadMethodCallException $e) { @@ -9,21 +18,15 @@ try { } try { - (new \Random\Randomizer( - new class () implements \Random\Engine { - public function generate(): string - { - return \random_bytes(4); /* 32-bit */ - } - } - ))->__construct( - new class () implements \Random\Engine { - public function generate(): string - { - return \random_bytes(4); /* 32-bit */ - } - } - ); + $r = new \Random\Randomizer(new \Random\Engine\Xoshiro256StarStar()); + $r->__construct(new \Random\Engine\PcgOneseq128XslRr64()); +} catch (\BadMethodCallException $e) { + echo $e->getMessage() . PHP_EOL; +} + +try { + $r = new \Random\Randomizer(new \UserEngine()); + $r->__construct(new \UserEngine()); } catch (\BadMethodCallException $e) { echo $e->getMessage() . PHP_EOL; } @@ -33,4 +36,5 @@ die('success'); --EXPECT-- Cannot call constructor twice Cannot call constructor twice +Cannot call constructor twice success From da07c171e3ac4463a7d8931f02e865ee31b64c92 Mon Sep 17 00:00:00 2001 From: zeriyoshi Date: Sat, 23 Jul 2022 23:55:57 +0900 Subject: [PATCH 09/10] random: revert sizeof - 1 in randomizer.c --- ext/random/randomizer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index a763d8e71c987..d5f4b46b2b28b 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -42,7 +42,7 @@ static inline void randomizer_common_init(php_random_randomizer *randomizer, zen zend_string *mname; zend_function *generate_method; - mname = zend_string_init("generate", sizeof("generate") - 1, 0); + mname = zend_string_init("generate", strlen("generate"), 0); generate_method = zend_hash_find_ptr(&engine_object->ce->function_table, mname); zend_string_release(mname); From 8708360174d5962ca561a88653bdea884676b8c8 Mon Sep 17 00:00:00 2001 From: zeriyoshi Date: Sun, 24 Jul 2022 00:23:56 +0900 Subject: [PATCH 10/10] Update NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index ecde7c963a61c..30d0c79280932 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,8 @@ PHP NEWS . Fixed bug GH-9083 (undefined behavior during shifting). (timwolla) . Fixed bug GH-9088, GH-9056 (incorrect expansion of bytes when generating uniform integers within a given range). (timwolla) + . Fixed bug GH-9089 (Fix memory leak on Randomizer::__construct() + call twice) (zeriyoshi) 21 Jul 2022, PHP 8.2.0beta1