-
Notifications
You must be signed in to change notification settings - Fork 7.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gmp_export() can cause overflow #16411
Comments
ext/gmp/gmp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c
index 04bfafe869..bbea70225e 100644
--- a/ext/gmp/gmp.c
+++ b/ext/gmp/gmp.c
@@ -920,9 +920,9 @@ ZEND_FUNCTION(gmp_init)
static bool gmp_import_export_validate(zend_long size, zend_long options, int *order, int *endian)
{
- if (size < 1) {
+ if (size < 1 || size > ZEND_LONG_MAX / 4) {
/* size argument is in second position */
- zend_argument_value_error(2, "must be greater than or equal to 1");
+ zend_argument_value_error(2, "must be greater than or equal to 1 and less than or equal to " ZEND_LONG_FMT, ZEND_LONG_MAX / 4);
return false;
}
@@ -1012,7 +1012,7 @@ ZEND_FUNCTION(gmp_export)
if (mpz_sgn(gmpnumber) == 0) {
RETVAL_EMPTY_STRING();
} else {
- size_t bits_per_word = size * 8;
+ size_t bits_per_word = (size_t) size * 8;
size_t count = (mpz_sizeinbase(gmpnumber, 2) + bits_per_word - 1) / bits_per_word;
zend_string *out_string = zend_string_safe_alloc(count, size, 0, 0); would prevent this overflow, but still an unexpected unsigned overflow would occur on the next line, so |
@cmb69 Feel free to make a PR, although I don't understand yet how you arrived at |
Would need to divide by 8, but since However, as I said, that value may still be to large for Line 1016 in 79c71c9
Would probably need another check here. |
Right that makes sense indeed |
We need not only to avoid the signed overflow while calculating `bits_per_word` (reported issue), but also the unsigned overflow when calculating `count`. While the former has a fixed threshold, the latter does not, since it also depends on the size in base 2. Thus we use a somewhat unconventional error message.
* PHP-8.2: Fix GH-16411: gmp_export() can cause overflow
* PHP-8.3: Fix GH-16411: gmp_export() can cause overflow
* PHP-8.4: Fix GH-16411: gmp_export() can cause overflow
Description
The following code:
Resulted in this output:
PHP Version
nightly
Operating System
ubuntu 22.04
The text was updated successfully, but these errors were encountered: