From a8a35345c8e1fbbf3f0b88766931bbf912abeb9b Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 11 Sep 2022 15:11:15 +0100 Subject: [PATCH] Credit to OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51241 Fix bounds checking bug. --- src/types.cpp | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index daf6e7a4..9b9e5730 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -611,24 +611,22 @@ Rational parseRational(const std::string& s, bool& ok) { } Rational floatToRationalCast(float f) { - // Convert f to double because it simplifies the "in_range" check - // below. (INT_MAX can be represented accurately as a double, but - // gets rounded when it's converted to float.) + // Convert f to double because it simplifies the range checks + // below. (All int values can be losslessly converted to double, but + // sometimes get rounded when converted to float.) const double d = f; - // Don't allow INT_MIN (0x80000000) because it can cause a UBSAN failure in std::gcd(). - const bool in_range = std::numeric_limits::min() < d && d <= std::numeric_limits::max(); - if (!in_range) { - return {d > 0 ? 1 : -1, 0}; - } // Beware: primitive conversion algorithm - int32_t den = 1000000; - const auto d_as_int32_t = static_cast(d); - if (Safe::abs(d_as_int32_t) > 21474836) { - den = 1; - } else if (Safe::abs(d_as_int32_t) > 214748) { - den = 100; - } else if (Safe::abs(d_as_int32_t) > 2147) { + int32_t den; + if (std::fabs(d) <= std::numeric_limits::max() / 1000000) { + den = 1000000; + } else if (std::fabs(d) <= std::numeric_limits::max() / 10000) { den = 10000; + } else if (std::fabs(d) <= std::numeric_limits::max() / 100) { + den = 100; + } else if (std::fabs(d) <= std::numeric_limits::max()) { + den = 1; + } else { + return {d > 0 ? 1 : -1, 0}; } const auto nom = static_cast(std::round(d * den)); const int32_t g = std::gcd(nom, den);