From ddadc9427b243e5224f3b398d96e0b99d9301642 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Wed, 28 Aug 2019 17:04:27 -0700 Subject: [PATCH] ICU-13745 fix undefined behavior: GregorianCalendar::setGregorianChange() - Julian days outside of INT32_MIN..INT32_MAX are normalized - Add a test case --- icu4c/source/i18n/gregocal.cpp | 28 +++++++++---------- icu4c/source/test/intltest/calregts.cpp | 36 +++++++++++++++++++++++++ icu4c/source/test/intltest/calregts.h | 2 ++ 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/icu4c/source/i18n/gregocal.cpp b/icu4c/source/i18n/gregocal.cpp index ee15b5291a9..6b15171c12b 100644 --- a/icu4c/source/i18n/gregocal.cpp +++ b/icu4c/source/i18n/gregocal.cpp @@ -324,26 +324,26 @@ GregorianCalendar::setGregorianChange(UDate date, UErrorCode& status) if (U_FAILURE(status)) return; - fGregorianCutover = date; - // Precompute two internal variables which we use to do the actual // cutover computations. These are the normalized cutover, which is the // midnight at or before the cutover, and the cutover year. The // normalized cutover is in pure date milliseconds; it contains no time // of day or timezone component, and it used to compare against other // pure date values. - int32_t cutoverDay = (int32_t)ClockMath::floorDivide(fGregorianCutover, (double)kOneDay); - fNormalizedGregorianCutover = cutoverDay * kOneDay; + double cutoverDay = ClockMath::floorDivide(date, (double)kOneDay); - // Handle the rare case of numeric overflow. If the user specifies a - // change of UDate(Long.MIN_VALUE), in order to get a pure Gregorian - // calendar, then the epoch day is -106751991168, which when multiplied - // by ONE_DAY gives 9223372036794351616 -- the negative value is too - // large for 64 bits, and overflows into a positive value. We correct - // this by using the next day, which for all intents is semantically - // equivalent. - if (cutoverDay < 0 && fNormalizedGregorianCutover > 0) { - fNormalizedGregorianCutover = (cutoverDay + 1) * kOneDay; + // Handle the rare case of numeric overflow where the user specifies a time + // outside of INT32_MIN .. INT32_MAX number of days. + + if (cutoverDay <= INT32_MIN) { + cutoverDay = INT32_MIN; + fGregorianCutover = fNormalizedGregorianCutover = cutoverDay * kOneDay; + } else if (cutoverDay >= INT32_MAX) { + cutoverDay = INT32_MAX; + fGregorianCutover = fNormalizedGregorianCutover = cutoverDay * kOneDay; + } else { + fNormalizedGregorianCutover = cutoverDay * kOneDay; + fGregorianCutover = date; } // Normalize the year so BC values are represented as 0 and negative @@ -360,7 +360,7 @@ GregorianCalendar::setGregorianChange(UDate date, UErrorCode& status) fGregorianCutoverYear = cal->get(UCAL_YEAR, status); if (cal->get(UCAL_ERA, status) == BC) fGregorianCutoverYear = 1 - fGregorianCutoverYear; - fCutoverJulianDay = cutoverDay; + fCutoverJulianDay = (int32_t)cutoverDay; delete cal; } diff --git a/icu4c/source/test/intltest/calregts.cpp b/icu4c/source/test/intltest/calregts.cpp index 8806d04be18..744ca69baf8 100644 --- a/icu4c/source/test/intltest/calregts.cpp +++ b/icu4c/source/test/intltest/calregts.cpp @@ -96,6 +96,7 @@ CalendarRegressionTest::runIndexedTest( int32_t index, UBool exec, const char* & CASE(52,TestPersianCalOverflow); CASE(53,TestIslamicCalOverflow); CASE(54,TestWeekOfYear13548); + CASE(55,Test13745); default: name = ""; break; } } @@ -1536,6 +1537,41 @@ void CalendarRegressionTest::test4141665() delete cal2; } +const UDate MILLIS_IN_DAY = 86400000.0; +/** + * ICU-13745 + * GregorianCalendar::setGregorianChange() overflow + */ +void CalendarRegressionTest::Test13745() +{ + UErrorCode status = U_ZERO_ERROR; + GregorianCalendar *cal = new GregorianCalendar(status); + if(U_FAILURE(status)) { + dataerrln("Error creating calendar %s", u_errorName(status)); + delete cal; + return; + } + + // this line would overflow before fix 13745 + cal->setGregorianChange(((double)INT32_MAX+1.0) * MILLIS_IN_DAY, status); + if(U_FAILURE(status)) { + errln("%s:%d Failure setting INT32_MAX+1 change on calendar: %s\n", __FILE__, __LINE__, u_errorName(status)); + return; + } + assertEquals("getGregorianChange()", (double)INT32_MAX * MILLIS_IN_DAY, cal->getGregorianChange()); + + // test underflow + cal->setGregorianChange(((double)INT32_MIN-1.0) * MILLIS_IN_DAY, status); + if(U_FAILURE(status)) { + errln("%s:%d Failure setting INT32_MAX-1 change on calendar: %s\n", __FILE__, __LINE__, u_errorName(status)); + return; + } + assertEquals("getGregorianChange()", (double)INT32_MIN * MILLIS_IN_DAY, cal->getGregorianChange()); + + delete cal; +} + + /** * @bug 4142933 * Bug states that ArrayIndexOutOfBoundsException is thrown by GregorianCalendar::roll() diff --git a/icu4c/source/test/intltest/calregts.h b/icu4c/source/test/intltest/calregts.h index b4166a0c61d..6fc4bdda16e 100644 --- a/icu4c/source/test/intltest/calregts.h +++ b/icu4c/source/test/intltest/calregts.h @@ -82,6 +82,8 @@ public: void TestIslamicCalOverflow(void); void TestWeekOfYear13548(void); + void Test13745(void); + void printdate(GregorianCalendar *cal, const char *string); void dowTest(UBool lenient) ;