From 0c04ae6f01feb19b35c55f15f93a354814e855e4 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 12 Oct 2024 07:14:56 +0100 Subject: [PATCH 1/6] ext/gmp: gmp_pow fix FPE with large values. even without sanitizers, it is reproducible but with the following ``` = 0) { INIT_GMP_RETVAL(gmpnum_result); - if (exp >= INT_MAX) { - mpz_t base_num, exp_num, mod; - mpz_init(base_num); - mpz_init(exp_num); - mpz_init(mod); - mpz_set_si(base_num, Z_LVAL_P(base_arg)); - mpz_set_si(exp_num, exp); - mpz_set_ui(mod, UINT_MAX); - mpz_powm(gmpnum_result, base_num, exp_num, mod); - mpz_clear(mod); - mpz_clear(exp_num); - mpz_clear(base_num); - } else { - mpz_ui_pow_ui(gmpnum_result, Z_LVAL_P(base_arg), exp); + if ((log10(Z_LVAL_P(base_arg)) * exp) > (double)ULONG_MAX) { + zend_value_error("base and exponent overflow"); + RETURN_THROWS(); } + mpz_ui_pow_ui(gmpnum_result, Z_LVAL_P(base_arg), exp); } else { mpz_ptr gmpnum_base; + unsigned long gmpnum; FETCH_GMP_ZVAL(gmpnum_base, base_arg, temp_base, 1); INIT_GMP_RETVAL(gmpnum_result); - if (exp >= INT_MAX) { - mpz_t exp_num, mod; - mpz_init(exp_num); - mpz_init(mod); - mpz_set_si(exp_num, exp); - mpz_set_ui(mod, UINT_MAX); - mpz_powm(gmpnum_result, gmpnum_base, exp_num, mod); - mpz_clear(mod); - mpz_clear(exp_num); - } else { - mpz_pow_ui(gmpnum_result, gmpnum_base, exp); + gmpnum = mpz_get_ui(gmpnum_base); + if ((log10(gmpnum) * exp) > (double)ULONG_MAX) { + FREE_GMP_TEMP(temp_base); + zend_value_error("base and exponent overflow"); + RETURN_THROWS(); } + mpz_pow_ui(gmpnum_result, gmpnum_base, exp); FREE_GMP_TEMP(temp_base); } } From e2b07d78e03150fcf75f2e5c7bceda7456336d1e Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 12 Oct 2024 07:31:15 +0100 Subject: [PATCH 2/6] add test --- ext/gmp/tests/gmp_pow_fpe.phpt | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/ext/gmp/tests/gmp_pow_fpe.phpt b/ext/gmp/tests/gmp_pow_fpe.phpt index d564853799c8d..591107467f386 100644 --- a/ext/gmp/tests/gmp_pow_fpe.phpt +++ b/ext/gmp/tests/gmp_pow_fpe.phpt @@ -6,15 +6,17 @@ gmp ---EXPECTF-- -object(GMP)#2 (1) { - ["num"]=> - string(%d) "%s" +try { + gmp_pow($g, PHP_INT_MAX); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; } -object(GMP)#2 (1) { - ["num"]=> - string(%d) "%s" +try { + gmp_pow(256, PHP_INT_MAX); +} catch (\ValueError $e) { + echo $e->getMessage(); } +?> +--EXPECT-- +base and exponent overflow +base and exponent overflow From 3534f0a406f017c650ced0fe423300fceb5f1432 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 21 Oct 2024 15:39:52 +0100 Subject: [PATCH 3/6] add tests --- ext/gmp/tests/gmp_pow_fpe.phpt | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ext/gmp/tests/gmp_pow_fpe.phpt b/ext/gmp/tests/gmp_pow_fpe.phpt index 591107467f386..1384fc368a3a0 100644 --- a/ext/gmp/tests/gmp_pow_fpe.phpt +++ b/ext/gmp/tests/gmp_pow_fpe.phpt @@ -14,9 +14,20 @@ try { try { gmp_pow(256, PHP_INT_MAX); } catch (\ValueError $e) { - echo $e->getMessage(); + echo $e->getMessage() . PHP_EOL; } + +var_dump(gmp_pow(gmp_add(gmp_mul(gmp_init(PHP_INT_MAX), gmp_init(PHP_INT_MAX)), 3), 256)); +var_dump(gmp_pow(gmp_init(PHP_INT_MAX), 256)); ?> ---EXPECT-- +--EXPECTF-- base and exponent overflow base and exponent overflow +object(GMP)#%d (1) { + ["num"]=> + string(%d) "%s" +} +object(GMP)#%d (1) { + ["num"]=> + string(%d) "%s" +} From 72b5653d0f254320f1a4e1ff815a96eb7bb92b97 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 24 Oct 2024 20:52:27 +0100 Subject: [PATCH 4/6] change comparison limit, now new tests overflow on 64 bits. --- ext/gmp/gmp.c | 8 +++++--- ext/gmp/tests/gmp_pow_fpe.phpt | 22 ++++++++++++---------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index b403f48e82cb6..fc800d5004652 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -1282,20 +1282,22 @@ ZEND_FUNCTION(gmp_pow) RETURN_THROWS(); } + double powmax = log((double)ZEND_LONG_MAX); + if (Z_TYPE_P(base_arg) == IS_LONG && Z_LVAL_P(base_arg) >= 0) { INIT_GMP_RETVAL(gmpnum_result); - if ((log10(Z_LVAL_P(base_arg)) * exp) > (double)ULONG_MAX) { + if ((log(Z_LVAL_P(base_arg)) * exp) > powmax) { zend_value_error("base and exponent overflow"); RETURN_THROWS(); } mpz_ui_pow_ui(gmpnum_result, Z_LVAL_P(base_arg), exp); } else { mpz_ptr gmpnum_base; - unsigned long gmpnum; + unsigned long int gmpnum; FETCH_GMP_ZVAL(gmpnum_base, base_arg, temp_base, 1); INIT_GMP_RETVAL(gmpnum_result); gmpnum = mpz_get_ui(gmpnum_base); - if ((log10(gmpnum) * exp) > (double)ULONG_MAX) { + if ((log(gmpnum) * exp) > powmax) { FREE_GMP_TEMP(temp_base); zend_value_error("base and exponent overflow"); RETURN_THROWS(); diff --git a/ext/gmp/tests/gmp_pow_fpe.phpt b/ext/gmp/tests/gmp_pow_fpe.phpt index 1384fc368a3a0..248922e80514d 100644 --- a/ext/gmp/tests/gmp_pow_fpe.phpt +++ b/ext/gmp/tests/gmp_pow_fpe.phpt @@ -17,17 +17,19 @@ try { echo $e->getMessage() . PHP_EOL; } -var_dump(gmp_pow(gmp_add(gmp_mul(gmp_init(PHP_INT_MAX), gmp_init(PHP_INT_MAX)), 3), 256)); -var_dump(gmp_pow(gmp_init(PHP_INT_MAX), 256)); +try { + gmp_pow(gmp_add(gmp_mul(gmp_init(PHP_INT_MAX), gmp_init(PHP_INT_MAX)), 3), 256); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + gmp_pow(gmp_init(PHP_INT_MAX), 256); +} catch (\ValueError $e) { + echo $e->getMessage(); +} ?> --EXPECTF-- base and exponent overflow base and exponent overflow -object(GMP)#%d (1) { - ["num"]=> - string(%d) "%s" -} -object(GMP)#%d (1) { - ["num"]=> - string(%d) "%s" -} +base and exponent overflow +base and exponent overflow From 7f432a13b25303af3316fab60a48e8d971d82c7d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 25 Oct 2024 11:12:02 +0100 Subject: [PATCH 5/6] zend_ulong --- ext/gmp/gmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index fc800d5004652..42a4a446867c2 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -1293,7 +1293,7 @@ ZEND_FUNCTION(gmp_pow) mpz_ui_pow_ui(gmpnum_result, Z_LVAL_P(base_arg), exp); } else { mpz_ptr gmpnum_base; - unsigned long int gmpnum; + zend_ulong gmpnum; FETCH_GMP_ZVAL(gmpnum_base, base_arg, temp_base, 1); INIT_GMP_RETVAL(gmpnum_result); gmpnum = mpz_get_ui(gmpnum_base); From 752627d65e72df83d2498bf92961239774d90ae3 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 25 Oct 2024 13:22:53 +0100 Subject: [PATCH 6/6] fix tests on 32 bits attempt. note: using consistent "style" for test on purpose. --- ext/gmp/tests/gmp_pow.phpt | 2 + ext/gmp/tests/gmp_pow_32bits.phpt | 77 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 ext/gmp/tests/gmp_pow_32bits.phpt diff --git a/ext/gmp/tests/gmp_pow.phpt b/ext/gmp/tests/gmp_pow.phpt index f42e44e31abed..1d77bd5e96c80 100644 --- a/ext/gmp/tests/gmp_pow.phpt +++ b/ext/gmp/tests/gmp_pow.phpt @@ -2,6 +2,8 @@ gmp_pow() basic tests --EXTENSIONS-- gmp +--SKIPIF-- + --FILE-- +--FILE-- +getMessage() . "\n"; +} +var_dump(gmp_strval(gmp_pow("-2",10))); +try { + gmp_pow(20,10); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +try { + gmp_pow(50,10); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +try { + gmp_pow(50,-5); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +try { + $n = gmp_init("20"); + gmp_pow($n,10); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +try { + $n = gmp_init("-20"); + gmp_pow($n,10); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +try { + var_dump(gmp_pow(2,array())); +} catch (TypeError $e) { + echo $e->getMessage(), "\n"; +} + +try { + var_dump(gmp_pow(array(),10)); +} catch (\TypeError $e) { + echo $e->getMessage() . \PHP_EOL; +} + +echo "Done\n"; +?> +--EXPECT-- +string(4) "1024" +string(4) "1024" +string(5) "-2048" +string(4) "1024" +string(1) "1" +gmp_pow(): Argument #2 ($exponent) must be greater than or equal to 0 +string(4) "1024" +base and exponent overflow +base and exponent overflow +gmp_pow(): Argument #2 ($exponent) must be greater than or equal to 0 +base and exponent overflow +base and exponent overflow +gmp_pow(): Argument #2 ($exponent) must be of type int, array given +gmp_pow(): Argument #1 ($num) must be of type GMP|string|int, array given +Done