gmp_export() can cause overflow

[YuanchengJiang]

Description

The following code:

<?php
gmp_export(-9223372036854775808, 9223372036854775807, -9223372036854775808);

Resulted in this output:

/php-src/ext/gmp/gmp.c:1015:31: runtime error: signed integer overflow: 9223372036854775807 * 8 cannot be represented in type 'long'

PHP Version

nightly

Operating System

ubuntu 22.04

[cmb69]

 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 size needs to be smaller than ZEND_LONG_MAX/4.

[nielsdos]

@cmb69 Feel free to make a PR, although I don't understand yet how you arrived at ZEND_LONG_MAX/4.

[cmb69]

Would need to divide by 8, but since size is signed, we can double to fit into unsigned; thus divide by 4.

However, as I said, that value may still be to large for

php-src/ext/gmp/gmp.c

Line 1016 in 79c71c9

size_t count = (mpz_sizeinbase(gmpnumber, 2) + bits_per_word - 1) / bits_per_word;

Would probably need another check here.

[nielsdos]

Right that makes sense indeed