From 10660d8e7ef888d51fba8807c635abc05888ed6d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 21 Nov 2024 06:35:12 +0000 Subject: [PATCH 1/2] Fix GH-16878: gmp_fact overflow on memory allocation attempt. --- ext/gmp/gmp.c | 16 ++++++++++++---- ext/gmp/tests/gh16878.phpt | 21 +++++++++++++++++++++ ext/gmp/tests/gmp_fact.phpt | 8 ++++---- 3 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 ext/gmp/tests/gh16878.phpt diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index c9603c8fb21e8..fa9b6861dc303 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -1281,9 +1281,16 @@ ZEND_FUNCTION(gmp_fact) RETURN_THROWS(); } +#if SIZEOF_SIZE_T == 4 + const zend_long maxbits = ULONG_MAX / GMP_NUMB_BITS; +#else + const zend_long maxbits = INT_MAX; +#endif + + if (Z_TYPE_P(a_arg) == IS_LONG) { - if (Z_LVAL_P(a_arg) < 0) { - zend_argument_value_error(1, "must be greater than or equal to 0"); + if (Z_LVAL_P(a_arg) < 0 || Z_LVAL_P(a_arg) > maxbits) { + zend_argument_value_error(1, "must be between 0 and " ZEND_LONG_FMT, maxbits); RETURN_THROWS(); } } else { @@ -1291,10 +1298,11 @@ ZEND_FUNCTION(gmp_fact) gmp_temp_t temp_a; FETCH_GMP_ZVAL(gmpnum, a_arg, temp_a, 1); + long r = mpz_get_si(gmpnum); FREE_GMP_TEMP(temp_a); - if (mpz_sgn(gmpnum) < 0) { - zend_argument_value_error(1, "must be greater than or equal to 0"); + if (r < 0 || r > maxbits) { + zend_argument_value_error(1, "must be between 0 and " ZEND_LONG_FMT, maxbits); RETURN_THROWS(); } } diff --git a/ext/gmp/tests/gh16878.phpt b/ext/gmp/tests/gh16878.phpt new file mode 100644 index 0000000000000..566931cd06b7d --- /dev/null +++ b/ext/gmp/tests/gh16878.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-16878 (gmp_fact overflow) +--EXTENSIONS-- +gmp +--FILE-- +getMessage() . PHP_EOL; +} + +try { + gmp_fact(gmp_init(PHP_INT_MAX)); +} catch (\ValueError $e) { + echo $e->getMessage(); +} +?> +--EXPECTF-- +gmp_fact(): Argument #1 ($num) must be between 0 and %d +gmp_fact(): Argument #1 ($num) must be between 0 and %d diff --git a/ext/gmp/tests/gmp_fact.phpt b/ext/gmp/tests/gmp_fact.phpt index e039314549405..b28f572c6975a 100644 --- a/ext/gmp/tests/gmp_fact.phpt +++ b/ext/gmp/tests/gmp_fact.phpt @@ -45,17 +45,17 @@ try { echo "Done\n"; ?> ---EXPECT-- +--EXPECTF-- string(1) "1" gmp_fact(): Argument #1 ($num) is not an integer string string(1) "1" -gmp_fact(): Argument #1 ($num) must be greater than or equal to 0 -gmp_fact(): Argument #1 ($num) must be greater than or equal to 0 +gmp_fact(): Argument #1 ($num) must be between 0 and %d +gmp_fact(): Argument #1 ($num) must be between 0 and %d string(19) "2432902008176640000" string(65) "30414093201713378043612608166064768844377641568960512000000000000" string(7) "3628800" string(1) "1" string(9) "479001600" -gmp_fact(): Argument #1 ($num) must be greater than or equal to 0 +gmp_fact(): Argument #1 ($num) must be between 0 and %d gmp_fact(): Argument #1 ($num) must be of type GMP|string|int, array given Done From ac20c825932463939dabb1f0bf3a70923ff8b1da Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 21 Nov 2024 12:51:23 +0000 Subject: [PATCH 2/2] changes from feedback --- ext/gmp/gmp.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index fa9b6861dc303..7669d76dc73f4 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -47,6 +47,12 @@ #define GMP_BIG_ENDIAN (1 << 3) #define GMP_NATIVE_ENDIAN (1 << 4) +#if SIZEOF_SIZE_T == 4 +#define GMP_ALLOC_MAXBITS (ULONG_MAX / GMP_NUMB_BITS) +#else +#define GMP_ALLOC_MAXBITS INT_MAX +#endif + #include "gmp_arginfo.h" ZEND_DECLARE_MODULE_GLOBALS(gmp) @@ -1276,21 +1282,17 @@ ZEND_FUNCTION(gmp_fact) { zval *a_arg; mpz_ptr gmpnum_result; + zend_long val; if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &a_arg) == FAILURE){ RETURN_THROWS(); } -#if SIZEOF_SIZE_T == 4 - const zend_long maxbits = ULONG_MAX / GMP_NUMB_BITS; -#else - const zend_long maxbits = INT_MAX; -#endif - if (Z_TYPE_P(a_arg) == IS_LONG) { - if (Z_LVAL_P(a_arg) < 0 || Z_LVAL_P(a_arg) > maxbits) { - zend_argument_value_error(1, "must be between 0 and " ZEND_LONG_FMT, maxbits); + val = Z_LVAL_P(a_arg); + if (val < 0 || val > GMP_ALLOC_MAXBITS) { + zend_argument_value_error(1, "must be between 0 and " ZEND_LONG_FMT, GMP_ALLOC_MAXBITS); RETURN_THROWS(); } } else { @@ -1298,17 +1300,18 @@ ZEND_FUNCTION(gmp_fact) gmp_temp_t temp_a; FETCH_GMP_ZVAL(gmpnum, a_arg, temp_a, 1); - long r = mpz_get_si(gmpnum); FREE_GMP_TEMP(temp_a); - if (r < 0 || r > maxbits) { - zend_argument_value_error(1, "must be between 0 and " ZEND_LONG_FMT, maxbits); + (void)gmpnum; + val = zval_get_long(a_arg); + if (val < 0 || val > GMP_ALLOC_MAXBITS) { + zend_argument_value_error(1, "must be between 0 and " ZEND_LONG_FMT, GMP_ALLOC_MAXBITS); RETURN_THROWS(); } } INIT_GMP_RETVAL(gmpnum_result); - mpz_fac_ui(gmpnum_result, zval_get_long(a_arg)); + mpz_fac_ui(gmpnum_result, val); } /* }}} */ @@ -1867,14 +1870,8 @@ ZEND_FUNCTION(gmp_random_bits) RETURN_THROWS(); } -#if SIZEOF_SIZE_T == 4 - const zend_long maxbits = ULONG_MAX / GMP_NUMB_BITS; -#else - const zend_long maxbits = INT_MAX; -#endif - - if (bits <= 0 || bits > maxbits) { - zend_argument_value_error(1, "must be between 1 and " ZEND_LONG_FMT, maxbits); + if (bits <= 0 || bits > GMP_ALLOC_MAXBITS) { + zend_argument_value_error(1, "must be between 1 and " ZEND_LONG_FMT, GMP_ALLOC_MAXBITS); RETURN_THROWS(); }