From 13b82eef8426f16960ff6d895d1d5949c55d9ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Tue, 10 Jan 2023 10:16:33 +0100 Subject: [PATCH] random: Randomizer::getFloat(): Fix check for empty open intervals (#10185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * random: Randomizer::getFloat(): Fix check for empty open intervals The check for invalid parameters for the IntervalBoundary::OpenOpen variant was not correct: If two consecutive doubles are passed as parameters, the resulting interval is empty, resulting in an uint64 underflow in the γ-section implementation. Instead of checking whether `$min < $max`, we must check that there is at least one more double between `$min` and `$max`, i.e. it must hold that: nextafter($min, $max) != $max Instead of duplicating the comparatively complicated and expensive `nextafter` logic for a rare error case we instead return `NAN` from the γ-section implementation when the parameters result in an empty interval and thus underflow. This allows us to reliably detect this specific error case *after* the fact, but without modifying the engine state. It also provides reliable error reporting for other internal functions that might use the γ-section implementation. * random: γ-section: Also check that that min is smaller than max This extends the empty-interval check in the γ-section implementation with a check that min is actually the smaller of the two parameters. * random: Use PHP_FLOAT_EPSILON in getFloat_error.phpt Co-authored-by: Christoph M. Becker --- ext/random/gammasection.c | 20 +++++++++++++++++++ ext/random/randomizer.c | 9 ++++++++- .../03_randomizer/methods/getFloat_error.phpt | 13 +++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/ext/random/gammasection.c b/ext/random/gammasection.c index fb0c2cd3bf9..aa4531fba22 100644 --- a/ext/random/gammasection.c +++ b/ext/random/gammasection.c @@ -66,6 +66,11 @@ PHPAPI double php_random_gammasection_closed_open(const php_random_algo *algo, p { double g = gamma_max(min, max); uint64_t hi = ceilint(min, max, g); + + if (UNEXPECTED(max <= min || hi < 1)) { + return NAN; + } + uint64_t k = 1 + php_random_range64(algo, status, hi - 1); /* [1, hi] */ if (fabs(min) <= fabs(max)) { @@ -79,6 +84,11 @@ PHPAPI double php_random_gammasection_closed_closed(const php_random_algo *algo, { double g = gamma_max(min, max); uint64_t hi = ceilint(min, max, g); + + if (UNEXPECTED(max < min)) { + return NAN; + } + uint64_t k = php_random_range64(algo, status, hi); /* [0, hi] */ if (fabs(min) <= fabs(max)) { @@ -92,6 +102,11 @@ PHPAPI double php_random_gammasection_open_closed(const php_random_algo *algo, p { double g = gamma_max(min, max); uint64_t hi = ceilint(min, max, g); + + if (UNEXPECTED(max <= min || hi < 1)) { + return NAN; + } + uint64_t k = php_random_range64(algo, status, hi - 1); /* [0, hi - 1] */ if (fabs(min) <= fabs(max)) { @@ -105,6 +120,11 @@ PHPAPI double php_random_gammasection_open_open(const php_random_algo *algo, php { double g = gamma_max(min, max); uint64_t hi = ceilint(min, max, g); + + if (UNEXPECTED(max <= min || hi < 2)) { + return NAN; + } + uint64_t k = 1 + php_random_range64(algo, status, hi - 2); /* [1, hi - 1] */ if (fabs(min) <= fabs(max)) { diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 0a801e35c74..75e88c8b010 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -190,7 +190,14 @@ PHP_METHOD(Random_Randomizer, getFloat) RETURN_THROWS(); } - RETURN_DOUBLE(php_random_gammasection_open_open(randomizer->algo, randomizer->status, min, max)); + RETVAL_DOUBLE(php_random_gammasection_open_open(randomizer->algo, randomizer->status, min, max)); + + if (UNEXPECTED(isnan(Z_DVAL_P(return_value)))) { + zend_value_error("The given interval is empty, there are no floats between argument #1 ($min) and argument #2 ($max)."); + RETURN_THROWS(); + } + + return; default: ZEND_UNREACHABLE(); } diff --git a/ext/random/tests/03_randomizer/methods/getFloat_error.phpt b/ext/random/tests/03_randomizer/methods/getFloat_error.phpt index 1e200f2507a..286435e1752 100644 --- a/ext/random/tests/03_randomizer/methods/getFloat_error.phpt +++ b/ext/random/tests/03_randomizer/methods/getFloat_error.phpt @@ -73,10 +73,17 @@ foreach ([ } catch (ValueError $e) { echo $e->getMessage(), PHP_EOL; } + + try { + // There is no float between the two parameters, thus making the OpenOpen interval empty. + var_dump(randomizer()->getFloat(1.0, 1 + PHP_FLOAT_EPSILON, $boundary)); + } catch (ValueError $e) { + echo $e->getMessage(), PHP_EOL; + } } ?> ---EXPECT-- +--EXPECTF-- ClosedClosed Random\Randomizer::getFloat(): Argument #1 ($min) must be finite Random\Randomizer::getFloat(): Argument #1 ($min) must be finite @@ -87,6 +94,7 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than or equal to argument #1 ($min) float(0) float(1.0E+17) +float(%f) ClosedOpen Random\Randomizer::getFloat(): Argument #1 ($min) must be finite Random\Randomizer::getFloat(): Argument #1 ($min) must be finite @@ -97,6 +105,7 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) +float(1) OpenClosed Random\Randomizer::getFloat(): Argument #1 ($min) must be finite Random\Randomizer::getFloat(): Argument #1 ($min) must be finite @@ -107,6 +116,7 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) +float(1.0000000000000002) OpenOpen Random\Randomizer::getFloat(): Argument #1 ($min) must be finite Random\Randomizer::getFloat(): Argument #1 ($min) must be finite @@ -117,3 +127,4 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min) +The given interval is empty, there are no floats between argument #1 ($min) and argument #2 ($max).