From 5fafd829272326676be9b85e4761d62592e09334 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Wed, 24 May 2023 22:19:17 +0200 Subject: [PATCH] AK+Everywhere: Don't crash on invalid months Sadly, we don't have proper error propagation here. However, crashing the Kernel just because a CDROM contains an invalid month seems like a bad idea. --- AK/Time.h | 5 +++- Kernel/FileSystem/ISO9660FS/Inode.cpp | 2 -- Tests/AK/TestDuration.cpp | 43 ++++++--------------------- 3 files changed, 13 insertions(+), 37 deletions(-) diff --git a/AK/Time.h b/AK/Time.h index 455fa36afa7..6f80075f338 100644 --- a/AK/Time.h +++ b/AK/Time.h @@ -54,7 +54,10 @@ unsigned day_of_week(int year, unsigned month, int day); // can be negative. constexpr int day_of_year(int year, unsigned month, int day) { - VERIFY(month >= 1 && month <= 12); + if (is_constant_evaluated()) + VERIFY(month >= 1 && month <= 12); // Note that this prevents bad constexpr months, but never actually prints anything. + else if (!(month >= 1 && month <= 12)) + return 0; constexpr Array seek_table = { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334 }; int day_of_year = seek_table[month - 1] + day - 1; diff --git a/Kernel/FileSystem/ISO9660FS/Inode.cpp b/Kernel/FileSystem/ISO9660FS/Inode.cpp index 1160e300bc9..3f2d4f121b9 100644 --- a/Kernel/FileSystem/ISO9660FS/Inode.cpp +++ b/Kernel/FileSystem/ISO9660FS/Inode.cpp @@ -194,8 +194,6 @@ UnixDateTime ISO9660Inode::parse_numerical_date_time(ISO::NumericalDateAndTime c i32 year_offset = date.years_since_1900 - 70; // FIXME: This ignores timezone information in date. - // FIXME: This calls `VERIFY(month >= 1 && month <= 12)` (but irritatingly allows days 0 and 255), - // which means we can crash the kernel by inserting a CD with an invalid recording month. return UnixDateTime::from_unix_time_parts(year_offset, date.month, date.day, date.hour, date.minute, date.second, 0); } diff --git a/Tests/AK/TestDuration.cpp b/Tests/AK/TestDuration.cpp index 2beaa20cd1a..ea4e92394b4 100644 --- a/Tests/AK/TestDuration.cpp +++ b/Tests/AK/TestDuration.cpp @@ -347,9 +347,9 @@ TEST_CASE(days_since_epoch) // At least shouldn't crash: EXPECT_EQ(days_since_epoch(1971, 1, 0), 364); - // FIXME shouldn't crash: EXPECT_EQ(days_since_epoch(1971, 0, 1), 334); - // FIXME shouldn't crash: EXPECT_EQ(days_since_epoch(1971, 0, 0), 333); - // FIXME shouldn't crash: EXPECT_EQ(days_since_epoch(1971, 13, 3), ???); + EXPECT_EQ(days_since_epoch(1971, 0, 1), 365); + EXPECT_EQ(days_since_epoch(1971, 0, 0), 365); + EXPECT_EQ(days_since_epoch(1971, 13, 3), 365); // 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). @@ -564,27 +564,14 @@ TEST_CASE(from_unix_time_parts_common_values) TEST_CASE(from_unix_time_parts_negative) { // Negative "common" values. These aren't really that well-defined, but we must make sure we don't crash. - // FIXME: We shouldn't VERIFY(), especially because this is used in the Kernel! EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 0, 23, 0, 0, 0).offset_to_epoch(), -3600, 0); EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 0, 24, 0, 0, 0).offset_to_epoch(), 0, 0); - // EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 0, 31, 0, 0, 0, 0).offset_to_epoch(), 0, 0); - EXPECT_CRASH("Month 0 currently not allowed", [] { - (void)UnixDateTime::from_unix_time_parts(1970, 0, 31, 0, 0, 0, 0); - return Test::Crash::Failure::DidNotCrash; - }); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 0, 31, 0, 0, 0, 0).offset_to_epoch(), 0, 0); EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 11, 30, 0, 0, 0, 0).offset_to_epoch(), 28771200, 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, 12, 31, 0, 0, 0, 0).offset_to_epoch(), 31449600, 0); - // EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1971, 0, 0, 0, 0, 0, 0).offset_to_epoch(), 28857600, 0); - EXPECT_CRASH("Month 0 currently not allowed", [] { - (void)UnixDateTime::from_unix_time_parts(1971, 0, 0, 0, 0, 0, 0); - return Test::Crash::Failure::DidNotCrash; - }); - // EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1971, 0, 1, 0, 0, 0, 0).offset_to_epoch(), 28944000, 0); - EXPECT_CRASH("Month 0 currently not allowed", [] { - (void)UnixDateTime::from_unix_time_parts(1971, 0, 1, 0, 0, 0, 0); - return Test::Crash::Failure::DidNotCrash; - }); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1971, 0, 0, 0, 0, 0, 0).offset_to_epoch(), 31536000, 0); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1971, 0, 1, 0, 0, 0, 0).offset_to_epoch(), 31536000, 0); EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1971, 1, 0, 0, 0, 0, 0).offset_to_epoch(), 31449600, 0); EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1971, 1, 1, 0, 0, 0, 0).offset_to_epoch(), 31536000, 0); EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1969, 1, 1, 0, 0, 0, 0).offset_to_epoch(), -31536000, 0); @@ -618,11 +605,7 @@ TEST_CASE(from_unix_time_parts_overflow) 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; - }); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(-2'147'483'648, 0, 0, 0, 0, 0, 0).offset_to_epoch(), -67768100567971200, 0); // Positive overflow EXPECT_DURATION(UnixDateTime::from_unix_time_parts(1970, 1, 1, 0, 0, 0, 65535).offset_to_epoch(), 65, 535'000'000); @@ -631,18 +614,10 @@ TEST_CASE(from_unix_time_parts_overflow) 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(1970, 255, 1, 0, 0, 0, 0).offset_to_epoch(), 0, 0); 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; - }); + EXPECT_DURATION(UnixDateTime::from_unix_time_parts(2'147'483'647, 255, 255, 255, 255, 255, 65535).offset_to_epoch(), 67767976202930420, 535'000'000); }