From a116c9e790db8961b76def77b68b0e01d8b808e7 Mon Sep 17 00:00:00 2001 From: Martchus Date: Mon, 24 May 2021 21:27:18 +0200 Subject: [PATCH] Avoid possibility of overflow in DateTime parsing functions * This is strictly undefined behavior so let's avoid it * As a side-effect it is now possible to omit the separators in DateTime::fromIsoString() --- chrono/datetime.cpp | 40 +++++++++++++++++++++++++++------------- tests/chronotests.cpp | 6 ++++++ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/chrono/datetime.cpp b/chrono/datetime.cpp index 0ed9984..b1f0b23 100644 --- a/chrono/datetime.cpp +++ b/chrono/datetime.cpp @@ -91,8 +91,7 @@ DateTime DateTime::fromString(const char *str) miliSeconds += (c - '0') * miliSecondsFact; miliSecondsFact /= 10; } else { - *valueIndex *= 10; - *valueIndex += c - '0'; + Detail::raiseAndAdd(*valueIndex, 10, c); } } else if ((c == '-' || c == ':' || c == '/') || (c == '.' && (valueIndex == secondsIndex)) || (c == ' ' && (valueIndex == dayIndex))) { if (++valueIndex == valuesEnd) { @@ -101,7 +100,7 @@ DateTime DateTime::fromString(const char *str) } else if (c == '\0') { break; } else { - throw ConversionException(argsToString("unexpected ", c)); + throw ConversionException(argsToString("Unexpected character \"", c, '\"')); } } return DateTime::fromDateAndTime(values[0], values[1], *dayIndex, values[3], values[4], *secondsIndex, miliSeconds); @@ -110,10 +109,11 @@ DateTime DateTime::fromString(const char *str) /*! * \brief Parses the specified ISO date time denotation provided as C-style string. * \returns Returns a pair where the first value is the parsed date time and the second value - * a time span which can be subtracted from the first value to get the UTC time. - * \remarks Not all variants allowed by ISO 8601 are supported right now, eg. delimiters can not - * be omitted. - * The common form (something like "2016-08-29T21:32:31.588539814+02:00") is supported of course. + * the time zone designator (a time span which can be subtracted from the first value to get the UTC time). + * \remarks + * - Parsing durations and time intervals is *not* supported. + * - Truncated representations are *not* supported. + * - Standardised extensions (ISO 8601-2:2019) are *not* supported. * \sa https://en.wikipedia.org/wiki/ISO_8601 */ std::pair DateTime::fromIsoString(const char *str) @@ -126,7 +126,9 @@ std::pair DateTime::fromIsoString(const char *str) int *const secondsIndex = values + 5; int *const miliSecondsIndex = values + 6; int *const deltaHourIndex = values + 7; + int *const valuesEnd = values + 9; int *valueIndex = values; + unsigned int remainingDigits = 4; bool deltaNegative = false; double miliSecondsFact = 100.0, miliSeconds = 0.0; for (const char *strIndex = str;; ++strIndex) { @@ -136,13 +138,21 @@ std::pair DateTime::fromIsoString(const char *str) miliSeconds += (c - '0') * miliSecondsFact; miliSecondsFact /= 10; } else { + if (!remainingDigits) { + if (++valueIndex == miliSecondsIndex || valueIndex >= valuesEnd) { + throw ConversionException("Max. number of digits exceeded"); + } + remainingDigits = 2; + } *valueIndex *= 10; *valueIndex += c - '0'; + remainingDigits -= 1; } } else if (c == 'T') { if (++valueIndex != hourIndex) { throw ConversionException("\"T\" expected before hour"); } + remainingDigits = 2; } else if (c == '-') { if (valueIndex < dayIndex) { ++valueIndex; @@ -150,34 +160,38 @@ std::pair DateTime::fromIsoString(const char *str) valueIndex = deltaHourIndex; deltaNegative = true; } else { - throw ConversionException("unexpected \"-\" after day"); + throw ConversionException("Unexpected \"-\" after day"); } + remainingDigits = 2; } else if (c == '.') { if (valueIndex != secondsIndex) { - throw ConversionException("unexpected \".\""); + throw ConversionException("Unexpected \".\""); } else { ++valueIndex; } } else if (c == ':') { if (valueIndex < hourIndex) { - throw ConversionException("unexpected \":\" before hour"); + throw ConversionException("Unexpected \":\" before hour"); } else if (valueIndex == secondsIndex) { - throw ConversionException("unexpected \":\" after second"); + throw ConversionException("Unexpected \":\" after second"); } else { ++valueIndex; } + remainingDigits = 2; } else if ((c == '+') && (++valueIndex >= secondsIndex)) { valueIndex = deltaHourIndex; deltaNegative = false; + remainingDigits = 2; } else if ((c == 'Z') && (++valueIndex >= secondsIndex)) { valueIndex = deltaHourIndex + 2; + remainingDigits = 2; } else if (c == '\0') { break; } else { - throw ConversionException(argsToString("unexpected \"", c, '\"')); + throw ConversionException(argsToString("Unexpected \"", c, '\"')); } } - auto delta(TimeSpan::fromMinutes(*deltaHourIndex * 60 + values[8])); + auto delta = TimeSpan::fromMinutes(*deltaHourIndex * 60 + values[8]); if (deltaNegative) { delta = TimeSpan(-delta.totalTicks()); } diff --git a/tests/chronotests.cpp b/tests/chronotests.cpp index 223d00d..64e55cf 100644 --- a/tests/chronotests.cpp +++ b/tests/chronotests.cpp @@ -182,6 +182,12 @@ void ChronoTests::testDateTime() const auto test7 = DateTime::fromIsoString("2021-05-20T23:02:45-04:00"); CPPUNIT_ASSERT_EQUAL_MESSAGE("no seconds fraction (negative timezone offset, 1)", DateTime::fromDateAndTime(2021, 5, 20, 23, 2, 45), test7.first); CPPUNIT_ASSERT_EQUAL_MESSAGE("no seconds fraction (negative timezone offset, 2)", TimeSpan::fromHours(-4.0), test7.second); + // implied separators / too many digits + CPPUNIT_ASSERT_EQUAL_MESSAGE("no separators", test5.first - test5.second, DateTime::fromIsoStringGmt("20170823T194015.985077682-0230")); + CPPUNIT_ASSERT_EQUAL_MESSAGE( + "not even T separator", DateTime::fromDateAndTime(2017, 8, 23, 19, 40, 15), DateTime::fromIsoStringGmt("20170823194015")); + CPPUNIT_ASSERT_THROW_MESSAGE("too many digits after seconds", DateTime::fromIsoString("2017082319401516"), ConversionException); + CPPUNIT_ASSERT_THROW_MESSAGE("too many digits after timezone offset", DateTime::fromIsoString("20170823194015.16+02300"), ConversionException); // test invalid characters CPPUNIT_ASSERT_THROW_MESSAGE("digits after Z", DateTime::fromIsoString("2017-O8-23T19:40:15.985077682Z02:00"), ConversionException); CPPUNIT_ASSERT_THROW_MESSAGE("invalid letter", DateTime::fromIsoString("2017-O8-23T19:40:15.985077682:+02:00"), ConversionException);