From 9d40ecacb50ef88c285e1b1d353d945c68a34a95 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Wed, 24 May 2023 22:06:39 +0200 Subject: [PATCH] AK: Fix signed overflow in unix time parts parsing --- AK/Time.h | 22 +++++++++++----------- Tests/AK/TestDuration.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/AK/Time.h b/AK/Time.h index ec3654fbb4..455fa36afa 100644 --- a/AK/Time.h +++ b/AK/Time.h @@ -381,20 +381,20 @@ public: // Note that the returned time is probably not equivalent to the same timestamp in UTC time, since UNIX time does not observe leap seconds. [[nodiscard]] constexpr static UnixDateTime from_unix_time_parts(i32 year, u8 month, u8 day, u8 hour, u8 minute, u8 second, u16 millisecond) { - constexpr auto milliseconds_per_day = 86'400'000; - constexpr auto milliseconds_per_hour = 3'600'000; - constexpr auto milliseconds_per_minute = 60'000; - constexpr auto milliseconds_per_second = 1'000; + constexpr auto seconds_per_day = 86'400; + constexpr auto seconds_per_hour = 3'600; + constexpr auto seconds_per_minute = 60; i64 days = days_since_epoch(year, month, day); - i64 milliseconds_since_epoch = days * milliseconds_per_day; + // With year=2'147'483'648, we can end up with days=569'603'931'504. + // Expressing that in milliseconds would require more than 64 bits, + // so we must choose seconds here, and not milliseconds. + i64 seconds_since_epoch = days * seconds_per_day; - milliseconds_since_epoch += hour * milliseconds_per_hour; - milliseconds_since_epoch += minute * milliseconds_per_minute; - milliseconds_since_epoch += second * milliseconds_per_second; - milliseconds_since_epoch += millisecond; - - return from_milliseconds_since_epoch(milliseconds_since_epoch); + seconds_since_epoch += hour * seconds_per_hour; + seconds_since_epoch += minute * seconds_per_minute; + seconds_since_epoch += second; + return from_seconds_since_epoch(seconds_since_epoch) + Duration::from_milliseconds(millisecond); } [[nodiscard]] constexpr static UnixDateTime from_seconds_since_epoch(i64 seconds) diff --git a/Tests/AK/TestDuration.cpp b/Tests/AK/TestDuration.cpp index 261a5fdf2f..2beaa20cd1 100644 --- a/Tests/AK/TestDuration.cpp +++ b/Tests/AK/TestDuration.cpp @@ -608,3 +608,41 @@ TEST_CASE(from_milliseconds) EXPECT_DURATION(Duration::from_milliseconds(9223372036854775807), 9223372036854775, 807'000'000); EXPECT_DURATION(Duration::from_milliseconds((i64)-0x8000'0000'0000'0000), -9223372036854776, 192'000'000); } + +TEST_CASE(from_unix_time_parts_overflow) +{ + // Negative overflow + // I can't easily verify that these values are perfectly exact and correct, but they're close enough. + // Also, for these "years" the most important thing is to avoid crashing (i.e. signed overflow UB). + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(0, 1, 1, 0, 0, 0, 0).offset_to_epoch(), -62167219200, 0); // Guess: -62167195440, off by 23760 seconds + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(-1'000'000, 1, 1, 0, 0, 0, 0).offset_to_epoch(), -31619119219200, 0); // Guess: -31619119195440, off by the same 23760 seconds + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(-2'147'483'648, 1, 1, 0, 0, 0, 0).offset_to_epoch(), -67768100567971200, 0); // Guess: -67768100567916336, off by 54864 seconds + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(-2'147'483'648, 1, 0, 0, 0, 0, 0).offset_to_epoch(), -67768100568057600, 0); // Guess: -67768100568002736, off by the same 54864 seconds + // EXPECT_DURATION(UnixDateTime::from_unix_time_parts(-2'147'483'648, 0, 0, 0, 0, 0, 0).offset_to_epoch(), -67768100568006336, 0); // Guess: -67768100568006336 + EXPECT_CRASH("Month 0 currently not allowed", [] { + (void)UnixDateTime::from_unix_time_parts(-2'147'483'648, 0, 0, 0, 0, 0, 0); + return Test::Crash::Failure::DidNotCrash; + }); + + // Positive overflow + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 1, 0, 0, 0, 65535).offset_to_epoch(), 65, 535'000'000); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 1, 0, 0, 255, 0).offset_to_epoch(), 255, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 1, 0, 255, 0, 0).offset_to_epoch(), 15300, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 1, 255, 0, 0, 0).offset_to_epoch(), 918000, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 255, 0, 0, 0, 0).offset_to_epoch(), 21945600, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 12, 1, 0, 0, 0, 0).offset_to_epoch(), 28857600, 0); + // EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 255, 1, 0, 0, 0, 0).offset_to_epoch(), 670585230, 0); + EXPECT_CRASH("Month 255 currently crashes", [] { + (void)UnixDateTime::from_unix_time_parts(1970, 255, 1, 0, 0, 0, 0); + return Test::Crash::Failure::DidNotCrash; + }); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1'000'000, 1, 1, 0, 0, 0, 0).offset_to_epoch(), 31494784780800, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(2'147'483'647, 1, 1, 0, 0, 0, 0).offset_to_epoch(), 67767976201996800, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(2'147'483'647, 12, 255, 0, 0, 0, 0).offset_to_epoch(), 67767976252800000, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(2'147'483'647, 12, 255, 255, 255, 255, 65535).offset_to_epoch(), 67767976253733620, 535'000'000); + // EXPECT_DURATION(UnixDateTime::from_unix_time_parts(2'147'483'647, 255, 255, 255, 255, 255, 65535).offset_to_epoch(), ????, 535'000'000); + EXPECT_CRASH("Month 255 currently crashes", [] { + (void)UnixDateTime::from_unix_time_parts(2'147'483'647, 255, 255, 255, 255, 255, 65535); + return Test::Crash::Failure::DidNotCrash; + }); +}