diff --git a/src/libxrpl/json/json_reader.cpp b/src/libxrpl/json/json_reader.cpp index f37cca506b..f9134e6629 100644 --- a/src/libxrpl/json/json_reader.cpp +++ b/src/libxrpl/json/json_reader.cpp @@ -5,12 +5,13 @@ #include #include +#include #include #include -#include #include #include #include +#include namespace json { // Implementation of class Reader @@ -605,36 +606,17 @@ bool Reader::decodeDouble(Token& token) { double value = 0; - int const bufferSize = 32; - int count = 0; - int const length = int(token.end - token.start); - // Sanity check to avoid buffer overflow exploits. - if (length < 0) - { - return addError("Unable to parse token length", token); - } - // Avoid using a string constant for the format control string given to - // sscanf, as this can cause hard to debug crashes on OS X. See here for - // more info: - // - // http://developer.apple.com/library/mac/#DOCUMENTATION/DeveloperTools/gcc-4.0.1/gcc/Incompatibilities.html - char format[] = "%lf"; - if (length <= bufferSize) - { - Char buffer[bufferSize + 1]; - memcpy(buffer, token.start, length); - buffer[length] = 0; - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - count = sscanf(buffer, format, &value); - } - else - { - std::string const buffer(token.start, token.end); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - count = sscanf(buffer.c_str(), format, &value); - } - if (count != 1) + auto const [ptr, ec] = std::from_chars(token.start, token.end, value); + + // Reject anything from_chars could not turn into a finite double: + // - ec != std::errc{}: no valid conversion, or an out-of-range magnitude + // (e.g. 1e400). + // - ptr != token.end: readNumber() is permissive about which characters + // it collects into a token (it will, for example, keep a '+' mid-token), + // but from_chars() will stop at the first character it cannot parse. + if (ec != std::errc{} || ptr != token.end) return addError("'" + std::string(token.start, token.end) + "' is not a number.", token); + currentValue() = value; return true; } diff --git a/src/libxrpl/json/json_writer.cpp b/src/libxrpl/json/json_writer.cpp index 853dd9a5ac..fdfa5d3cf0 100644 --- a/src/libxrpl/json/json_writer.cpp +++ b/src/libxrpl/json/json_writer.cpp @@ -4,13 +4,14 @@ #include #include -#include +#include #include #include #include #include #include #include +#include #include namespace json { @@ -76,20 +77,18 @@ valueToString(UInt value) std::string valueToString(double value) { - // Allocate a buffer that is more than large enough to store the 16 digits - // of precision requested below. + // Format with 16 significant digits. + // We need not request the alternative representation that always has a + // decimal point because JSON doesn't distinguish the concepts of reals and integers. + // A double never needs more than 32 characters in this form, + // so to_chars cannot actually run out of room here. char buffer[32]; - // Print into the buffer. We need not request the alternative representation - // that always has a decimal point because JSON doesn't distinguish the - // concepts of reals and integers. -#if defined(_MSC_VER) && defined(__STDC_SECURE_LIB__) // Use secure version with visual studio 2005 - // to avoid warning. - sprintf_s(buffer, sizeof(buffer), "%.16g", value); -#else - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) - snprintf(buffer, sizeof(buffer), "%.16g", value); -#endif - return buffer; + auto const [ptr, ec] = + std::to_chars(buffer, buffer + sizeof(buffer), value, std::chars_format::general, 16); + XRPL_ASSERT(ec == std::errc{}, "json::valueToString(double) : conversion fits buffer"); + if (ec != std::errc{}) + return {}; + return std::string(buffer, ptr); } std::string diff --git a/src/tests/libxrpl/json/Value.cpp b/src/tests/libxrpl/json/Value.cpp index a1cd8160d7..a58a5df9fd 100644 --- a/src/tests/libxrpl/json/Value.cpp +++ b/src/tests/libxrpl/json/Value.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -592,6 +593,57 @@ TEST(json_value, bad_json) EXPECT_TRUE(r.parse(s, j)); } +namespace { + +std::optional +parseValue(std::string const& doc) +{ + json::Value j; + json::Reader r; + if (!r.parse("{\"v\":" + doc + "}", j)) + return std::nullopt; + return j["v"]; +} + +} // namespace + +TEST(json_value, parse_double_valid) +{ + // 1e300 is large but still representable, so it parses (unlike the out-of-range cases below). + for (auto const& [text, expected] : + {std::pair{"2.5", 2.5}, + std::pair{"-3.25e2", -325.0}, + std::pair{"0.0", 0.0}, + std::pair{"1E3", 1000.0}, + std::pair{"-0.5e-1", -0.05}, + std::pair{"1e300", 1e300}}) + { + auto const v = parseValue(text); + ASSERT_TRUE(v.has_value()) << text; + // NOLINTBEGIN(bugprone-unchecked-optional-access) + EXPECT_TRUE(v->isDouble()) << text; + EXPECT_EQ(v->asDouble(), expected) << text; + // NOLINTEND(bugprone-unchecked-optional-access) + } +} + +TEST(json_value, parse_double_out_of_range) +{ + // Magnitudes with no finite double representation are rejected. + for (char const* oor : {"1e400", "-1e400", "0.001e500", "1e-400", "-1e-400", "123e-500"}) + EXPECT_FALSE(parseValue(oor).has_value()) << oor; +} + +TEST(json_value, parse_double_malformed) +{ + // readNumber() collects any run of digits and '.eE+-' into a single Double + // token, so these malformed tokens reach decodeDouble. Each has a valid + // leading prefix that from_chars would accept on its own; requiring the + // entire token be consumed rejects them instead of silently truncating. + for (char const* bad : {"1+2", "1-2", "1.2.3", "1e5e6", "1..2", "++5", "1e", "1e+", ".", "-"}) + EXPECT_FALSE(parseValue(bad).has_value()) << bad; +} + TEST(json_value, edge_cases) { std::uint32_t const maxUInt = std::numeric_limits::max();