diff --git a/NEWS b/NEWS index 219e54c1f9d5..7e6a6dd443ce 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,8 @@ PHP NEWS - GMP: . Fixed bug GH-22549 (Assertion failure / UB on a compound GMP power or shift assignment with a negative exponent). (iliaal) + . Fixed GMP power and shift operators to reject GMP right operands outside + the unsigned long range instead of silently truncating them. (Weilin Du) - Intl: . Fixed NumberFormatter::parse() and NumberFormatter::parseCurrency() to diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index 59209c4e1be3..ce083168e683 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -327,8 +327,18 @@ static zend_result binop_operator_helper(gmp_binary_op_t gmp_op, zval *return_va typedef void (*gmp_binary_ui_op_t)(mpz_ptr, mpz_srcptr, gmp_ulong); +static zend_result gmp_shift_operator_range_error(uint8_t opcode) { + zend_throw_error( + zend_ce_value_error, "%s must be between 0 and %lu", + opcode == ZEND_POW ? "Exponent" : "Shift", ULONG_MAX + ); + return FAILURE; +} + static zend_result shift_operator_helper(gmp_binary_ui_op_t op, zval *return_value, zval *op1, zval *op2, uint8_t opcode) { zend_long shift = 0; + gmp_ulong shift_ui = 0; + bool have_shift_ui = false; if (UNEXPECTED(Z_TYPE_P(op2) != IS_LONG)) { if (UNEXPECTED(!IS_GMP(op2))) { @@ -349,20 +359,25 @@ static zend_result shift_operator_helper(gmp_binary_ui_op_t op, zval *return_val goto typeof_op_failure; } } else { - // TODO We shouldn't cast the GMP object to int here - shift = zval_get_long(op2); + mpz_ptr gmpnum_shift = GET_GMP_FROM_ZVAL(op2); + if (!mpz_fits_ulong_p(gmpnum_shift)) { + return gmp_shift_operator_range_error(opcode); + } + shift_ui = (gmp_ulong) mpz_get_ui(gmpnum_shift); + have_shift_ui = true; } } else { shift = Z_LVAL_P(op2); } - if (shift < 0 || shift > ULONG_MAX) { - zend_throw_error( - zend_ce_value_error, "%s must be between 0 and %lu", - opcode == ZEND_POW ? "Exponent" : "Shift", ULONG_MAX - ); - return FAILURE; - } else { + if (!have_shift_ui) { + if (shift < 0 || shift > ULONG_MAX) { + return gmp_shift_operator_range_error(opcode); + } + shift_ui = (gmp_ulong) shift; + } + + { mpz_ptr gmpnum_op, gmpnum_result; if (!gmp_zend_parse_arg_into_mpz_ex(op1, &gmpnum_op, 1, true)) { @@ -370,7 +385,7 @@ static zend_result shift_operator_helper(gmp_binary_ui_op_t op, zval *return_val } INIT_GMP_RETVAL(gmpnum_result); - op(gmpnum_result, gmpnum_op, (gmp_ulong) shift); + op(gmpnum_result, gmpnum_op, shift_ui); return SUCCESS; } diff --git a/ext/gmp/tests/gmp_operator_rhs_gmp_overflow.phpt b/ext/gmp/tests/gmp_operator_rhs_gmp_overflow.phpt new file mode 100644 index 000000000000..a221e497b7cb --- /dev/null +++ b/ext/gmp/tests/gmp_operator_rhs_gmp_overflow.phpt @@ -0,0 +1,26 @@ +--TEST-- +GMP operator right operand rejects values outside the unsigned long range +--EXTENSIONS-- +gmp +--FILE-- +getMessage(), PHP_EOL; +} + +try { + var_dump(gmp_init(2) << $too_large); +} catch (ValueError $e) { + echo $e->getMessage(), PHP_EOL; +} + +echo "Done\n"; +?> +--EXPECTF-- +Exponent must be between 0 and %d +Shift must be between 0 and %d +Done