From 32547f11271ebd9a4f29202179e1cc5e12c333a8 Mon Sep 17 00:00:00 2001 From: Saki Takamachi <34942839+SakiTakamachi@users.noreply.github.com> Date: Fri, 14 Mar 2025 17:52:50 +0900 Subject: [PATCH 1/2] ext/bcmath: If the result is `0`, `n_scale` is set to `0`. (#18056) --- NEWS | 1 + ext/bcmath/bcmath.c | 9 +++++---- ext/bcmath/libbcmath/src/bcmath.h | 2 +- ext/bcmath/libbcmath/src/div.c | 1 + ext/bcmath/libbcmath/src/divmod.c | 1 + ext/bcmath/libbcmath/src/recmul.c | 1 + ext/bcmath/libbcmath/src/round.c | 24 +++++++++++++++--------- 7 files changed, 25 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index f941ce90e47f1..bd7c6b3a26ae0 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ PHP NEWS - BCMath: . Simplify `bc_divide()` code. (SakiTakamachi) + . If the result is 0, n_scale is set to 0. (SakiTakamachi) - CLI: . Add --ini=diff to print INI settings changed from the builtin default. diff --git a/ext/bcmath/bcmath.c b/ext/bcmath/bcmath.c index 233045bd7cd7e..962f839ba83f4 100644 --- a/ext/bcmath/bcmath.c +++ b/ext/bcmath/bcmath.c @@ -807,8 +807,8 @@ PHP_FUNCTION(bcround) goto cleanup; } - bc_round(num, precision, mode, &result); - RETVAL_NEW_STR(bc_num2str_ex(result, result->n_scale)); + size_t scale = bc_round(num, precision, mode, &result); + RETVAL_NEW_STR(bc_num2str_ex(result, scale)); cleanup: { bc_free_num(&num); @@ -1799,9 +1799,10 @@ PHP_METHOD(BcMath_Number, round) bcmath_number_obj_t *intern = get_bcmath_number_from_zval(ZEND_THIS); bc_num ret = NULL; - bc_round(intern->num, precision, rounding_mode, &ret); + size_t scale = bc_round(intern->num, precision, rounding_mode, &ret); + bc_rm_trailing_zeros(ret); - bcmath_number_obj_t *new_intern = bcmath_number_new_obj(ret, ret->n_scale); + bcmath_number_obj_t *new_intern = bcmath_number_new_obj(ret, scale); RETURN_OBJ(&new_intern->std); } diff --git a/ext/bcmath/libbcmath/src/bcmath.h b/ext/bcmath/libbcmath/src/bcmath.h index f7e14019bd336..1f05ad51f7f26 100644 --- a/ext/bcmath/libbcmath/src/bcmath.h +++ b/ext/bcmath/libbcmath/src/bcmath.h @@ -157,7 +157,7 @@ bool bc_divmod(bc_num num1, bc_num num2, bc_num *quo, bc_num *rem, size_t scale) bc_num bc_floor_or_ceil(bc_num num, bool is_floor); -void bc_round(bc_num num, zend_long places, zend_long mode, bc_num *result); +size_t bc_round(bc_num num, zend_long places, zend_long mode, bc_num *result); typedef enum { OK, diff --git a/ext/bcmath/libbcmath/src/div.c b/ext/bcmath/libbcmath/src/div.c index cda452569dedc..ec7619fb77090 100644 --- a/ext/bcmath/libbcmath/src/div.c +++ b/ext/bcmath/libbcmath/src/div.c @@ -430,6 +430,7 @@ bool bc_divide(bc_num numerator, bc_num divisor, bc_num *quot, size_t scale) _bc_rm_leading_zeros(*quot); if (bc_is_zero(*quot)) { (*quot)->n_sign = PLUS; + (*quot)->n_scale = 0; } else { (*quot)->n_sign = numerator->n_sign == divisor->n_sign ? PLUS : MINUS; } diff --git a/ext/bcmath/libbcmath/src/divmod.c b/ext/bcmath/libbcmath/src/divmod.c index 294a281b2e687..477ec30e916ea 100644 --- a/ext/bcmath/libbcmath/src/divmod.c +++ b/ext/bcmath/libbcmath/src/divmod.c @@ -74,6 +74,7 @@ bool bc_divmod(bc_num num1, bc_num num2, bc_num *quot, bc_num *rem, size_t scale (*rem)->n_scale = MIN(scale, (*rem)->n_scale); if (bc_is_zero(*rem)) { (*rem)->n_sign = PLUS; + (*rem)->n_scale = 0; } return true; diff --git a/ext/bcmath/libbcmath/src/recmul.c b/ext/bcmath/libbcmath/src/recmul.c index b92a1045ac3b5..de06a4ca037ec 100644 --- a/ext/bcmath/libbcmath/src/recmul.c +++ b/ext/bcmath/libbcmath/src/recmul.c @@ -264,6 +264,7 @@ bc_num bc_multiply(bc_num n1, bc_num n2, size_t scale) _bc_rm_leading_zeros(prod); if (bc_is_zero(prod)) { prod->n_sign = PLUS; + prod->n_scale = 0; } return prod; } diff --git a/ext/bcmath/libbcmath/src/round.c b/ext/bcmath/libbcmath/src/round.c index ac3c7c41a315e..44df6036cbe3b 100644 --- a/ext/bcmath/libbcmath/src/round.c +++ b/ext/bcmath/libbcmath/src/round.c @@ -18,7 +18,8 @@ #include "private.h" #include -void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) +/* Returns the scale of the value after rounding. */ +size_t bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) { /* clear result */ bc_free_num(result); @@ -43,19 +44,19 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) case PHP_ROUND_HALF_ODD: case PHP_ROUND_TOWARD_ZERO: *result = bc_copy_num(BCG(_zero_)); - return; + return 0; case PHP_ROUND_CEILING: if (num->n_sign == MINUS) { *result = bc_copy_num(BCG(_zero_)); - return; + return 0; } break; case PHP_ROUND_FLOOR: if (num->n_sign == PLUS) { *result = bc_copy_num(BCG(_zero_)); - return; + return 0; } break; @@ -67,7 +68,7 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) if (bc_is_zero(num)) { *result = bc_copy_num(BCG(_zero_)); - return; + return 0; } /* If precision is -3, it becomes 1000. */ @@ -78,7 +79,7 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) } (*result)->n_value[0] = 1; (*result)->n_sign = num->n_sign; - return; + return 0; } /* Just like bcadd('1', '1', 4) becomes '2.0000', it pads with zeros at the end if necessary. */ @@ -90,7 +91,7 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) (*result)->n_sign = num->n_sign; memcpy((*result)->n_value, num->n_value, num->n_len + num->n_scale); } - return; + return precision; } /* @@ -222,7 +223,12 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) } check_zero: - if (bc_is_zero(*result)) { - (*result)->n_sign = PLUS; + { + size_t scale = (*result)->n_scale; + if (bc_is_zero(*result)) { + (*result)->n_sign = PLUS; + (*result)->n_scale = 0; + } + return scale; } } From 1c182674b09b88cb3ca954740504ba57aa1826ad Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 3 Mar 2025 13:12:21 +0100 Subject: [PATCH 2/2] Destroy temporary module classes in reverse order We destroy classes of dl()'ed modules in clean_module_classes(), during shutdown. Child classes of a module use structures of the parent class (such as inherited properties), which are destroyed earlier, so we have a use-after-free when destroying a child class. Here I destroy classes in reverse order, as it is done in zend_shutdown() for persistent classes. Fixes GH-17961 Fixes GH-15367 --- NEWS | 6 ++++ Zend/zend_API.c | 23 ++++++-------- ext/dl_test/dl_test.c | 12 ++++++++ ext/dl_test/dl_test.stub.php | 12 ++++++++ ext/dl_test/dl_test_arginfo.h | 58 ++++++++++++++++++++++++++++++++++- 5 files changed, 97 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index a6e51761301b8..3f4b5aa2bc83a 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,12 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.20 +- Core: + . Fixed bug GH-17961 (use-after-free during dl()'ed module class destruction). + (Arnaud) + . Fixed bug GH-15367 (dl() of module with aliased class crashes in shutdown). + (Arnaud) + - DOM: . Fix weird unpack behaviour in DOM. (nielsdos) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 13c640a64dd51..47de6e7897546 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -22,6 +22,7 @@ #include "zend.h" #include "zend_execute.h" #include "zend_API.h" +#include "zend_hash.h" #include "zend_modules.h" #include "zend_extensions.h" #include "zend_constants.h" @@ -3111,21 +3112,17 @@ ZEND_API zend_result zend_get_module_started(const char *module_name) /* {{{ */ } /* }}} */ -static int clean_module_class(zval *el, void *arg) /* {{{ */ -{ - zend_class_entry *ce = (zend_class_entry *)Z_PTR_P(el); - int module_number = *(int *)arg; - if (ce->type == ZEND_INTERNAL_CLASS && ce->info.internal.module->module_number == module_number) { - return ZEND_HASH_APPLY_REMOVE; - } else { - return ZEND_HASH_APPLY_KEEP; - } -} -/* }}} */ - static void clean_module_classes(int module_number) /* {{{ */ { - zend_hash_apply_with_argument(EG(class_table), clean_module_class, (void *) &module_number); + /* Child classes may reuse structures from parent classes, so destroy in reverse order. */ + Bucket *bucket; + ZEND_HASH_REVERSE_FOREACH_BUCKET(EG(class_table), bucket) { + zend_class_entry *ce = Z_CE(bucket->val); + if (ce->type == ZEND_INTERNAL_CLASS && ce->info.internal.module->module_number == module_number) { + zend_hash_del_bucket(EG(class_table), bucket); + } + } ZEND_HASH_FOREACH_END(); + } /* }}} */ diff --git a/ext/dl_test/dl_test.c b/ext/dl_test/dl_test.c index 1121a96b9df8d..1176a874d6027 100644 --- a/ext/dl_test/dl_test.c +++ b/ext/dl_test/dl_test.c @@ -92,10 +92,22 @@ PHP_METHOD(DlTest, test) RETURN_STR(retval); } +PHP_METHOD(DlTestSuperClass, test) +{ + ZEND_PARSE_PARAMETERS_NONE(); + + RETURN_NULL(); +} + /* {{{ PHP_MINIT_FUNCTION */ PHP_MINIT_FUNCTION(dl_test) { + zend_class_entry *ce; + register_class_DlTest(); + ce = register_class_DlTestSuperClass(); + register_class_DlTestSubClass(ce); + register_class_DlTestAliasedClass(); /* Test backwards compatibility */ if (getenv("PHP_DL_TEST_USE_OLD_REGISTER_INI_ENTRIES")) { diff --git a/ext/dl_test/dl_test.stub.php b/ext/dl_test/dl_test.stub.php index cd8b3916bae2e..c2d8ff577780f 100644 --- a/ext/dl_test/dl_test.stub.php +++ b/ext/dl_test/dl_test.stub.php @@ -12,3 +12,15 @@ function dl_test_test2(string $str = ""): string {} class DlTest { public function test(string $str = ""): string {} } + +class DlTestSuperClass { + public int $a; + public function test(string $str = ""): string {} +} + +class DlTestSubClass extends DlTestSuperClass { +} + +/** @alias DlTestClassAlias */ +class DlTestAliasedClass { +} diff --git a/ext/dl_test/dl_test_arginfo.h b/ext/dl_test/dl_test_arginfo.h index 0618bbdb222ca..549ac220b58bc 100644 --- a/ext/dl_test/dl_test_arginfo.h +++ b/ext/dl_test/dl_test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 2dbacf5282b0f8e53923ac70495c2da43c7237e3 */ + * Stub hash: 0641a8eeff00e6c8083fe4a8639f970e3ba80db9 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_dl_test_test1, 0, 0, IS_VOID, 0) ZEND_END_ARG_INFO() @@ -10,10 +10,13 @@ ZEND_END_ARG_INFO() #define arginfo_class_DlTest_test arginfo_dl_test_test2 +#define arginfo_class_DlTestSuperClass_test arginfo_dl_test_test2 + ZEND_FUNCTION(dl_test_test1); ZEND_FUNCTION(dl_test_test2); ZEND_METHOD(DlTest, test); +ZEND_METHOD(DlTestSuperClass, test); static const zend_function_entry ext_functions[] = { @@ -28,6 +31,22 @@ static const zend_function_entry class_DlTest_methods[] = { ZEND_FE_END }; + +static const zend_function_entry class_DlTestSuperClass_methods[] = { + ZEND_ME(DlTestSuperClass, test, arginfo_class_DlTestSuperClass_test, ZEND_ACC_PUBLIC) + ZEND_FE_END +}; + + +static const zend_function_entry class_DlTestSubClass_methods[] = { + ZEND_FE_END +}; + + +static const zend_function_entry class_DlTestAliasedClass_methods[] = { + ZEND_FE_END +}; + static zend_class_entry *register_class_DlTest(void) { zend_class_entry ce, *class_entry; @@ -37,3 +56,40 @@ static zend_class_entry *register_class_DlTest(void) return class_entry; } + +static zend_class_entry *register_class_DlTestSuperClass(void) +{ + zend_class_entry ce, *class_entry; + + INIT_CLASS_ENTRY(ce, "DlTestSuperClass", class_DlTestSuperClass_methods); + class_entry = zend_register_internal_class_ex(&ce, NULL); + + zval property_a_default_value; + ZVAL_UNDEF(&property_a_default_value); + zend_string *property_a_name = zend_string_init("a", sizeof("a") - 1, 1); + zend_declare_typed_property(class_entry, property_a_name, &property_a_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_LONG)); + zend_string_release(property_a_name); + + return class_entry; +} + +static zend_class_entry *register_class_DlTestSubClass(zend_class_entry *class_entry_DlTestSuperClass) +{ + zend_class_entry ce, *class_entry; + + INIT_CLASS_ENTRY(ce, "DlTestSubClass", class_DlTestSubClass_methods); + class_entry = zend_register_internal_class_ex(&ce, class_entry_DlTestSuperClass); + + return class_entry; +} + +static zend_class_entry *register_class_DlTestAliasedClass(void) +{ + zend_class_entry ce, *class_entry; + + INIT_CLASS_ENTRY(ce, "DlTestAliasedClass", class_DlTestAliasedClass_methods); + class_entry = zend_register_internal_class_ex(&ce, NULL); + zend_register_class_alias("DlTestClassAlias", class_entry); + + return class_entry; +}